Skip to content

Commit c7aa411

Browse files
y21y21
committed
add a book page for lighter, more granular lint groups
1 parent a01975b commit c7aa411

File tree

4 files changed

+353
-0
lines changed

4 files changed

+353
-0
lines changed

‎Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ serde_json = "1.0.122"
3838
toml = "0.7.3"
3939
walkdir = "2.3"
4040
filetime = "0.2.9"
41+
indoc = "1.0"
4142
itertools = "0.12"
4243

4344
# UI test dependencies

‎book/src/SUMMARY.md

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
- [Configuration](configuration.md)
88
- [Lint Configuration](lint_configuration.md)
99
- [Clippy's Lints](lints.md)
10+
- [More granular lint groups](granular_lint_groups.md)
1011
- [Continuous Integration](continuous_integration/README.md)
1112
- [GitHub Actions](continuous_integration/github_actions.md)
1213
- [GitLab CI](continuous_integration/gitlab.md)

‎book/src/granular_lint_groups.md

+219
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
1+
# More granular lint groups
2+
3+
Clippy groups its lints into [9 primary categories](lints.md),
4+
two of which are allow-by-default (pedantic and restriction).
5+
6+
One downside with having such few but broad categories for allow-by-default lints
7+
is that it significantly decreases discoverability, as `restriction` often acts as a blanket category
8+
for any lint that most users likely would not want enforced on their codebase.
9+
10+
This page should help with that, by defining more granular, unofficial lint groups.
11+
For example, some people might not be interested in all the style-related `pedantic` lints,
12+
but *are* interested in the `perf`-related ones, so it can be worth going through some of these.
13+
14+
<!--
15+
NOTE: Do not edit the contents in between lint-group-start and lint-group-end manually.
16+
These sections are generated based on the `GROUPS` array defined in tests/lint-groups.rs,
17+
so consider updating that instead and re-running `cargo bless`.
18+
The descriptions however are fine to edit.
19+
-->
20+
21+
## Perf-pedantic
22+
23+
These are `pedantic` lints that look for code patterns that could be expressed in a more efficient way.
24+
These would be candidates for the `perf` category, however suggestions made by them can also sometimes hurt readability
25+
and obfuscate the meaning, so occasional `#[allow]`s are expected to be used.
26+
27+
<!-- lint-group-start: perf-pedantic -->
28+
Lints: `assigning_clones`, `inefficient_to_string`, `naive_bytecount`, `needless_bitwise_bool`, `trivially_copy_pass_by_ref`, `unnecessary_join`, `unnecessary_box_returns`
29+
30+
<details>
31+
<summary>#![warn] attribute</summary>
32+
33+
```
34+
#![warn(
35+
clippy::assigning_clones,
36+
clippy::inefficient_to_string,
37+
clippy::naive_bytecount,
38+
clippy::needless_bitwise_bool,
39+
clippy::trivially_copy_pass_by_ref,
40+
clippy::unnecessary_join,
41+
clippy::unnecessary_box_returns
42+
)]
43+
```
44+
</details>
45+
46+
<details>
47+
<summary>Lint table</summary>
48+
49+
```
50+
[lints.clippy]
51+
assigning_clones = "warn"
52+
inefficient_to_string = "warn"
53+
naive_bytecount = "warn"
54+
needless_bitwise_bool = "warn"
55+
trivially_copy_pass_by_ref = "warn"
56+
unnecessary_join = "warn"
57+
unnecessary_box_returns = "warn"
58+
```
59+
</details>
60+
<!-- lint-group-end: perf-pedantic -->
61+
62+
63+
## Perf-restriction
64+
65+
These are `restriction` lints that can improve the performance of code, but are very specific
66+
and sometimes *significantly* hurt readability with very little gain in the usual case.
67+
These should ideally only be applied to specific functions or modules that were profiled
68+
and where it is very clear that any performance gain matters.
69+
70+
As always (but especially here), you should double-check that applying these actually helps
71+
and that any performance wins are worth the introduced complexity.
72+
73+
<!-- lint-group-start: perf-restriction -->
74+
Lints: `format_push_string`, `missing_asserts_for_indexing`
75+
76+
<details>
77+
<summary>#![warn] attribute</summary>
78+
79+
```
80+
#![warn(
81+
clippy::format_push_string,
82+
clippy::missing_asserts_for_indexing
83+
)]
84+
```
85+
</details>
86+
87+
<details>
88+
<summary>Lint table</summary>
89+
90+
```
91+
[lints.clippy]
92+
format_push_string = "warn"
93+
missing_asserts_for_indexing = "warn"
94+
```
95+
</details>
96+
<!-- lint-group-end: perf-restriction -->
97+
98+
## Perf-nursery
99+
100+
These are `nursery` lints that either were previously in the `perf` category or are intended to be in `perf`
101+
but have too many false positives.
102+
Some of them may also be simply wrong in certain situations and end up slower,
103+
so you should make sure to read the description to learn about possible edge cases.
104+
105+
<!-- lint-group-start: perf-nursery -->
106+
Lints: `redundant_clone`, `iter_with_drain`, `mutex_integer`, `or_fun_call`, `significant_drop_tightening`, `trivial_regex`, `needless_collect`
107+
108+
<details>
109+
<summary>#![warn] attribute</summary>
110+
111+
```
112+
#![warn(
113+
clippy::redundant_clone,
114+
clippy::iter_with_drain,
115+
clippy::mutex_integer,
116+
clippy::or_fun_call,
117+
clippy::significant_drop_tightening,
118+
clippy::trivial_regex,
119+
clippy::needless_collect
120+
)]
121+
```
122+
</details>
123+
124+
<details>
125+
<summary>Lint table</summary>
126+
127+
```
128+
[lints.clippy]
129+
redundant_clone = "warn"
130+
iter_with_drain = "warn"
131+
mutex_integer = "warn"
132+
or_fun_call = "warn"
133+
significant_drop_tightening = "warn"
134+
trivial_regex = "warn"
135+
needless_collect = "warn"
136+
```
137+
</details>
138+
<!-- lint-group-end: perf-nursery -->
139+
140+
## Panicking
141+
142+
These are `restriction` lints that look for patterns that can introduce panics.
143+
144+
Usually panics are not something that one should want to avoid and most of the time panicking is perfectly valid
145+
(hence why these lints are allow-by-default),
146+
but users may want to forbid any use of panicky functions altogether in specific contexts.
147+
148+
One use case could be to annotate `GlobalAlloc` impls in which unwinding is Undefined Behavior.
149+
150+
<!-- lint-group-start: panicking -->
151+
Lints: `arithmetic_side_effects`, `expect_used`, `unwrap_used`, `panic`, `unreachable`, `todo`, `unimplemented`, `string_slice`, `indexing_slicing`
152+
153+
<details>
154+
<summary>#![warn] attribute</summary>
155+
156+
```
157+
#![warn(
158+
clippy::arithmetic_side_effects,
159+
clippy::expect_used,
160+
clippy::unwrap_used,
161+
clippy::panic,
162+
clippy::unreachable,
163+
clippy::todo,
164+
clippy::unimplemented,
165+
clippy::string_slice,
166+
clippy::indexing_slicing
167+
)]
168+
```
169+
</details>
170+
171+
<details>
172+
<summary>Lint table</summary>
173+
174+
```
175+
[lints.clippy]
176+
arithmetic_side_effects = "warn"
177+
expect_used = "warn"
178+
unwrap_used = "warn"
179+
panic = "warn"
180+
unreachable = "warn"
181+
todo = "warn"
182+
unimplemented = "warn"
183+
string_slice = "warn"
184+
indexing_slicing = "warn"
185+
```
186+
</details>
187+
<!-- lint-group-end: panicking -->
188+
189+
## Debugging
190+
191+
These are lints that can be useful to disable in CI, as they might indicate that code needs more work
192+
or has remaining debugging artifacts.
193+
194+
<!-- lint-group-start: debugging -->
195+
Lints: `dbg_macro`, `todo`, `unimplemented`
196+
197+
<details>
198+
<summary>#![warn] attribute</summary>
199+
200+
```
201+
#![warn(
202+
clippy::dbg_macro,
203+
clippy::todo,
204+
clippy::unimplemented
205+
)]
206+
```
207+
</details>
208+
209+
<details>
210+
<summary>Lint table</summary>
211+
212+
```
213+
[lints.clippy]
214+
dbg_macro = "warn"
215+
todo = "warn"
216+
unimplemented = "warn"
217+
```
218+
</details>
219+
<!-- lint-group-end: debugging -->

‎tests/lint-groups.rs

+132
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
//! This test checks and updates `book/src/granular_lint_groups.md`
2+
//!
3+
//! Specifically the parts in between lint-group-start and lint-group-end are not meant to be edited
4+
//! by hand and are instead generated based on the const `GROUPS` slice.
5+
#![feature(rustc_private)]
6+
7+
use std::collections::HashSet;
8+
use std::{env, fs};
9+
10+
use clippy_lints::LintInfo;
11+
use clippy_lints::declared_lints::LINTS;
12+
use indoc::formatdoc;
13+
use itertools::Itertools;
14+
use regex::{Captures, Regex};
15+
16+
const GROUPS: &[(&str, &[&str])] = &[
17+
("perf-pedantic", &[
18+
"assigning_clones",
19+
"inefficient_to_string",
20+
"naive_bytecount",
21+
"needless_bitwise_bool",
22+
"trivially_copy_pass_by_ref",
23+
"unnecessary_join",
24+
"unnecessary_box_returns",
25+
]),
26+
("perf-restriction", &[
27+
"format_push_string",
28+
"missing_asserts_for_indexing",
29+
]),
30+
("perf-nursery", &[
31+
"redundant_clone",
32+
"iter_with_drain",
33+
"mutex_integer",
34+
"or_fun_call",
35+
"significant_drop_tightening",
36+
"trivial_regex",
37+
"needless_collect",
38+
]),
39+
("panicking", &[
40+
"arithmetic_side_effects",
41+
"expect_used",
42+
"unwrap_used",
43+
"panic",
44+
"unreachable",
45+
"todo",
46+
"unimplemented",
47+
"string_slice",
48+
"indexing_slicing",
49+
]),
50+
("debugging", &["dbg_macro", "todo", "unimplemented"]),
51+
];
52+
53+
#[test]
54+
fn check_lint_groups() {
55+
let file = fs::read_to_string("book/src/granular_lint_groups.md").expect("failed to read granular_lint_groups.md");
56+
let all_lint_names: HashSet<_> = LINTS
57+
.iter()
58+
.map(|LintInfo { lint, .. }| lint.name.strip_prefix("clippy::").unwrap().to_ascii_lowercase())
59+
.collect();
60+
61+
let regex = Regex::new(
62+
"(?s)\
63+
(?<header><!-- lint-group-start: (?<name_start>[\\w-]+) -->)\
64+
(?<lints>.*?)\
65+
(?<footer><!-- lint-group-end: (?<name_end>[\\w-]+) -->)\
66+
",
67+
)
68+
.unwrap();
69+
70+
let replaced = regex.replace_all(&file, |captures: &Captures<'_>| -> String {
71+
let name = &captures["name_start"];
72+
73+
assert_eq!(
74+
name, &captures["name_end"],
75+
"lint-group-start and lint-group-end lint names must match"
76+
);
77+
78+
let lints = GROUPS
79+
.iter()
80+
.find_map(|&(name2, lints)| (name == name2).then_some(lints))
81+
.unwrap_or_else(|| panic!("lint group {name} does not exist"));
82+
83+
for &lint in lints {
84+
assert!(
85+
all_lint_names.contains(lint),
86+
"lint {lint} in group {name} does not exist"
87+
);
88+
}
89+
90+
let spoiler = |summary: &str, contents: &str| {
91+
formatdoc! {"
92+
<details>
93+
<summary>{summary}</summary>
94+
95+
```
96+
{contents}
97+
```
98+
</details>
99+
"}
100+
};
101+
102+
let lint_list = format!("Lints: {}", lints.iter().map(|lint| format!("`{lint}`")).join(", "));
103+
let warn_attr = spoiler(
104+
"#![warn] attribute",
105+
&format!(
106+
"#![warn({})]",
107+
lints.iter().map(|lint| format!("\n clippy::{lint}")).join(",") + "\n"
108+
),
109+
);
110+
let lint_table = spoiler(
111+
"Lint table",
112+
&format!(
113+
"[lints.clippy]\n{}",
114+
&lints.iter().map(|lint| format!(r#"{lint} = "warn""#)).join("\n")
115+
),
116+
);
117+
118+
format!(
119+
"{header}\n{lint_list}\n\n{warn_attr}\n{lint_table}{footer}",
120+
header = &captures["header"],
121+
footer = &captures["footer"]
122+
)
123+
});
124+
125+
if replaced != file {
126+
if env::var_os("RUSTC_BLESS").is_some_and(|n| n != "0") {
127+
fs::write("book/src/granular_lint_groups.md", &*replaced).unwrap();
128+
} else {
129+
panic!("granular_lint_groups.md file has changed! Run `cargo bless --test lint-groups` to update.");
130+
}
131+
}
132+
}

0 commit comments

Comments
 (0)