Skip to content

Commit 2942e37

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

File tree

4 files changed

+341
-0
lines changed

4 files changed

+341
-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

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