Skip to content

Commit e574d7c

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 cd15aeb commit e574d7c

File tree

2 files changed

+116
-0
lines changed

2 files changed

+116
-0
lines changed

clippy_utils/src/paths.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,27 @@ path_macros! {
126126
macro_path: PathNS::Macro,
127127
}
128128

129+
// Paths in standard library (`alloc`/`core`/`std`, or against builtin scalar types)
130+
// should be added as diagnostic items directly into the standard library, through a
131+
// PR against the `rust-lang/rust` repository. If makes Clippy more robust in case
132+
// the item is moved around, e.g. if the library structure is reorganized.
133+
//
134+
// If their use is required before the next sync (which happens every two weeks),
135+
// they can be temporarily added below, prefixed with `DIAG_ITEM`, and commented
136+
// with a reference to the PR adding the diagnostic item:
137+
//
138+
// ```rust
139+
// // `sym::io_error_new` added in <https://github.com/rust-lang/rust/pull/142787>
140+
// pub static DIAG_ITEM_IO_ERROR_NEW: PathLookup = value_path!(std::io::Error::new);
141+
// ```
142+
//
143+
// During development, the "Added in …" comment is not required, but will make the
144+
// test fail once the PR is submitted and becomes mandatory to ensure that a proper
145+
// PR against `rust-lang/rust` has been created.
146+
//
147+
// You can request advice from the Clippy team members if you are not sure of how to
148+
// add the diagnostic item to the standard library, or how to name it.
149+
129150
// Paths in external crates
130151
pub static FUTURES_IO_ASYNCREADEXT: PathLookup = type_path!(futures_util::AsyncReadExt);
131152
pub static FUTURES_IO_ASYNCWRITEEXT: PathLookup = type_path!(futures_util::AsyncWriteExt);

tests/stdlib-diag-items.rs

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
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]) {
22+
if !caps[1].starts_with("DIAG_ITEM_") {
23+
errors.push(format!(
24+
"Path `{}` for `{}` should start with `DIAG_ITEM`",
25+
&caps[1], &caps[2]
26+
));
27+
continue;
28+
}
29+
}
30+
if let Some(upper) = caps[1].strip_prefix("DIAG_ITEM_") {
31+
let Some(comment) = comment_re.captures(prev) else {
32+
errors.push(format!(
33+
"Definition for `{}` should be preceded by PR-related comment",
34+
&caps[1]
35+
));
36+
continue;
37+
};
38+
let upper_sym = comment[1].to_uppercase();
39+
if upper != upper_sym {
40+
errors.push(format!(
41+
"Path for symbol `{}` should be named `DIAG_ITEM_{}`",
42+
&comment[1], upper_sym
43+
));
44+
continue;
45+
}
46+
}
47+
}
48+
}
49+
errors
50+
}
51+
52+
#[test]
53+
fn stdlib_diag_items() -> Result<(), io::Error> {
54+
if option_env!("RUSTC_TEST_SUITE").is_some() {
55+
return Ok(());
56+
}
57+
58+
let diagnostics = parse_content(&std::fs::read_to_string(PATHS_FILE)?);
59+
if diagnostics.is_empty() {
60+
Ok(())
61+
} else {
62+
eprintln!("Issues found in {PATHS_FILE}:");
63+
for diag in diagnostics {
64+
eprintln!("- {diag}");
65+
}
66+
Err(io::Error::new(io::ErrorKind::Other, "problems found"))
67+
}
68+
}
69+
70+
#[test]
71+
fn internal_diag_items_test() {
72+
let content = r"
73+
// Missing comment
74+
pub static DIAG_ITEM_IO_ERROR_NEW: PathLookup = value_path!(std::io::Error::new);
75+
76+
// Wrong static name
77+
// `sym::io_error` added in <https://github.com/rust-lang/rust/pull/142787>
78+
pub static DIAG_ITEM_ERROR: PathLookup = value_path!(std::io::Error);
79+
80+
// Missing DIAG_ITEM
81+
// `sym::io_foobar` added in <https://github.com/rust-lang/rust/pull/142787>
82+
pub static IO_FOOBAR_PATH: PathLookup = value_path!(std::io);
83+
";
84+
85+
let diags = parse_content(content);
86+
let diags = diags.iter().map(String::as_str).collect::<Vec<_>>();
87+
assert_eq!(
88+
diags.as_slice(),
89+
[
90+
"Definition for `DIAG_ITEM_IO_ERROR_NEW` should be preceded by PR-related comment",
91+
"Path for symbol `io_error` should be named `DIAG_ITEM_IO_ERROR`",
92+
"Path `IO_FOOBAR_PATH` for `std::io` should start with `DIAG_ITEM`"
93+
]
94+
);
95+
}

0 commit comments

Comments
 (0)