Skip to content

Commit cd74bf2

Browse files
committed
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.
1 parent 5b23bd4 commit cd74bf2

File tree

3 files changed

+125
-8
lines changed

3 files changed

+125
-8
lines changed

clippy_utils/src/consts.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -805,10 +805,10 @@ impl<'tcx> ConstEvalCtxt<'tcx> {
805805
| sym::i128_legacy_const_max
806806
)
807807
) || self.tcx.opt_parent(did).is_some_and(|parent| {
808-
paths::F16_CONSTS.matches(&self.tcx, parent)
809-
|| paths::F32_CONSTS.matches(&self.tcx, parent)
810-
|| paths::F64_CONSTS.matches(&self.tcx, parent)
811-
|| paths::F128_CONSTS.matches(&self.tcx, parent)
808+
paths::DIAG_ITEM_F16_CONSTS_MOD.matches(&self.tcx, parent)
809+
|| paths::DIAG_ITEM_F32_CONSTS_MOD.matches(&self.tcx, parent)
810+
|| paths::DIAG_ITEM_F64_CONSTS_MOD.matches(&self.tcx, parent)
811+
|| paths::DIAG_ITEM_F128_CONSTS_MOD.matches(&self.tcx, parent)
812812
})) =>
813813
{
814814
did

clippy_utils/src/paths.rs

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,35 @@ path_macros! {
127127
macro_path: PathNS::Macro,
128128
}
129129

130-
pub static F16_CONSTS: PathLookup = type_path!(core::f16::consts);
131-
pub static F32_CONSTS: PathLookup = type_path!(core::f32::consts);
132-
pub static F64_CONSTS: PathLookup = type_path!(core::f64::consts);
133-
pub static F128_CONSTS: PathLookup = type_path!(core::f128::consts);
130+
// Paths in standard library (`alloc`/`core`/`std`, or against builtin scalar types)
131+
// should be added as diagnostic items directly into the standard library, through a
132+
// PR against the `rust-lang/rust` repository. That makes Clippy more robust in case
133+
// the item is moved around, e.g. if the library structure is reorganized.
134+
//
135+
// If their use is required before the next sync (which happens every two weeks),
136+
// they can be temporarily added below, prefixed with `DIAG_ITEM`, and commented
137+
// with a reference to the PR adding the diagnostic item:
138+
//
139+
// ```rust
140+
// // `sym::io_error_new` added in <https://github.com/rust-lang/rust/pull/142787>
141+
// pub static DIAG_ITEM_IO_ERROR_NEW: PathLookup = value_path!(std::io::Error::new);
142+
// ```
143+
//
144+
// During development, the "Added in …" comment is not required, but will make the
145+
// test fail once the PR is submitted and becomes mandatory to ensure that a proper
146+
// PR against `rust-lang/rust` has been created.
147+
//
148+
// You can request advice from the Clippy team members if you are not sure of how to
149+
// add the diagnostic item to the standard library, or how to name it.
150+
151+
// `sym::f16_consts_mod` added in <https://github.com/rust-lang/rust/pull/147420>
152+
pub static DIAG_ITEM_F16_CONSTS_MOD: PathLookup = type_path!(core::f16::consts);
153+
// `sym::f32_consts_mod` added in <https://github.com/rust-lang/rust/pull/147420>
154+
pub static DIAG_ITEM_F32_CONSTS_MOD: PathLookup = type_path!(core::f32::consts);
155+
// `sym::f64_consts_mod` added in <https://github.com/rust-lang/rust/pull/147420>
156+
pub static DIAG_ITEM_F64_CONSTS_MOD: PathLookup = type_path!(core::f64::consts);
157+
// `sym::f128_consts_mod` added in <https://github.com/rust-lang/rust/pull/147420>
158+
pub static DIAG_ITEM_F128_CONSTS_MOD: PathLookup = type_path!(core::f128::consts);
134159

135160
// Paths in external crates
136161
pub static FUTURES_IO_ASYNCREADEXT: PathLookup = type_path!(futures_util::AsyncReadExt);

tests/stdlib-diag-items.rs

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// This tests checks that if a path is defined for an entity in the standard
2+
// library, the proper prefix is used and a reference to a PR against
3+
// `rust-lang/rust` is mentionned.
4+
5+
// This test is a no-op if run as part of the compiler test suite
6+
// and will always succeed.
7+
8+
use itertools::Itertools;
9+
use regex::Regex;
10+
use std::io;
11+
12+
const PATHS_FILE: &str = "clippy_utils/src/paths.rs";
13+
14+
fn parse_content(content: &str) -> Vec<String> {
15+
let comment_re = Regex::new(r"^// `sym::(.*)` added in <https://github.com/rust-lang/rust/pull/\d+>$").unwrap();
16+
let path_re =
17+
Regex::new(r"^pub static ([A-Z_]+): PathLookup = (?:macro|type|value)_path!\((([a-z]+)::.*)\);").unwrap();
18+
let mut errors = vec![];
19+
for (prev, line) in content.lines().tuple_windows() {
20+
if let Some(caps) = path_re.captures(line) {
21+
if ["alloc", "core", "std"].contains(&&caps[3]) && !caps[1].starts_with("DIAG_ITEM_") {
22+
errors.push(format!(
23+
"Path `{}` for `{}` should start with `DIAG_ITEM`",
24+
&caps[1], &caps[2]
25+
));
26+
continue;
27+
}
28+
if let Some(upper) = caps[1].strip_prefix("DIAG_ITEM_") {
29+
let Some(comment) = comment_re.captures(prev) else {
30+
errors.push(format!(
31+
"Definition for `{}` should be preceded by PR-related comment",
32+
&caps[1]
33+
));
34+
continue;
35+
};
36+
let upper_sym = comment[1].to_uppercase();
37+
if upper != upper_sym {
38+
errors.push(format!(
39+
"Path for symbol `{}` should be named `DIAG_ITEM_{}`",
40+
&comment[1], upper_sym
41+
));
42+
}
43+
}
44+
}
45+
}
46+
errors
47+
}
48+
49+
#[test]
50+
fn stdlib_diag_items() -> Result<(), io::Error> {
51+
if option_env!("RUSTC_TEST_SUITE").is_some() {
52+
return Ok(());
53+
}
54+
55+
let diagnostics = parse_content(&std::fs::read_to_string(PATHS_FILE)?);
56+
if diagnostics.is_empty() {
57+
Ok(())
58+
} else {
59+
eprintln!("Issues found in {PATHS_FILE}:");
60+
for diag in diagnostics {
61+
eprintln!("- {diag}");
62+
}
63+
Err(io::Error::other("problems found"))
64+
}
65+
}
66+
67+
#[test]
68+
fn internal_diag_items_test() {
69+
let content = r"
70+
// Missing comment
71+
pub static DIAG_ITEM_IO_ERROR_NEW: PathLookup = value_path!(std::io::Error::new);
72+
73+
// Wrong static name
74+
// `sym::io_error` added in <https://github.com/rust-lang/rust/pull/142787>
75+
pub static DIAG_ITEM_ERROR: PathLookup = value_path!(std::io::Error);
76+
77+
// Missing DIAG_ITEM
78+
// `sym::io_foobar` added in <https://github.com/rust-lang/rust/pull/142787>
79+
pub static IO_FOOBAR_PATH: PathLookup = value_path!(std::io);
80+
";
81+
82+
let diags = parse_content(content);
83+
let diags = diags.iter().map(String::as_str).collect::<Vec<_>>();
84+
assert_eq!(
85+
diags.as_slice(),
86+
[
87+
"Definition for `DIAG_ITEM_IO_ERROR_NEW` should be preceded by PR-related comment",
88+
"Path for symbol `io_error` should be named `DIAG_ITEM_IO_ERROR`",
89+
"Path `IO_FOOBAR_PATH` for `std::io` should start with `DIAG_ITEM`"
90+
]
91+
);
92+
}

0 commit comments

Comments
 (0)