Add Clippy like lint groups (#16464)

This PR overhauls how lint groups work within Cargo, aligning them with
how Clippy handles groups[^1], all while simplifying the implementation
(in my opinion). The first commit adds eight lint groups that mirror
[Clippy's
groups](https://github.com/rust-lang/rust-clippy?tab=readme-ov-file#clippy),
while the subsequent commits work towards matching how Clippy handles
groups, as well as improving how lint groups work within the existing
linting system.

The major changes:
- Lints now have a primary group, instead of a set of groups they belong
to
- Lints now inherit their default level from their primary group
  - This vastly improved the code for getting the level for a lint
- Surprisingly, I found this change helped with deciding what group a
lint should belong to

Note: Dynamic groups (i.e., `all` or `warnings`) aren't supported at
this time
Note: There are open questions about which `Level` we should default the
groups to. This PR does not attempt to answer those questions and simply
uses Clippy's default levels for convenience.

[^1]: Clippy uses groups and categories interchangeably

Author: epage

crates/xtask-lint-docs/src/main.rs

@@ -22,7 +22,7 @@ fn main() -> anyhow::Result<()> {
     let mut lint_docs = String::new();
     for lint in cargo::lints::LINTS.iter().sorted_by_key(|lint| lint.name) {
         if lint.docs.is_some() {
-            let sectipn = match lint.default_level {
+            let sectipn = match lint.primary_group.default_level {
                 LintLevel::Allow => &mut allow,
                 LintLevel::Warn => &mut warn,
                 LintLevel::Deny => &mut deny,
@@ -41,6 +41,8 @@ fn main() -> anyhow::Result<()> {
     )?;
     writeln!(buf)?;
 
+    lint_groups(&mut buf)?;
+
     if !allow.is_empty() {
         add_level_section(LintLevel::Allow, &allow, &mut buf)?;
     }
@@ -69,9 +71,51 @@ fn main() -> anyhow::Result<()> {
     Ok(())
 }
 
+fn lint_groups(buf: &mut String) -> anyhow::Result<()> {
+    let (max_name_len, max_desc_len) = cargo::lints::LINT_GROUPS.iter().filter(|g| !g.hidden).fold(
+        (0, 0),
+        |(max_name_len, max_desc_len), group| {
+            // We add 9 to account for the "cargo::" prefix and backticks
+            let name_len = group.name.chars().count() + 9;
+            let desc_len = group.desc.chars().count();
+            (max_name_len.max(name_len), max_desc_len.max(desc_len))
+        },
+    );
+    let default_level_len = "Default level".chars().count();
+    writeln!(buf, "\n")?;
+    writeln!(
+        buf,
+        "| {:<max_name_len$} | {:<max_desc_len$} | Default level |",
+        "Group", "Description",
+    )?;
+    writeln!(
+        buf,
+        "|-{}-|-{}-|-{}-|",
+        "-".repeat(max_name_len),
+        "-".repeat(max_desc_len),
+        "-".repeat(default_level_len)
+    )?;
+    for group in cargo::lints::LINT_GROUPS.iter() {
+        if group.hidden {
+            continue;
+        }
+        let group_name = format!("`cargo::{}`", group.name);
+        writeln!(
+            buf,
+            "| {:<max_name_len$} | {:<max_desc_len$} | {:<default_level_len$} |",
+            group_name,
+            group.desc,
+            group.default_level.to_string(),
+        )?;
+    }
+    writeln!(buf, "\n")?;
+    Ok(())
+}
+
 fn add_lint(lint: &Lint, buf: &mut String) -> std::fmt::Result {
     writeln!(buf, "## `{}`", lint.name)?;
-    writeln!(buf, "Set to `{}` by default", lint.default_level)?;
+    writeln!(buf, "Group: `{}`\n", lint.primary_group.name)?;
+    writeln!(buf, "Level: `{}`", lint.primary_group.default_level)?;
     writeln!(buf, "{}\n", lint.docs.as_ref().unwrap())
 }
 

src/cargo/lints/mod.rs

@@ -10,14 +10,25 @@ use cargo_util_schemas::manifest::TomlToolLints;
 use pathdiff::diff_paths;
 
 use std::borrow::Cow;
+use std::cmp::{Reverse, max_by_key};
 use std::fmt::Display;
 use std::ops::Range;
 use std::path::Path;
 
 pub mod rules;
 pub use rules::LINTS;
 
-const LINT_GROUPS: &[LintGroup] = &[TEST_DUMMY_UNSTABLE];
+pub const LINT_GROUPS: &[LintGroup] = &[
+    COMPLEXITY,
+    CORRECTNESS,
+    NURSERY,
+    PEDANTIC,
+    PERF,
+    RESTRICTION,
+    STYLE,
+    SUSPICIOUS,
+    TEST_DUMMY_UNSTABLE,
+];
 
 /// Scope at which a lint runs: package-level or workspace-level.
 pub enum ManifestFor<'a> {
@@ -141,17 +152,12 @@ fn find_lint_or_group<'a>(
     if let Some(lint) = LINTS.iter().find(|l| l.name == name) {
         Some((
             lint.name,
-            &lint.default_level,
+            &lint.primary_group.default_level,
             &lint.edition_lint_opts,
             &lint.feature_gate,
         ))
     } else if let Some(group) = LINT_GROUPS.iter().find(|g| g.name == name) {
-        Some((
-            group.name,
-            &group.default_level,
-            &group.edition_lint_opts,
-            &group.feature_gate,
-        ))
+        Some((group.name, &group.default_level, &None, &group.feature_gate))
     } else {
         None
     }
@@ -267,25 +273,88 @@ pub struct LintGroup {
     pub name: &'static str,
     pub default_level: LintLevel,
     pub desc: &'static str,
-    pub edition_lint_opts: Option<(Edition, LintLevel)>,
     pub feature_gate: Option<&'static Feature>,
+    pub hidden: bool,
 }
 
+const COMPLEXITY: LintGroup = LintGroup {
+    name: "complexity",
+    desc: "code that does something simple but in a complex way",
+    default_level: LintLevel::Warn,
+    feature_gate: None,
+    hidden: false,
+};
+
+const CORRECTNESS: LintGroup = LintGroup {
+    name: "correctness",
+    desc: "code that is outright wrong or useless",
+    default_level: LintLevel::Deny,
+    feature_gate: None,
+    hidden: false,
+};
+
+const NURSERY: LintGroup = LintGroup {
+    name: "nursery",
+    desc: "new lints that are still under development",
+    default_level: LintLevel::Allow,
+    feature_gate: None,
+    hidden: false,
+};
+
+const PEDANTIC: LintGroup = LintGroup {
+    name: "pedantic",
+    desc: "lints which are rather strict or have occasional false positives",
+    default_level: LintLevel::Allow,
+    feature_gate: None,
+    hidden: false,
+};
+
+const PERF: LintGroup = LintGroup {
+    name: "perf",
+    desc: "code that can be written to run faster",
+    default_level: LintLevel::Warn,
+    feature_gate: None,
+    hidden: false,
+};
+
+const RESTRICTION: LintGroup = LintGroup {
+    name: "restriction",
+    desc: "lints which prevent the use of Cargo features",
+    default_level: LintLevel::Allow,
+    feature_gate: None,
+    hidden: false,
+};
+
+const STYLE: LintGroup = LintGroup {
+    name: "style",
+    desc: "code that should be written in a more idiomatic way",
+    default_level: LintLevel::Warn,
+    feature_gate: None,
+    hidden: false,
+};
+
+const SUSPICIOUS: LintGroup = LintGroup {
+    name: "suspicious",
+    desc: "code that is most likely wrong or useless",
+    default_level: LintLevel::Warn,
+    feature_gate: None,
+    hidden: false,
+};
+
 /// This lint group is only to be used for testing purposes
 const TEST_DUMMY_UNSTABLE: LintGroup = LintGroup {
     name: "test_dummy_unstable",
     desc: "test_dummy_unstable is meant to only be used in tests",
     default_level: LintLevel::Allow,
-    edition_lint_opts: None,
     feature_gate: Some(Feature::test_dummy_unstable()),
+    hidden: true,
 };
 
 #[derive(Copy, Clone, Debug)]
 pub struct Lint {
     pub name: &'static str,
     pub desc: &'static str,
-    pub groups: &'static [LintGroup],
-    pub default_level: LintLevel,
+    pub primary_group: &'static LintGroup,
     pub edition_lint_opts: Option<(Edition, LintLevel)>,
     pub feature_gate: Option<&'static Feature>,
     /// This is a markdown formatted string that will be used when generating
@@ -310,33 +379,28 @@ impl Lint {
             return (LintLevel::Allow, LintLevelReason::Default);
         }
 
-        self.groups
-            .iter()
-            .map(|g| {
-                (
-                    g.name,
-                    level_priority(
-                        g.name,
-                        g.default_level,
-                        g.edition_lint_opts,
-                        pkg_lints,
-                        edition,
-                    ),
-                )
-            })
-            .chain(std::iter::once((
-                self.name,
-                level_priority(
-                    self.name,
-                    self.default_level,
-                    self.edition_lint_opts,
-                    pkg_lints,
-                    edition,
-                ),
-            )))
-            .max_by_key(|(n, (l, _, p))| (l == &LintLevel::Forbid, *p, std::cmp::Reverse(*n)))
-            .map(|(_, (l, r, _))| (l, r))
-            .unwrap()
+        let lint_level_priority = level_priority(
+            self.name,
+            self.primary_group.default_level,
+            self.edition_lint_opts,
+            pkg_lints,
+            edition,
+        );
+
+        let group_level_priority = level_priority(
+            self.primary_group.name,
+            self.primary_group.default_level,
+            None,
+            pkg_lints,
+            edition,
+        );
+
+        let (_, (l, r, _)) = max_by_key(
+            (self.name, lint_level_priority),
+            (self.primary_group.name, group_level_priority),
+            |(n, (l, _, p))| (l == &LintLevel::Forbid, *p, Reverse(*n)),
+        );
+        (l, r)
     }
 
     fn emitted_source(&self, lint_level: LintLevel, reason: LintLevelReason) -> String {

src/cargo/lints/rules/blanket_hint_mostly_unused.rs

@@ -14,14 +14,14 @@ use crate::GlobalContext;
 use crate::core::MaybePackage;
 use crate::lints::Lint;
 use crate::lints::LintLevel;
+use crate::lints::SUSPICIOUS;
 use crate::lints::get_key_value_span;
 use crate::lints::rel_cwd_manifest_path;
 
 pub const LINT: Lint = Lint {
     name: "blanket_hint_mostly_unused",
     desc: "blanket_hint_mostly_unused lint",
-    groups: &[],
-    default_level: LintLevel::Warn,
+    primary_group: &SUSPICIOUS,
     edition_lint_opts: None,
     feature_gate: None,
     docs: Some(

src/cargo/lints/rules/im_a_teapot.rs

@@ -21,8 +21,7 @@ use crate::lints::rel_cwd_manifest_path;
 pub const LINT: Lint = Lint {
     name: "im_a_teapot",
     desc: "`im_a_teapot` is specified",
-    groups: &[TEST_DUMMY_UNSTABLE],
-    default_level: LintLevel::Allow,
+    primary_group: &TEST_DUMMY_UNSTABLE,
     edition_lint_opts: None,
     feature_gate: Some(Feature::test_dummy_unstable()),
     docs: None,

src/cargo/lints/rules/implicit_minimum_version_req.rs

@@ -21,15 +21,15 @@ use crate::lints::Lint;
 use crate::lints::LintLevel;
 use crate::lints::LintLevelReason;
 use crate::lints::ManifestFor;
+use crate::lints::PEDANTIC;
 use crate::lints::get_key_value;
 use crate::lints::rel_cwd_manifest_path;
 use crate::util::OptVersionReq;
 
 pub const LINT: Lint = Lint {
     name: "implicit_minimum_version_req",
     desc: "dependency version requirement without an explicit minimum version",
-    groups: &[],
-    default_level: LintLevel::Allow,
+    primary_group: &PEDANTIC,
     edition_lint_opts: None,
     feature_gate: None,
     docs: Some(

src/cargo/lints/rules/unknown_lints.rs

@@ -12,13 +12,13 @@ use crate::lints::LINTS;
 use crate::lints::Lint;
 use crate::lints::LintLevel;
 use crate::lints::ManifestFor;
+use crate::lints::SUSPICIOUS;
 use crate::lints::get_key_value_span;
 
 pub const LINT: Lint = Lint {
     name: "unknown_lints",
     desc: "unknown lint",
-    groups: &[],
-    default_level: LintLevel::Warn,
+    primary_group: &SUSPICIOUS,
     edition_lint_opts: None,
     feature_gate: None,
     docs: Some(

src/doc/src/reference/lints.md

@@ -2,6 +2,20 @@
 
 Note: [Cargo's linting system is unstable](unstable.md#lintscargo) and can only be used on nightly toolchains
 
+
+
+| Group                | Description                                                      | Default level |
+|----------------------|------------------------------------------------------------------|---------------|
+| `cargo::complexity`  | code that does something simple but in a complex way             | warn          |
+| `cargo::correctness` | code that is outright wrong or useless                           | deny          |
+| `cargo::nursery`     | new lints that are still under development                       | allow         |
+| `cargo::pedantic`    | lints which are rather strict or have occasional false positives | allow         |
+| `cargo::perf`        | code that can be written to run faster                           | warn          |
+| `cargo::restriction` | lints which prevent the use of Cargo features                    | allow         |
+| `cargo::style`       | code that should be written in a more idiomatic way              | warn          |
+| `cargo::suspicious`  | code that is most likely wrong or useless                        | warn          |
+
+
 ## Allowed-by-default
 
 These lints are all set to the 'allow' level by default.
@@ -14,7 +28,9 @@ These lints are all set to the 'warn' level by default.
 - [`unknown_lints`](#unknown_lints)
 
 ## `blanket_hint_mostly_unused`
-Set to `warn` by default
+Group: `suspicious`
+
+Level: `warn`
 
 ### What it does
 Checks if `hint-mostly-unused` being applied to all dependencies.
@@ -42,7 +58,9 @@ hint-mostly-unused = true
 
 
 ## `implicit_minimum_version_req`
-Set to `allow` by default
+Group: `pedantic`
+
+Level: `allow`
 
 ### What it does
 
@@ -91,7 +109,9 @@ serde = "1.0.219"
 
 
 ## `unknown_lints`
-Set to `warn` by default
+Group: `suspicious`
+
+Level: `warn`
 
 ### What it does
 Checks for unknown lints in the `[lints.cargo]` table