lint_groups_priority: ignore lints & groups at the same level

Author: Alexendoo

clippy_lints/src/cargo/lint_groups_priority.rs

@@ -71,12 +71,6 @@ struct CargoToml {
     workspace: Workspace,
 }
 
-#[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),
@@ -86,69 +80,78 @@ fn toml_span(range: Range<usize>, file: &SourceFile) -> Span {
     )
 }
 
-fn check_table(cx: &LateContext<'_>, table: LintTable, groups: &FxHashSet<&str>, file: &SourceFile) {
-    let mut by_priority = BTreeMap::<_, LintsAndGroups>::new();
+fn check_table(cx: &LateContext<'_>, table: LintTable, known_groups: &FxHashSet<&str>, file: &SourceFile) {
+    let mut lints = Vec::new();
+    let mut groups = Vec::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));
+        if name.get_ref() == "warnings" {
+            continue;
+        }
+
+        if known_groups.contains(name.get_ref().as_str()) {
+            groups.push((name, config));
         } else {
-            lints_and_groups.lints.push(name);
+            lints.push((name, config.into_inner()));
         }
     }
-    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 {
+    for (group, group_config) in groups {
+        let priority = group_config.get_ref().priority();
+        let level = group_config.get_ref().level();
+        let mut conflicts = lints
+            .iter()
+            .filter(|(_, lint_config)| lint_config.priority() == priority && lint_config.level() != level);
+
+        let Some(last_conflict) = conflicts.next_back() 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,
-                    );
-                },
-            );
-        }
+        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(group_config.span(), file);
+
+                if group_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, _) = conflicts
+                    .find(|(lint, _)| lint.span().start > group.span().start)
+                    .unwrap_or(last_conflict);
+                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();
+                let low_priority = lints
+                    .iter()
+                    .map(|(_, config)| config.priority().saturating_sub(1))
+                    .min()
+                    .unwrap_or(-1);
+                Serialize::serialize(
+                    &LintConfigTable {
+                        level: 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,
+                );
+            },
+        );
     }
 }
 

tests/ui-cargo/lint_groups_priority/fail/Cargo.stderr

@@ -29,45 +29,45 @@ help: to have lints override the group set `unused` to a lower priority
    |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 error: lint group `pedantic` has the same priority (-1) as a lint
-  --> Cargo.toml:19:1
+  --> Cargo.toml:15:1
    |
-19 | pedantic = { level = "warn", priority = -1 }
+15 | pedantic = { level = "warn", priority = -1 }
    | ^^^^^^^^
-20 | similar_names = { level = "allow", priority = -1 }
+16 | 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 }
+15 | pedantic = { level = "warn", priority = -2 }
    |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 error: lint group `rust_2018_idioms` has the same priority (0) as a lint
-  --> Cargo.toml:23:1
+  --> Cargo.toml:19:1
    |
-23 | rust_2018_idioms = "warn"
+19 | rust_2018_idioms = "warn"
    | ^^^^^^^^^^^^^^^^   ------ has an implicit priority of 0
-24 | bare_trait_objects = "allow"
+20 | bare_trait_objects = "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 `rust_2018_idioms` to a lower priority
    |
-23 | rust_2018_idioms = { level = "warn", priority = -1 }
+19 | rust_2018_idioms = { level = "warn", priority = -1 }
    |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 error: lint group `pedantic` has the same priority (0) as a lint
-  --> Cargo.toml:27:1
+  --> Cargo.toml:23:1
    |
-27 | pedantic = "warn"
+23 | pedantic = "warn"
    | ^^^^^^^^   ------ has an implicit priority of 0
-28 | similar_names = "allow"
+24 | similar_names = "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 `pedantic` to a lower priority
    |
-27 | pedantic = { level = "warn", priority = -1 }
+23 | pedantic = { level = "warn", priority = -1 }
    |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 error: could not compile `fail` (lib) due to 5 previous errors

tests/ui-cargo/lint_groups_priority/fail/Cargo.toml

@@ -11,10 +11,6 @@ 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 }

tests/ui-cargo/lint_groups_priority/pass/Cargo.toml

@@ -3,6 +3,13 @@ name = "pass"
 version = "0.1.0"
 publish = false
 
+[lints.rust]
+# Warnings does not conflict with any group or lint
+warnings = "deny"
+# Groups & lints at the same level do not conflict
+rust_2018_idioms = "warn"
+unsafe_code = "warn"
+
 [lints.clippy]
 pedantic = { level = "warn", priority = -1 }
 style = { level = "warn", priority = 1 }