Skip to content

Explain how to temporarily use paths instead of diagnostic items #15150

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions clippy_utils/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,27 @@ path_macros! {
macro_path: PathNS::Macro,
}

// Paths in standard library (`alloc`/`core`/`std`, or against builtin scalar types)
// should be added as diagnostic items directly into the standard library, through a
// PR against the `rust-lang/rust` repository. If makes Clippy more robust in case
// the item is moved around, e.g. if the library structure is reorganized.
//
// If their use is required before the next sync (which happens every two weeks),
// they can be temporarily added below, prefixed with `DIAG_ITEM`, and commented
// with a reference to the PR adding the diagnostic item:
//
// ```rust
// // `sym::io_error_new` added in <https://github.com/rust-lang/rust/pull/142787>
// pub static DIAG_ITEM_IO_ERROR_NEW: PathLookup = value_path!(std::io::Error::new);
// ```
//
// During development, the "Added in …" comment is not required, but will make the
// test fail once the PR is submitted and becomes mandatory to ensure that a proper
// PR against `rust-lang/rust` has been created.
//
// You can request advice from the Clippy team members if you are not sure of how to
// add the diagnostic item to the standard library, or how to name it.

// Paths in external crates
pub static FUTURES_IO_ASYNCREADEXT: PathLookup = type_path!(futures_util::AsyncReadExt);
pub static FUTURES_IO_ASYNCWRITEEXT: PathLookup = type_path!(futures_util::AsyncWriteExt);
Expand Down
92 changes: 92 additions & 0 deletions tests/stdlib-diag-items.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// This tests checks that if a path is defined for an entity in the standard
// library, the proper prefix is used and a reference to a PR against
// `rust-lang/rust` is mentionned.

// This test is a no-op if run as part of the compiler test suite
// and will always succeed.

use itertools::Itertools;
use regex::Regex;
use std::io;

const PATHS_FILE: &str = "clippy_utils/src/paths.rs";

fn parse_content(content: &str) -> Vec<String> {
let comment_re = Regex::new(r"^// `sym::(.*)` added in <https://github.com/rust-lang/rust/pull/\d+>$").unwrap();
let path_re =
Regex::new(r"^pub static ([A-Z_]+): PathLookup = (?:macro|type|value)_path!\((([a-z]+)::.*)\);").unwrap();
let mut errors = vec![];
for (prev, line) in content.lines().tuple_windows() {
if let Some(caps) = path_re.captures(line) {
if ["alloc", "core", "std"].contains(&&caps[3]) && !caps[1].starts_with("DIAG_ITEM_") {
errors.push(format!(
"Path `{}` for `{}` should start with `DIAG_ITEM`",
&caps[1], &caps[2]
));
continue;
}
if let Some(upper) = caps[1].strip_prefix("DIAG_ITEM_") {
let Some(comment) = comment_re.captures(prev) else {
errors.push(format!(
"Definition for `{}` should be preceded by PR-related comment",
&caps[1]
));
continue;
};
let upper_sym = comment[1].to_uppercase();
if upper != upper_sym {
errors.push(format!(
"Path for symbol `{}` should be named `DIAG_ITEM_{}`",
&comment[1], upper_sym
));
}
}
}
}
errors
}

#[test]
fn stdlib_diag_items() -> Result<(), io::Error> {
if option_env!("RUSTC_TEST_SUITE").is_some() {
return Ok(());
}

let diagnostics = parse_content(&std::fs::read_to_string(PATHS_FILE)?);
if diagnostics.is_empty() {
Ok(())
} else {
eprintln!("Issues found in {PATHS_FILE}:");
for diag in diagnostics {
eprintln!("- {diag}");
}
Err(io::Error::other("problems found"))
}
}

#[test]
fn internal_diag_items_test() {
let content = r"
// Missing comment
pub static DIAG_ITEM_IO_ERROR_NEW: PathLookup = value_path!(std::io::Error::new);

// Wrong static name
// `sym::io_error` added in <https://github.com/rust-lang/rust/pull/142787>
pub static DIAG_ITEM_ERROR: PathLookup = value_path!(std::io::Error);

// Missing DIAG_ITEM
// `sym::io_foobar` added in <https://github.com/rust-lang/rust/pull/142787>
pub static IO_FOOBAR_PATH: PathLookup = value_path!(std::io);
";

let diags = parse_content(content);
let diags = diags.iter().map(String::as_str).collect::<Vec<_>>();
assert_eq!(
diags.as_slice(),
[
"Definition for `DIAG_ITEM_IO_ERROR_NEW` should be preceded by PR-related comment",
"Path for symbol `io_error` should be named `DIAG_ITEM_IO_ERROR`",
"Path `IO_FOOBAR_PATH` for `std::io` should start with `DIAG_ITEM`"
]
);
}