Skip to content

Commit b58b88c

Browse files
committed
Auto merge of #11832 - Alexendoo:lint-groups-priority, r=flip1995
Add `lint_groups_priority` lint Warns when a lint group in Cargo.toml's `[lints]` section shares the same priority as a lint. This is in the cargo section but is categorised as `correctness` so it's on by default, it doesn't call `cargo metadata` though and parses the `Cargo.toml` directly The lint should be temporary until rust-lang/cargo#12918 is resolved, but in the meanwhile this is an common issue to run into - #11237 - #11751 - #11830 changelog: Add [`lint_groups_priority`] lint r? `@flip1995`
2 parents 455c07b + 6619e8c commit b58b88c

File tree

9 files changed

+289
-1
lines changed

9 files changed

+289
-1
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5277,6 +5277,7 @@ Released 2018-09-13
52775277
[`let_with_type_underscore`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_with_type_underscore
52785278
[`lines_filter_map_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#lines_filter_map_ok
52795279
[`linkedlist`]: https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist
5280+
[`lint_groups_priority`]: https://rust-lang.github.io/rust-clippy/master/index.html#lint_groups_priority
52805281
[`little_endian_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#little_endian_bytes
52815282
[`logic_bug`]: https://rust-lang.github.io/rust-clippy/master/index.html#logic_bug
52825283
[`lossy_float_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#lossy_float_literal
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
use super::LINT_GROUPS_PRIORITY;
2+
use clippy_utils::diagnostics::span_lint_and_then;
3+
use rustc_data_structures::fx::FxHashSet;
4+
use rustc_errors::Applicability;
5+
use rustc_lint::{unerased_lint_store, LateContext};
6+
use rustc_span::{BytePos, Pos, SourceFile, Span, SyntaxContext};
7+
use serde::{Deserialize, Serialize};
8+
use std::collections::BTreeMap;
9+
use std::ops::Range;
10+
use std::path::Path;
11+
use toml::Spanned;
12+
13+
#[derive(Deserialize, Serialize, Debug)]
14+
struct LintConfigTable {
15+
level: String,
16+
priority: Option<i64>,
17+
}
18+
19+
#[derive(Deserialize, Debug)]
20+
#[serde(untagged)]
21+
enum LintConfig {
22+
Level(String),
23+
Table(LintConfigTable),
24+
}
25+
26+
impl LintConfig {
27+
fn level(&self) -> &str {
28+
match self {
29+
LintConfig::Level(level) => level,
30+
LintConfig::Table(table) => &table.level,
31+
}
32+
}
33+
34+
fn priority(&self) -> i64 {
35+
match self {
36+
LintConfig::Level(_) => 0,
37+
LintConfig::Table(table) => table.priority.unwrap_or(0),
38+
}
39+
}
40+
41+
fn is_implicit(&self) -> bool {
42+
if let LintConfig::Table(table) = self {
43+
table.priority.is_none()
44+
} else {
45+
true
46+
}
47+
}
48+
}
49+
50+
type LintTable = BTreeMap<Spanned<String>, Spanned<LintConfig>>;
51+
52+
#[derive(Deserialize, Debug)]
53+
struct Lints {
54+
#[serde(default)]
55+
rust: LintTable,
56+
#[serde(default)]
57+
clippy: LintTable,
58+
}
59+
60+
#[derive(Deserialize, Debug)]
61+
struct CargoToml {
62+
lints: Lints,
63+
}
64+
65+
#[derive(Default, Debug)]
66+
struct LintsAndGroups {
67+
lints: Vec<Spanned<String>>,
68+
groups: Vec<(Spanned<String>, Spanned<LintConfig>)>,
69+
}
70+
71+
fn toml_span(range: Range<usize>, file: &SourceFile) -> Span {
72+
Span::new(
73+
file.start_pos + BytePos::from_usize(range.start),
74+
file.start_pos + BytePos::from_usize(range.end),
75+
SyntaxContext::root(),
76+
None,
77+
)
78+
}
79+
80+
fn check_table(cx: &LateContext<'_>, table: LintTable, groups: &FxHashSet<&str>, file: &SourceFile) {
81+
let mut by_priority = BTreeMap::<_, LintsAndGroups>::new();
82+
for (name, config) in table {
83+
let lints_and_groups = by_priority.entry(config.as_ref().priority()).or_default();
84+
if groups.contains(name.get_ref().as_str()) {
85+
lints_and_groups.groups.push((name, config));
86+
} else {
87+
lints_and_groups.lints.push(name);
88+
}
89+
}
90+
let low_priority = by_priority
91+
.iter()
92+
.find(|(_, lints_and_groups)| !lints_and_groups.lints.is_empty())
93+
.map_or(-1, |(&lowest_lint_priority, _)| lowest_lint_priority - 1);
94+
95+
for (priority, LintsAndGroups { lints, groups }) in by_priority {
96+
let Some(last_lint_alphabetically) = lints.last() else {
97+
continue;
98+
};
99+
100+
for (group, config) in groups {
101+
span_lint_and_then(
102+
cx,
103+
LINT_GROUPS_PRIORITY,
104+
toml_span(group.span(), file),
105+
&format!(
106+
"lint group `{}` has the same priority ({priority}) as a lint",
107+
group.as_ref()
108+
),
109+
|diag| {
110+
let config_span = toml_span(config.span(), file);
111+
if config.as_ref().is_implicit() {
112+
diag.span_label(config_span, "has an implicit priority of 0");
113+
}
114+
// add the label to next lint after this group that has the same priority
115+
let lint = lints
116+
.iter()
117+
.filter(|lint| lint.span().start > group.span().start)
118+
.min_by_key(|lint| lint.span().start)
119+
.unwrap_or(last_lint_alphabetically);
120+
diag.span_label(toml_span(lint.span(), file), "has the same priority as this lint");
121+
diag.note("the order of the lints in the table is ignored by Cargo");
122+
let mut suggestion = String::new();
123+
Serialize::serialize(
124+
&LintConfigTable {
125+
level: config.as_ref().level().into(),
126+
priority: Some(low_priority),
127+
},
128+
toml::ser::ValueSerializer::new(&mut suggestion),
129+
)
130+
.unwrap();
131+
diag.span_suggestion_verbose(
132+
config_span,
133+
format!(
134+
"to have lints override the group set `{}` to a lower priority",
135+
group.as_ref()
136+
),
137+
suggestion,
138+
Applicability::MaybeIncorrect,
139+
);
140+
},
141+
);
142+
}
143+
}
144+
}
145+
146+
pub fn check(cx: &LateContext<'_>) {
147+
if let Ok(file) = cx.tcx.sess.source_map().load_file(Path::new("Cargo.toml"))
148+
&& let Some(src) = file.src.as_deref()
149+
&& let Ok(cargo_toml) = toml::from_str::<CargoToml>(src)
150+
{
151+
let mut rustc_groups = FxHashSet::default();
152+
let mut clippy_groups = FxHashSet::default();
153+
for (group, ..) in unerased_lint_store(cx.tcx.sess).get_lint_groups() {
154+
match group.split_once("::") {
155+
None => {
156+
rustc_groups.insert(group);
157+
},
158+
Some(("clippy", group)) => {
159+
clippy_groups.insert(group);
160+
},
161+
_ => {},
162+
}
163+
}
164+
165+
check_table(cx, cargo_toml.lints.rust, &rustc_groups, &file);
166+
check_table(cx, cargo_toml.lints.clippy, &clippy_groups, &file);
167+
}
168+
}

clippy_lints/src/cargo/mod.rs

+42-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
mod common_metadata;
22
mod feature_name;
3+
mod lint_groups_priority;
34
mod multiple_crate_versions;
45
mod wildcard_dependencies;
56

@@ -165,6 +166,43 @@ declare_clippy_lint! {
165166
"wildcard dependencies being used"
166167
}
167168

169+
declare_clippy_lint! {
170+
/// ### What it does
171+
/// Checks for lint groups with the same priority as lints in the `Cargo.toml`
172+
/// [`[lints]` table](https://doc.rust-lang.org/cargo/reference/manifest.html#the-lints-section).
173+
///
174+
/// This lint will be removed once [cargo#12918](https://github.com/rust-lang/cargo/issues/12918)
175+
/// is resolved.
176+
///
177+
/// ### Why is this bad?
178+
/// The order of lints in the `[lints]` is ignored, to have a lint override a group the
179+
/// `priority` field needs to be used, otherwise the sort order is undefined.
180+
///
181+
/// ### Known problems
182+
/// Does not check lints inherited using `lints.workspace = true`
183+
///
184+
/// ### Example
185+
/// ```toml
186+
/// # Passed as `--allow=clippy::similar_names --warn=clippy::pedantic`
187+
/// # which results in `similar_names` being `warn`
188+
/// [lints.clippy]
189+
/// pedantic = "warn"
190+
/// similar_names = "allow"
191+
/// ```
192+
/// Use instead:
193+
/// ```toml
194+
/// # Passed as `--warn=clippy::pedantic --allow=clippy::similar_names`
195+
/// # which results in `similar_names` being `allow`
196+
/// [lints.clippy]
197+
/// pedantic = { level = "warn", priority = -1 }
198+
/// similar_names = "allow"
199+
/// ```
200+
#[clippy::version = "1.76.0"]
201+
pub LINT_GROUPS_PRIORITY,
202+
correctness,
203+
"a lint group in `Cargo.toml` at the same priority as a lint"
204+
}
205+
168206
pub struct Cargo {
169207
pub allowed_duplicate_crates: FxHashSet<String>,
170208
pub ignore_publish: bool,
@@ -175,7 +213,8 @@ impl_lint_pass!(Cargo => [
175213
REDUNDANT_FEATURE_NAMES,
176214
NEGATIVE_FEATURE_NAMES,
177215
MULTIPLE_CRATE_VERSIONS,
178-
WILDCARD_DEPENDENCIES
216+
WILDCARD_DEPENDENCIES,
217+
LINT_GROUPS_PRIORITY,
179218
]);
180219

181220
impl LateLintPass<'_> for Cargo {
@@ -188,6 +227,8 @@ impl LateLintPass<'_> for Cargo {
188227
];
189228
static WITH_DEPS_LINTS: &[&Lint] = &[MULTIPLE_CRATE_VERSIONS];
190229

230+
lint_groups_priority::check(cx);
231+
191232
if !NO_DEPS_LINTS
192233
.iter()
193234
.all(|&lint| is_lint_allowed(cx, lint, CRATE_HIR_ID))

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
7171
crate::borrow_deref_ref::BORROW_DEREF_REF_INFO,
7272
crate::box_default::BOX_DEFAULT_INFO,
7373
crate::cargo::CARGO_COMMON_METADATA_INFO,
74+
crate::cargo::LINT_GROUPS_PRIORITY_INFO,
7475
crate::cargo::MULTIPLE_CRATE_VERSIONS_INFO,
7576
crate::cargo::NEGATIVE_FEATURE_NAMES_INFO,
7677
crate::cargo::REDUNDANT_FEATURE_NAMES_INFO,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
error: lint group `rust_2018_idioms` has the same priority (0) as a lint
2+
--> Cargo.toml:7:1
3+
|
4+
7 | rust_2018_idioms = "warn"
5+
| ^^^^^^^^^^^^^^^^ ------ has an implicit priority of 0
6+
8 | bare_trait_objects = "allow"
7+
| ------------------ has the same priority as this lint
8+
|
9+
= note: the order of the lints in the table is ignored by Cargo
10+
= note: `#[deny(clippy::lint_groups_priority)]` on by default
11+
help: to have lints override the group set `rust_2018_idioms` to a lower priority
12+
|
13+
7 | rust_2018_idioms = { level = "warn", priority = -1 }
14+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
15+
16+
error: lint group `unused` has the same priority (0) as a lint
17+
--> Cargo.toml:10:1
18+
|
19+
10 | unused = { level = "deny" }
20+
| ^^^^^^ ------------------ has an implicit priority of 0
21+
11 | unused_braces = { level = "allow", priority = 1 }
22+
12 | unused_attributes = { level = "allow" }
23+
| ----------------- has the same priority as this lint
24+
|
25+
= note: the order of the lints in the table is ignored by Cargo
26+
help: to have lints override the group set `unused` to a lower priority
27+
|
28+
10 | unused = { level = "deny", priority = -1 }
29+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
30+
31+
error: lint group `pedantic` has the same priority (-1) as a lint
32+
--> Cargo.toml:19:1
33+
|
34+
19 | pedantic = { level = "warn", priority = -1 }
35+
| ^^^^^^^^
36+
20 | similar_names = { level = "allow", priority = -1 }
37+
| ------------- has the same priority as this lint
38+
|
39+
= note: the order of the lints in the table is ignored by Cargo
40+
help: to have lints override the group set `pedantic` to a lower priority
41+
|
42+
19 | pedantic = { level = "warn", priority = -2 }
43+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
44+
45+
error: could not compile `fail` (lib) due to 3 previous errors
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
[package]
2+
name = "fail"
3+
version = "0.1.0"
4+
publish = false
5+
6+
[lints.rust]
7+
rust_2018_idioms = "warn"
8+
bare_trait_objects = "allow"
9+
10+
unused = { level = "deny" }
11+
unused_braces = { level = "allow", priority = 1 }
12+
unused_attributes = { level = "allow" }
13+
14+
# `warnings` is not a group so the order it is passed does not matter
15+
warnings = "deny"
16+
deprecated = "allow"
17+
18+
[lints.clippy]
19+
pedantic = { level = "warn", priority = -1 }
20+
similar_names = { level = "allow", priority = -1 }
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
[package]
2+
name = "pass"
3+
version = "0.1.0"
4+
publish = false
5+
6+
[lints.clippy]
7+
pedantic = { level = "warn", priority = -1 }
8+
style = { level = "warn", priority = 1 }
9+
similar_names = "allow"
10+
dbg_macro = { level = "warn", priority = 2 }
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+

0 commit comments

Comments
 (0)