Auto merge of rust-lang#12827 - Alexendoo:lint-groups-priority, r=Jarcho

`lint_groups_priority`: ignore lints & groups at the same level

Fixes rust-lang#12270

changelog: none

Author: bors

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,27 +80,28 @@ 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 {
-            continue;
-        };
-
-        for (group, config) in groups {
+    for (group, group_config) in groups {
+        let priority = group_config.get_ref().priority();
+        let level = group_config.get_ref().level();
+        if let Some((conflict, _)) = lints
+            .iter()
+            .rfind(|(_, lint_config)| lint_config.priority() == priority && lint_config.level() != level)
+        {
             span_lint_and_then(
                 cx,
                 LINT_GROUPS_PRIORITY,
@@ -116,22 +111,23 @@ fn check_table(cx: &LateContext<'_>, table: LintTable, groups: &FxHashSet<&str>,
                     group.as_ref()
                 ),
                 |diag| {
-                    let config_span = toml_span(config.span(), file);
-                    if config.as_ref().is_implicit() {
+                    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 = 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.span_label(toml_span(conflict.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: config.as_ref().level().into(),
+                            level: level.into(),
                             priority: Some(low_priority),
                         },
                         toml::ser::ValueSerializer::new(&mut suggestion),

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

@@ -1,17 +1,18 @@
 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
+  --> Cargo.toml:7:1
+   |
+7  | rust_2018_idioms = "warn"
+   | ^^^^^^^^^^^^^^^^   ------ has an implicit priority of 0
+...
+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
+   = 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 }
-  |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+   |
+7  | rust_2018_idioms = { level = "warn", priority = -1 }
+   |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 error: lint group `unused` has the same priority (0) as a lint
   --> Cargo.toml:10:1
@@ -29,45 +30,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 }