feat: Add lint for global use of hint-mostly-unused (#15995)

This PR adds a lint that checks for global use of `hint-mostly-unused`.
This also adds a "workspace lint pass", since we only want to lint on
the workspace's `[profile]`, as all other `[profile]` are ignored. This
new pass uses `[workspace.lints.cargo]` for configuration if
`[workspace]` exists, and `[lints]` if it doesn't[^1].

Note: This lint will get emitted if `-Zprofile-hint-mostly-unused` is
set, regardless of whether `-Zcargo-lints` is set.

CC @joshtriplett

[^1]: We can change the behavior in the future if it is not correct

Author: epage

src/cargo/core/workspace.rs

@@ -25,7 +25,9 @@ use crate::util::context::FeatureUnification;
 use crate::util::edit_distance;
 use crate::util::errors::{CargoResult, ManifestError};
 use crate::util::interning::InternedString;
-use crate::util::lints::{analyze_cargo_lints_table, check_im_a_teapot};
+use crate::util::lints::{
+    analyze_cargo_lints_table, blanket_hint_mostly_unused, check_im_a_teapot,
+};
 use crate::util::toml::{InheritableFields, read_manifest};
 use crate::util::{
     Filesystem, GlobalContext, IntoUrl, context::CargoResolverConfig, context::ConfigRelativePath,
@@ -409,10 +411,7 @@ impl<'gctx> Workspace<'gctx> {
     }
 
     pub fn profiles(&self) -> Option<&TomlProfiles> {
-        match self.root_maybe() {
-            MaybePackage::Package(p) => p.manifest().profiles(),
-            MaybePackage::Virtual(vm) => vm.profiles(),
-        }
+        self.root_maybe().profiles()
     }
 
     /// Returns the root path of this workspace.
@@ -907,10 +906,7 @@ impl<'gctx> Workspace<'gctx> {
 
     /// Returns the unstable nightly-only features enabled via `cargo-features` in the manifest.
     pub fn unstable_features(&self) -> &Features {
-        match self.root_maybe() {
-            MaybePackage::Package(p) => p.manifest().unstable_features(),
-            MaybePackage::Virtual(vm) => vm.unstable_features(),
-        }
+        self.root_maybe().unstable_features()
     }
 
     pub fn resolve_behavior(&self) -> ResolveBehavior {
@@ -1206,14 +1202,17 @@ impl<'gctx> Workspace<'gctx> {
 
     pub fn emit_warnings(&self) -> CargoResult<()> {
         let mut first_emitted_error = None;
+
+        if let Err(e) = self.emit_ws_lints() {
+            first_emitted_error = Some(e);
+        }
+
         for (path, maybe_pkg) in &self.packages.packages {
             if let MaybePackage::Package(pkg) = maybe_pkg {
-                if self.gctx.cli_unstable().cargo_lints {
-                    if let Err(e) = self.emit_lints(pkg, &path)
-                        && first_emitted_error.is_none()
-                    {
-                        first_emitted_error = Some(e);
-                    }
+                if let Err(e) = self.emit_pkg_lints(pkg, &path)
+                    && first_emitted_error.is_none()
+                {
+                    first_emitted_error = Some(e);
                 }
             }
             let warnings = match maybe_pkg {
@@ -1248,7 +1247,7 @@ impl<'gctx> Workspace<'gctx> {
         }
     }
 
-    pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
+    pub fn emit_pkg_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
         let mut error_count = 0;
         let toml_lints = pkg
             .manifest()
@@ -1262,26 +1261,74 @@ impl<'gctx> Workspace<'gctx> {
             .cloned()
             .unwrap_or(manifest::TomlToolLints::default());
 
-        let ws_contents = match self.root_maybe() {
-            MaybePackage::Package(pkg) => pkg.manifest().contents(),
-            MaybePackage::Virtual(v) => v.contents(),
-        };
+        let ws_contents = self.root_maybe().contents();
 
-        let ws_document = match self.root_maybe() {
-            MaybePackage::Package(pkg) => pkg.manifest().document(),
-            MaybePackage::Virtual(v) => v.document(),
-        };
+        let ws_document = self.root_maybe().document();
+
+        if self.gctx.cli_unstable().cargo_lints {
+            analyze_cargo_lints_table(
+                pkg,
+                &path,
+                &cargo_lints,
+                ws_contents,
+                ws_document,
+                self.root_manifest(),
+                self.gctx,
+            )?;
+            check_im_a_teapot(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
+        }
+
+        if error_count > 0 {
+            Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
+                "encountered {error_count} errors(s) while running lints"
+            ))
+            .into())
+        } else {
+            Ok(())
+        }
+    }
+
+    pub fn emit_ws_lints(&self) -> CargoResult<()> {
+        let mut error_count = 0;
+
+        let cargo_lints = match self.root_maybe() {
+            MaybePackage::Package(pkg) => {
+                let toml = pkg.manifest().normalized_toml();
+                if let Some(ws) = &toml.workspace {
+                    ws.lints.as_ref()
+                } else {
+                    toml.lints.as_ref().map(|l| &l.lints)
+                }
+            }
+            MaybePackage::Virtual(vm) => vm
+                .normalized_toml()
+                .workspace
+                .as_ref()
+                .unwrap()
+                .lints
+                .as_ref(),
+        }
+        .and_then(|t| t.get("cargo"))
+        .cloned()
+        .unwrap_or(manifest::TomlToolLints::default());
+
+        if self.gctx.cli_unstable().cargo_lints {
+            // Calls to lint functions go in here
+        }
+
+        // This is a short term hack to allow `blanket_hint_mostly_unused`
+        // to run without requiring `-Zcargo-lints`, which should hopefully
+        // improve the testing expierience while we are collecting feedback
+        if self.gctx.cli_unstable().profile_hint_mostly_unused {
+            blanket_hint_mostly_unused(
+                self.root_maybe(),
+                self.root_manifest(),
+                &cargo_lints,
+                &mut error_count,
+                self.gctx,
+            )?;
+        }
 
-        analyze_cargo_lints_table(
-            pkg,
-            &path,
-            &cargo_lints,
-            ws_contents,
-            ws_document,
-            self.root_manifest(),
-            self.gctx,
-        )?;
-        check_im_a_teapot(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
         if error_count > 0 {
             Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
                 "encountered {error_count} errors(s) while running lints"
@@ -1888,6 +1935,41 @@ impl MaybePackage {
             MaybePackage::Virtual(_) => false,
         }
     }
+
+    pub fn contents(&self) -> &str {
+        match self {
+            MaybePackage::Package(p) => p.manifest().contents(),
+            MaybePackage::Virtual(v) => v.contents(),
+        }
+    }
+
+    pub fn document(&self) -> &toml::Spanned<toml::de::DeTable<'static>> {
+        match self {
+            MaybePackage::Package(p) => p.manifest().document(),
+            MaybePackage::Virtual(v) => v.document(),
+        }
+    }
+
+    pub fn edition(&self) -> Edition {
+        match self {
+            MaybePackage::Package(p) => p.manifest().edition(),
+            MaybePackage::Virtual(_) => Edition::default(),
+        }
+    }
+
+    pub fn profiles(&self) -> Option<&TomlProfiles> {
+        match self {
+            MaybePackage::Package(p) => p.manifest().profiles(),
+            MaybePackage::Virtual(v) => v.profiles(),
+        }
+    }
+
+    pub fn unstable_features(&self) -> &Features {
+        match self {
+            MaybePackage::Package(p) => p.manifest().unstable_features(),
+            MaybePackage::Virtual(vm) => vm.unstable_features(),
+        }
+    }
 }
 
 impl WorkspaceRootConfig {

src/cargo/util/lints.rs

@@ -1,14 +1,14 @@
-use crate::core::{Edition, Feature, Features, Manifest, Package};
+use crate::core::{Edition, Feature, Features, Manifest, MaybePackage, Package};
 use crate::{CargoResult, GlobalContext};
-use annotate_snippets::{AnnotationKind, Group, Level, Snippet};
-use cargo_util_schemas::manifest::{TomlLintLevel, TomlToolLints};
+use annotate_snippets::{AnnotationKind, Group, Level, Patch, Snippet};
+use cargo_util_schemas::manifest::{ProfilePackageSpec, TomlLintLevel, TomlToolLints};
 use pathdiff::diff_paths;
 use std::fmt::Display;
 use std::ops::Range;
 use std::path::Path;
 
 const LINT_GROUPS: &[LintGroup] = &[TEST_DUMMY_UNSTABLE];
-pub const LINTS: &[Lint] = &[IM_A_TEAPOT, UNKNOWN_LINTS];
+pub const LINTS: &[Lint] = &[BLANKET_HINT_MOSTLY_UNUSED, IM_A_TEAPOT, UNKNOWN_LINTS];
 
 pub fn analyze_cargo_lints_table(
     pkg: &Package,
@@ -473,6 +473,158 @@ pub fn check_im_a_teapot(
     Ok(())
 }
 
+const BLANKET_HINT_MOSTLY_UNUSED: Lint = Lint {
+    name: "blanket_hint_mostly_unused",
+    desc: "blanket_hint_mostly_unused lint",
+    groups: &[],
+    default_level: LintLevel::Warn,
+    edition_lint_opts: None,
+    feature_gate: None,
+    docs: Some(
+        r#"
+### What it does
+Checks if `hint-mostly-unused` being applied to all dependencies.
+
+### Why it is bad
+`hint-mostly-unused` indicates that most of a crate's API surface will go
+unused by anything depending on it; this hint can speed up the build by
+attempting to minimize compilation time for items that aren't used at all.
+Misapplication to crates that don't fit that criteria will slow down the build
+rather than speeding it up. It should be selectively applied to dependencies
+that meet these criteria. Applying it globally is always a misapplication and
+will likely slow down the build.
+
+### Example
+```toml
+[profile.dev.package."*"]
+hint-mostly-unused = true
+```
+
+Should instead be:
+```toml
+[profile.dev.package.huge-mostly-unused-dependency]
+hint-mostly-unused = true
+```
+"#,
+    ),
+};
+
+pub fn blanket_hint_mostly_unused(
+    maybe_pkg: &MaybePackage,
+    path: &Path,
+    pkg_lints: &TomlToolLints,
+    error_count: &mut usize,
+    gctx: &GlobalContext,
+) -> CargoResult<()> {
+    let (lint_level, reason) = BLANKET_HINT_MOSTLY_UNUSED.level(
+        pkg_lints,
+        maybe_pkg.edition(),
+        maybe_pkg.unstable_features(),
+    );
+
+    if lint_level == LintLevel::Allow {
+        return Ok(());
+    }
+
+    let level = lint_level.to_diagnostic_level();
+    let manifest_path = rel_cwd_manifest_path(path, gctx);
+    let mut paths = Vec::new();
+
+    if let Some(profiles) = maybe_pkg.profiles() {
+        for (profile_name, top_level_profile) in &profiles.0 {
+            if let Some(true) = top_level_profile.hint_mostly_unused {
+                paths.push((
+                    vec!["profile", profile_name.as_str(), "hint-mostly-unused"],
+                    true,
+                ));
+            }
+
+            if let Some(build_override) = &top_level_profile.build_override
+                && let Some(true) = build_override.hint_mostly_unused
+            {
+                paths.push((
+                    vec![
+                        "profile",
+                        profile_name.as_str(),
+                        "build-override",
+                        "hint-mostly-unused",
+                    ],
+                    false,
+                ));
+            }
+
+            if let Some(packages) = &top_level_profile.package
+                && let Some(profile) = packages.get(&ProfilePackageSpec::All)
+                && let Some(true) = profile.hint_mostly_unused
+            {
+                paths.push((
+                    vec![
+                        "profile",
+                        profile_name.as_str(),
+                        "package",
+                        "*",
+                        "hint-mostly-unused",
+                    ],
+                    false,
+                ));
+            }
+        }
+    }
+
+    for (i, (path, show_per_pkg_suggestion)) in paths.iter().enumerate() {
+        if lint_level.is_error() {
+            *error_count += 1;
+        }
+        let title = "`hint-mostly-unused` is being blanket applied to all dependencies";
+        let help_txt =
+            "scope `hint-mostly-unused` to specific packages with a lot of unused object code";
+        if let (Some(span), Some(table_span)) = (
+            get_key_value_span(maybe_pkg.document(), &path),
+            get_key_value_span(maybe_pkg.document(), &path[..path.len() - 1]),
+        ) {
+            let mut report = Vec::new();
+            let mut primary_group = level.clone().primary_title(title).element(
+                Snippet::source(maybe_pkg.contents())
+                    .path(&manifest_path)
+                    .annotation(
+                        AnnotationKind::Primary.span(table_span.key.start..table_span.key.end),
+                    )
+                    .annotation(AnnotationKind::Context.span(span.key.start..span.value.end)),
+            );
+
+            if *show_per_pkg_suggestion {
+                report.push(
+                    Level::HELP.secondary_title(help_txt).element(
+                        Snippet::source(maybe_pkg.contents())
+                            .path(&manifest_path)
+                            .patch(Patch::new(
+                                table_span.key.end..table_span.key.end,
+                                ".package.<pkg_name>",
+                            )),
+                    ),
+                );
+            } else {
+                primary_group = primary_group.element(Level::HELP.message(help_txt));
+            }
+
+            if i == 0 {
+                primary_group =
+                    primary_group
+                        .element(Level::NOTE.message(
+                            BLANKET_HINT_MOSTLY_UNUSED.emitted_source(lint_level, reason),
+                        ));
+            }
+
+            // The primary group should always be first
+            report.insert(0, primary_group);
+
+            gctx.shell().print_report(&report, lint_level.force())?;
+        }
+    }
+
+    Ok(())
+}
+
 const UNKNOWN_LINTS: Lint = Lint {
     name: "unknown_lints",
     desc: "unknown lint",