fix(lints): Mark the im_a_teapot lint as unstable

Muscraft · Muscraft · commit 6bcfd66a9694 · 2024-04-30T18:24:49.000-06:00
diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs
@@ -406,6 +406,10 @@ macro_rules! features {
             fn is_enabled(&self, features: &Features) -> bool {
                 (self.get)(features)
             }
+
+            pub(crate) fn name(&self) -> &str {
+                self.name
+            }
         }
 
         impl Features {
diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs
@@ -24,7 +24,9 @@ use crate::sources::{PathSource, CRATES_IO_INDEX, CRATES_IO_REGISTRY};
 use crate::util::edit_distance;
 use crate::util::errors::{CargoResult, ManifestError};
 use crate::util::interning::InternedString;
-use crate::util::lints::{check_im_a_teapot, check_implicit_features, unused_dependencies};
+use crate::util::lints::{
+    analyze_cargo_lints_table, check_im_a_teapot, check_implicit_features, unused_dependencies,
+};
 use crate::util::toml::{read_manifest, InheritableFields};
 use crate::util::{
     context::CargoResolverConfig, context::CargoResolverPrecedence, context::ConfigRelativePath,
@@ -1227,6 +1229,26 @@ impl<'gctx> Workspace<'gctx> {
             .is_some_and(|l| l.workspace)
             .then(|| ws_cargo_lints);
 
+        let ws_contents = match self.root_maybe() {
+            MaybePackage::Package(pkg) => pkg.manifest().contents(),
+            MaybePackage::Virtual(v) => v.contents(),
+        };
+
+        let ws_document = match self.root_maybe() {
+            MaybePackage::Package(pkg) => pkg.manifest().document(),
+            MaybePackage::Virtual(v) => v.document(),
+        };
+
+        analyze_cargo_lints_table(
+            pkg,
+            &path,
+            &normalized_lints,
+            ws_cargo_lints,
+            ws_contents,
+            ws_document,
+            self.root_manifest(),
+            self.gctx,
+        )?;
         check_im_a_teapot(
             pkg,
             &path,
diff --git a/src/cargo/util/lints.rs b/src/cargo/util/lints.rs
@@ -12,6 +12,157 @@ use std::ops::Range;
 use std::path::Path;
 use toml_edit::ImDocument;
 
+const LINTS: &[Lint] = &[IM_A_TEAPOT, IMPLICIT_FEATURES, UNUSED_OPTIONAL_DEPENDENCY];
+
+pub fn analyze_cargo_lints_table(
+    pkg: &Package,
+    path: &Path,
+    pkg_lints: &TomlToolLints,
+    ws_lints: Option<&TomlToolLints>,
+    ws_contents: &str,
+    ws_document: &ImDocument<String>,
+    ws_path: &Path,
+    gctx: &GlobalContext,
+) -> CargoResult<()> {
+    let mut error_count = 0;
+    let manifest = pkg.manifest();
+    let manifest_path = rel_cwd_manifest_path(path, gctx);
+    let ws_path = rel_cwd_manifest_path(ws_path, gctx);
+
+    for lint_name in pkg_lints
+        .keys()
+        .chain(ws_lints.map(|l| l.keys()).unwrap_or_default())
+    {
+        if let Some(lint) = LINTS.iter().find(|l| l.name == lint_name) {
+            let (_, reason) = lint.level(pkg_lints, ws_lints, manifest.edition());
+
+            // Only run analysis on user-specified lints
+            if !reason.is_user_specified() {
+                continue;
+            }
+
+            // Only run this on lints that are gated by a feature
+            if let Some(feature_gate) = lint.feature_gate {
+                verify_feature_enabled(
+                    lint.name,
+                    feature_gate,
+                    reason,
+                    manifest,
+                    &manifest_path,
+                    ws_contents,
+                    ws_document,
+                    &ws_path,
+                    &mut error_count,
+                    gctx,
+                )?;
+            }
+        }
+    }
+    if error_count > 0 {
+        Err(anyhow::anyhow!(
+            "encountered {error_count} errors(s) while verifying lints",
+        ))
+    } else {
+        Ok(())
+    }
+}
+
+fn verify_feature_enabled(
+    lint_name: &str,
+    feature_gate: &Feature,
+    reason: LintLevelReason,
+    manifest: &Manifest,
+    manifest_path: &str,
+    ws_contents: &str,
+    ws_document: &ImDocument<String>,
+    ws_path: &str,
+    error_count: &mut usize,
+    gctx: &GlobalContext,
+) -> CargoResult<()> {
+    if !manifest.unstable_features().is_enabled(feature_gate) {
+        let dash_name = lint_name.replace("_", "-");
+        let dash_feature_name = feature_gate.name().replace("_", "-");
+        let title = format!("use of unstable lint `{}`", dash_name);
+        let label = format!(
+            "this is behind `{}`, which is not enabled",
+            dash_feature_name
+        );
+        let second_title = format!("`cargo::{}` was inherited", dash_name);
+        let help = format!(
+            "consider adding `cargo-features = [\"{}\"]` to the top of the manifest",
+            dash_feature_name
+        );
+        let message = match reason {
+            LintLevelReason::Package => {
+                let span = get_span(
+                    manifest.document(),
+                    &["lints", "cargo", dash_name.as_str()],
+                    false,
+                )
+                .or(get_span(
+                    manifest.document(),
+                    &["lints", "cargo", lint_name],
+                    false,
+                ))
+                .unwrap();
+
+                Level::Error
+                    .title(&title)
+                    .snippet(
+                        Snippet::source(manifest.contents())
+                            .origin(&manifest_path)
+                            .annotation(Level::Error.span(span).label(&label))
+                            .fold(true),
+                    )
+                    .footer(Level::Help.title(&help))
+            }
+            LintLevelReason::Workspace => {
+                let lint_span = get_span(
+                    ws_document,
+                    &["workspace", "lints", "cargo", dash_name.as_str()],
+                    false,
+                )
+                .or(get_span(
+                    ws_document,
+                    &["workspace", "lints", "cargo", lint_name],
+                    false,
+                ))
+                .unwrap();
+                let inherit_span_key =
+                    get_span(manifest.document(), &["lints", "workspace"], false).unwrap();
+                let inherit_span_value =
+                    get_span(manifest.document(), &["lints", "workspace"], true).unwrap();
+
+                Level::Error
+                    .title(&title)
+                    .snippet(
+                        Snippet::source(ws_contents)
+                            .origin(&ws_path)
+                            .annotation(Level::Error.span(lint_span).label(&label))
+                            .fold(true),
+                    )
+                    .footer(
+                        Level::Note.title(&second_title).snippet(
+                            Snippet::source(manifest.contents())
+                                .origin(&manifest_path)
+                                .annotation(
+                                    Level::Note
+                                        .span(inherit_span_key.start..inherit_span_value.end),
+                                )
+                                .fold(true),
+                        ),
+                    )
+                    .footer(Level::Help.title(&help))
+            }
+            _ => unreachable!("LintLevelReason should be one that is user specified"),
+        };
+
+        *error_count += 1;
+        gctx.shell().print_message(message)?;
+    }
+    Ok(())
+}
+
 fn get_span(document: &ImDocument<String>, path: &[&str], get_value: bool) -> Option<Range<usize>> {
     let mut table = document.as_item().as_table_like()?;
     let mut iter = path.into_iter().peekable();
@@ -122,12 +273,6 @@ impl Lint {
             .map(|(_, (l, r, _))| (l, r))
             .unwrap()
     }
-
-    /// Returns true if the lint is feature gated and the feature is not enabled
-    fn is_feature_gated(&self, manifest: &Manifest) -> bool {
-        self.feature_gate
-            .map_or(false, |f| !manifest.unstable_features().is_enabled(f))
-    }
 }
 
 #[derive(Copy, Clone, Debug, PartialEq)]
@@ -190,6 +335,17 @@ impl Display for LintLevelReason {
     }
 }
 
+impl LintLevelReason {
+    fn is_user_specified(&self) -> bool {
+        match self {
+            LintLevelReason::Default => false,
+            LintLevelReason::Edition(_) => false,
+            LintLevelReason::Package => true,
+            LintLevelReason::Workspace => true,
+        }
+    }
+}
+
 fn level_priority(
     name: &str,
     default_level: LintLevel,
@@ -249,10 +405,6 @@ pub fn check_im_a_teapot(
     let manifest = pkg.manifest();
     let (lint_level, reason) = IM_A_TEAPOT.level(pkg_lints, ws_lints, manifest.edition());
 
-    if IM_A_TEAPOT.is_feature_gated(manifest) {
-        return Ok(());
-    }
-
     if lint_level == LintLevel::Allow {
         return Ok(());
     }
diff --git a/tests/testsuite/lints_table.rs b/tests/testsuite/lints_table.rs
@@ -1100,10 +1100,71 @@ im-a-teapot = "warn"
 
     p.cargo("check -Zcargo-lints")
         .masquerade_as_nightly_cargo(&["cargo-lints"])
+        .with_status(101)
         .with_stderr(
             "\
-[CHECKING] foo v0.0.1 ([CWD])
-[FINISHED] [..]
+error: use of unstable lint `im-a-teapot`
+ --> Cargo.toml:9:1
+  |
+9 | im-a-teapot = \"warn\"
+  | ^^^^^^^^^^^ this is behind `test-dummy-unstable`, which is not enabled
+  |
+  = help: consider adding `cargo-features = [\"test-dummy-unstable\"]` to the top of the manifest
+error: encountered 1 errors(s) while verifying lints
+",
+        )
+        .run();
+}
+
+#[cargo_test]
+fn check_feature_gated_workspace() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+[workspace]
+members = ["foo"]
+
+[workspace.lints.cargo]
+im-a-teapot = { level = "warn", priority = 10 }
+test-dummy-unstable = { level = "forbid", priority = -1 }
+            "#,
+        )
+        .file(
+            "foo/Cargo.toml",
+            r#"
+[package]
+name = "foo"
+version = "0.0.1"
+edition = "2015"
+authors = []
+
+[lints]
+workspace = true
+            "#,
+        )
+        .file("foo/src/lib.rs", "")
+        .build();
+
+    p.cargo("check -Zcargo-lints")
+        .masquerade_as_nightly_cargo(&["cargo-lints"])
+        .with_status(101)
+        .with_stderr(
+            "\
+error: use of unstable lint `im-a-teapot`
+ --> Cargo.toml:6:1
+  |
+6 | im-a-teapot = { level = \"warn\", priority = 10 }
+  | ^^^^^^^^^^^ this is behind `test-dummy-unstable`, which is not enabled
+  |
+note: `cargo::im-a-teapot` was inherited
+ --> foo/Cargo.toml:9:1
+  |
+9 | workspace = true
+  | ----------------
+  |
+  = help: consider adding `cargo-features = [\"test-dummy-unstable\"]` to the top of the manifest
+error: encountered 1 errors(s) while verifying lints
 ",
         )
         .run();

Original file line number	Diff line number	Diff line change
`@@ -406,6 +406,10 @@ macro_rules! features {`
`406`	`406`	`fn is_enabled(&self, features: &Features) -> bool {`
`407`	`407`	`(self.get)(features)`
`408`	`408`	`}`
	`409`	`+`
	`410`	`+ pub(crate) fn name(&self) -> &str {`
	`411`	`+ self.name`
	`412`	`+ }`
`409`	`413`	`}`
`410`	`414`
`411`	`415`	`impl Features {`