From f1bc3768054856b775a29502e8fc7ea323130582 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Sat, 21 Jun 2025 15:43:20 +0200 Subject: [PATCH] Explain how to temporarily use paths instead of diagnostic items Paths into the standard library may be added temporarily until a diagnostic item is added to the library and synced with Clippy sources. This documents how to do this. This also adds a test to check that consistent and easy to notice names are used, and that a PR into `rust-lang/rust` has been opened to add the corresponding diagnostic items. The test is a no-op if run as part of the compiler test suite and will always succeed. --- clippy_utils/src/paths.rs | 21 +++++++++ tests/stdlib-diag-items.rs | 92 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 tests/stdlib-diag-items.rs diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 8bbcb220210a..ba372e30d92b 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -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 +// 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); diff --git a/tests/stdlib-diag-items.rs b/tests/stdlib-diag-items.rs new file mode 100644 index 000000000000..d93cf621402f --- /dev/null +++ b/tests/stdlib-diag-items.rs @@ -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 { + let comment_re = Regex::new(r"^// `sym::(.*)` added in $").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 +pub static DIAG_ITEM_ERROR: PathLookup = value_path!(std::io::Error); + +// Missing DIAG_ITEM +// `sym::io_foobar` added in +pub static IO_FOOBAR_PATH: PathLookup = value_path!(std::io); +"; + + let diags = parse_content(content); + let diags = diags.iter().map(String::as_str).collect::>(); + 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`" + ] + ); +}