Add lint_groups_priority lint

Warns when a lint group in Cargo.toml's `[lints]` section shares the
same priority as a lint
This commit is contained in:
Alex Macleod 2024-01-31 18:33:41 +00:00
parent 455c07b7cc
commit 6619e8c27d
9 changed files with 289 additions and 1 deletions

View File

@ -5277,6 +5277,7 @@ Released 2018-09-13
[`let_with_type_underscore`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_with_type_underscore [`let_with_type_underscore`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_with_type_underscore
[`lines_filter_map_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#lines_filter_map_ok [`lines_filter_map_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#lines_filter_map_ok
[`linkedlist`]: https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist [`linkedlist`]: https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist
[`lint_groups_priority`]: https://rust-lang.github.io/rust-clippy/master/index.html#lint_groups_priority
[`little_endian_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#little_endian_bytes [`little_endian_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#little_endian_bytes
[`logic_bug`]: https://rust-lang.github.io/rust-clippy/master/index.html#logic_bug [`logic_bug`]: https://rust-lang.github.io/rust-clippy/master/index.html#logic_bug
[`lossy_float_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#lossy_float_literal [`lossy_float_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#lossy_float_literal

View File

@ -0,0 +1,168 @@
use super::LINT_GROUPS_PRIORITY;
use clippy_utils::diagnostics::span_lint_and_then;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
use rustc_lint::{unerased_lint_store, LateContext};
use rustc_span::{BytePos, Pos, SourceFile, Span, SyntaxContext};
use serde::{Deserialize, Serialize};
use std::collections::BTreeMap;
use std::ops::Range;
use std::path::Path;
use toml::Spanned;
#[derive(Deserialize, Serialize, Debug)]
struct LintConfigTable {
level: String,
priority: Option<i64>,
}
#[derive(Deserialize, Debug)]
#[serde(untagged)]
enum LintConfig {
Level(String),
Table(LintConfigTable),
}
impl LintConfig {
fn level(&self) -> &str {
match self {
LintConfig::Level(level) => level,
LintConfig::Table(table) => &table.level,
}
}
fn priority(&self) -> i64 {
match self {
LintConfig::Level(_) => 0,
LintConfig::Table(table) => table.priority.unwrap_or(0),
}
}
fn is_implicit(&self) -> bool {
if let LintConfig::Table(table) = self {
table.priority.is_none()
} else {
true
}
}
}
type LintTable = BTreeMap<Spanned<String>, Spanned<LintConfig>>;
#[derive(Deserialize, Debug)]
struct Lints {
#[serde(default)]
rust: LintTable,
#[serde(default)]
clippy: LintTable,
}
#[derive(Deserialize, Debug)]
struct CargoToml {
lints: Lints,
}
#[derive(Default, Debug)]
struct LintsAndGroups {
lints: Vec<Spanned<String>>,
groups: Vec<(Spanned<String>, Spanned<LintConfig>)>,
}
fn toml_span(range: Range<usize>, file: &SourceFile) -> Span {
Span::new(
file.start_pos + BytePos::from_usize(range.start),
file.start_pos + BytePos::from_usize(range.end),
SyntaxContext::root(),
None,
)
}
fn check_table(cx: &LateContext<'_>, table: LintTable, groups: &FxHashSet<&str>, file: &SourceFile) {
let mut by_priority = BTreeMap::<_, LintsAndGroups>::new();
for (name, config) in table {
let lints_and_groups = by_priority.entry(config.as_ref().priority()).or_default();
if groups.contains(name.get_ref().as_str()) {
lints_and_groups.groups.push((name, config));
} else {
lints_and_groups.lints.push(name);
}
}
let low_priority = by_priority
.iter()
.find(|(_, lints_and_groups)| !lints_and_groups.lints.is_empty())
.map_or(-1, |(&lowest_lint_priority, _)| lowest_lint_priority - 1);
for (priority, LintsAndGroups { lints, groups }) in by_priority {
let Some(last_lint_alphabetically) = lints.last() else {
continue;
};
for (group, config) in groups {
span_lint_and_then(
cx,
LINT_GROUPS_PRIORITY,
toml_span(group.span(), file),
&format!(
"lint group `{}` has the same priority ({priority}) as a lint",
group.as_ref()
),
|diag| {
let config_span = toml_span(config.span(), file);
if config.as_ref().is_implicit() {
diag.span_label(config_span, "has an implicit priority of 0");
}
// add the label to next lint after this group that has the same priority
let lint = lints
.iter()
.filter(|lint| lint.span().start > group.span().start)
.min_by_key(|lint| lint.span().start)
.unwrap_or(last_lint_alphabetically);
diag.span_label(toml_span(lint.span(), file), "has the same priority as this lint");
diag.note("the order of the lints in the table is ignored by Cargo");
let mut suggestion = String::new();
Serialize::serialize(
&LintConfigTable {
level: config.as_ref().level().into(),
priority: Some(low_priority),
},
toml::ser::ValueSerializer::new(&mut suggestion),
)
.unwrap();
diag.span_suggestion_verbose(
config_span,
format!(
"to have lints override the group set `{}` to a lower priority",
group.as_ref()
),
suggestion,
Applicability::MaybeIncorrect,
);
},
);
}
}
}
pub fn check(cx: &LateContext<'_>) {
if let Ok(file) = cx.tcx.sess.source_map().load_file(Path::new("Cargo.toml"))
&& let Some(src) = file.src.as_deref()
&& let Ok(cargo_toml) = toml::from_str::<CargoToml>(src)
{
let mut rustc_groups = FxHashSet::default();
let mut clippy_groups = FxHashSet::default();
for (group, ..) in unerased_lint_store(cx.tcx.sess).get_lint_groups() {
match group.split_once("::") {
None => {
rustc_groups.insert(group);
},
Some(("clippy", group)) => {
clippy_groups.insert(group);
},
_ => {},
}
}
check_table(cx, cargo_toml.lints.rust, &rustc_groups, &file);
check_table(cx, cargo_toml.lints.clippy, &clippy_groups, &file);
}
}

View File

@ -1,5 +1,6 @@
mod common_metadata; mod common_metadata;
mod feature_name; mod feature_name;
mod lint_groups_priority;
mod multiple_crate_versions; mod multiple_crate_versions;
mod wildcard_dependencies; mod wildcard_dependencies;
@ -165,6 +166,43 @@
"wildcard dependencies being used" "wildcard dependencies being used"
} }
declare_clippy_lint! {
/// ### What it does
/// Checks for lint groups with the same priority as lints in the `Cargo.toml`
/// [`[lints]` table](https://doc.rust-lang.org/cargo/reference/manifest.html#the-lints-section).
///
/// This lint will be removed once [cargo#12918](https://github.com/rust-lang/cargo/issues/12918)
/// is resolved.
///
/// ### Why is this bad?
/// The order of lints in the `[lints]` is ignored, to have a lint override a group the
/// `priority` field needs to be used, otherwise the sort order is undefined.
///
/// ### Known problems
/// Does not check lints inherited using `lints.workspace = true`
///
/// ### Example
/// ```toml
/// # Passed as `--allow=clippy::similar_names --warn=clippy::pedantic`
/// # which results in `similar_names` being `warn`
/// [lints.clippy]
/// pedantic = "warn"
/// similar_names = "allow"
/// ```
/// Use instead:
/// ```toml
/// # Passed as `--warn=clippy::pedantic --allow=clippy::similar_names`
/// # which results in `similar_names` being `allow`
/// [lints.clippy]
/// pedantic = { level = "warn", priority = -1 }
/// similar_names = "allow"
/// ```
#[clippy::version = "1.76.0"]
pub LINT_GROUPS_PRIORITY,
correctness,
"a lint group in `Cargo.toml` at the same priority as a lint"
}
pub struct Cargo { pub struct Cargo {
pub allowed_duplicate_crates: FxHashSet<String>, pub allowed_duplicate_crates: FxHashSet<String>,
pub ignore_publish: bool, pub ignore_publish: bool,
@ -175,7 +213,8 @@ pub struct Cargo {
REDUNDANT_FEATURE_NAMES, REDUNDANT_FEATURE_NAMES,
NEGATIVE_FEATURE_NAMES, NEGATIVE_FEATURE_NAMES,
MULTIPLE_CRATE_VERSIONS, MULTIPLE_CRATE_VERSIONS,
WILDCARD_DEPENDENCIES WILDCARD_DEPENDENCIES,
LINT_GROUPS_PRIORITY,
]); ]);
impl LateLintPass<'_> for Cargo { impl LateLintPass<'_> for Cargo {
@ -188,6 +227,8 @@ fn check_crate(&mut self, cx: &LateContext<'_>) {
]; ];
static WITH_DEPS_LINTS: &[&Lint] = &[MULTIPLE_CRATE_VERSIONS]; static WITH_DEPS_LINTS: &[&Lint] = &[MULTIPLE_CRATE_VERSIONS];
lint_groups_priority::check(cx);
if !NO_DEPS_LINTS if !NO_DEPS_LINTS
.iter() .iter()
.all(|&lint| is_lint_allowed(cx, lint, CRATE_HIR_ID)) .all(|&lint| is_lint_allowed(cx, lint, CRATE_HIR_ID))

View File

@ -71,6 +71,7 @@
crate::borrow_deref_ref::BORROW_DEREF_REF_INFO, crate::borrow_deref_ref::BORROW_DEREF_REF_INFO,
crate::box_default::BOX_DEFAULT_INFO, crate::box_default::BOX_DEFAULT_INFO,
crate::cargo::CARGO_COMMON_METADATA_INFO, crate::cargo::CARGO_COMMON_METADATA_INFO,
crate::cargo::LINT_GROUPS_PRIORITY_INFO,
crate::cargo::MULTIPLE_CRATE_VERSIONS_INFO, crate::cargo::MULTIPLE_CRATE_VERSIONS_INFO,
crate::cargo::NEGATIVE_FEATURE_NAMES_INFO, crate::cargo::NEGATIVE_FEATURE_NAMES_INFO,
crate::cargo::REDUNDANT_FEATURE_NAMES_INFO, crate::cargo::REDUNDANT_FEATURE_NAMES_INFO,

View File

@ -0,0 +1,45 @@
error: lint group `rust_2018_idioms` has the same priority (0) as a lint
--> Cargo.toml:7:1
|
7 | rust_2018_idioms = "warn"
| ^^^^^^^^^^^^^^^^ ------ has an implicit priority of 0
8 | bare_trait_objects = "allow"
| ------------------ has the same priority as this lint
|
= note: the order of the lints in the table is ignored by Cargo
= note: `#[deny(clippy::lint_groups_priority)]` on by default
help: to have lints override the group set `rust_2018_idioms` to a lower priority
|
7 | rust_2018_idioms = { level = "warn", priority = -1 }
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
error: lint group `unused` has the same priority (0) as a lint
--> Cargo.toml:10:1
|
10 | unused = { level = "deny" }
| ^^^^^^ ------------------ has an implicit priority of 0
11 | unused_braces = { level = "allow", priority = 1 }
12 | unused_attributes = { level = "allow" }
| ----------------- has the same priority as this lint
|
= note: the order of the lints in the table is ignored by Cargo
help: to have lints override the group set `unused` to a lower priority
|
10 | unused = { level = "deny", priority = -1 }
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
error: lint group `pedantic` has the same priority (-1) as a lint
--> Cargo.toml:19:1
|
19 | pedantic = { level = "warn", priority = -1 }
| ^^^^^^^^
20 | similar_names = { level = "allow", priority = -1 }
| ------------- has the same priority as this lint
|
= note: the order of the lints in the table is ignored by Cargo
help: to have lints override the group set `pedantic` to a lower priority
|
19 | pedantic = { level = "warn", priority = -2 }
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
error: could not compile `fail` (lib) due to 3 previous errors

View File

@ -0,0 +1,20 @@
[package]
name = "fail"
version = "0.1.0"
publish = false
[lints.rust]
rust_2018_idioms = "warn"
bare_trait_objects = "allow"
unused = { level = "deny" }
unused_braces = { level = "allow", priority = 1 }
unused_attributes = { level = "allow" }
# `warnings` is not a group so the order it is passed does not matter
warnings = "deny"
deprecated = "allow"
[lints.clippy]
pedantic = { level = "warn", priority = -1 }
similar_names = { level = "allow", priority = -1 }

View File

@ -0,0 +1 @@

View File

@ -0,0 +1,10 @@
[package]
name = "pass"
version = "0.1.0"
publish = false
[lints.clippy]
pedantic = { level = "warn", priority = -1 }
style = { level = "warn", priority = 1 }
similar_names = "allow"
dbg_macro = { level = "warn", priority = 2 }

View File

@ -0,0 +1 @@