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
diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs
index 096b333..fc20aa2 100644
--- a/src/cargo/core/workspace.rs
+++ b/src/cargo/core/workspace.rs
@@ -25,7 +25,9 @@
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 @@
}
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 @@
/// 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 @@
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 @@
}
}
- 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 @@
.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();
- 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 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,
+ )?;
+ }
+
if error_count > 0 {
Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
"encountered {error_count} errors(s) while running lints"
@@ -1888,6 +1935,41 @@
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 {
diff --git a/src/cargo/util/lints.rs b/src/cargo/util/lints.rs
index 0763be7..ca43ff0 100644
--- a/src/cargo/util/lints.rs
+++ b/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 @@
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",
diff --git a/src/doc/src/reference/lints.md b/src/doc/src/reference/lints.md
index d1f1244..8393efb 100644
--- a/src/doc/src/reference/lints.md
+++ b/src/doc/src/reference/lints.md
@@ -5,8 +5,37 @@
## Warn-by-default
These lints are all set to the 'warn' level by default.
+- [`blanket_hint_mostly_unused`](#blanket_hint_mostly_unused)
- [`unknown_lints`](#unknown_lints)
+## `blanket_hint_mostly_unused`
+Set to `warn` by default
+
+### 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
+```
+
+
## `unknown_lints`
Set to `warn` by default
diff --git a/tests/testsuite/lints/blanket_hint_mostly_unused.rs b/tests/testsuite/lints/blanket_hint_mostly_unused.rs
new file mode 100644
index 0000000..18e55a8
--- /dev/null
+++ b/tests/testsuite/lints/blanket_hint_mostly_unused.rs
@@ -0,0 +1,167 @@
+use crate::prelude::*;
+use cargo_test_support::project;
+use cargo_test_support::str;
+
+#[cargo_test(nightly, reason = "-Zhint-mostly-unused is unstable")]
+fn named_profile_blanket() {
+ let p = project()
+ .file(
+ "Cargo.toml",
+ r#"
+[package]
+name = "foo"
+version = "0.0.1"
+edition = "2015"
+
+[profile.dev]
+hint-mostly-unused = true
+"#,
+ )
+ .file("src/main.rs", "fn main() {}")
+ .build();
+ p.cargo("check -Zprofile-hint-mostly-unused -v")
+ .masquerade_as_nightly_cargo(&["profile-hint-mostly-unused", "cargo-lints"])
+ .with_stderr_data(str![[r#"
+[WARNING] `hint-mostly-unused` is being blanket applied to all dependencies
+ --> Cargo.toml:7:10
+ |
+7 | [profile.dev]
+ | ^^^
+8 | hint-mostly-unused = true
+ | -------------------------
+ |
+ = [NOTE] `cargo::blanket_hint_mostly_unused` is set to `warn` by default
+[HELP] scope `hint-mostly-unused` to specific packages with a lot of unused object code
+ |
+7 | [profile.dev.package.<pkg_name>]
+ | +++++++++++++++++++
+[CHECKING] foo v0.0.1 ([ROOT]/foo)
+[RUNNING] `rustc --crate-name foo [..]`
+[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
+
+"#]])
+ .run();
+}
+
+#[cargo_test(nightly, reason = "-Zhint-mostly-unused is unstable")]
+fn profile_package_wildcard() {
+ let p = project()
+ .file(
+ "Cargo.toml",
+ r#"
+[package]
+name = "foo"
+version = "0.0.1"
+edition = "2015"
+
+[profile.dev.package."*"]
+hint-mostly-unused = true
+"#,
+ )
+ .file("src/main.rs", "fn main() {}")
+ .build();
+ p.cargo("check -Zprofile-hint-mostly-unused -v")
+ .masquerade_as_nightly_cargo(&["profile-hint-mostly-unused", "cargo-lints"])
+ .with_stderr_data(str![[r#"
+[WARNING] `hint-mostly-unused` is being blanket applied to all dependencies
+ --> Cargo.toml:7:22
+ |
+7 | [profile.dev.package."*"]
+ | ^^^
+8 | hint-mostly-unused = true
+ | -------------------------
+ |
+ = [HELP] scope `hint-mostly-unused` to specific packages with a lot of unused object code
+ = [NOTE] `cargo::blanket_hint_mostly_unused` is set to `warn` by default
+[CHECKING] foo v0.0.1 ([ROOT]/foo)
+[RUNNING] `rustc --crate-name foo [..]`
+[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
+
+"#]])
+ .run();
+}
+
+#[cargo_test(nightly, reason = "-Zhint-mostly-unused is unstable")]
+fn profile_build_override() {
+ let p = project()
+ .file(
+ "Cargo.toml",
+ r#"
+[package]
+name = "foo"
+version = "0.0.1"
+edition = "2015"
+
+[profile.dev.build-override]
+hint-mostly-unused = true
+"#,
+ )
+ .file("src/main.rs", "fn main() {}")
+ .build();
+ p.cargo("check -Zprofile-hint-mostly-unused -v")
+ .masquerade_as_nightly_cargo(&["profile-hint-mostly-unused", "cargo-lints"])
+ .with_stderr_data(str![[r#"
+[WARNING] `hint-mostly-unused` is being blanket applied to all dependencies
+ --> Cargo.toml:7:14
+ |
+7 | [profile.dev.build-override]
+ | ^^^^^^^^^^^^^^
+8 | hint-mostly-unused = true
+ | -------------------------
+ |
+ = [HELP] scope `hint-mostly-unused` to specific packages with a lot of unused object code
+ = [NOTE] `cargo::blanket_hint_mostly_unused` is set to `warn` by default
+[CHECKING] foo v0.0.1 ([ROOT]/foo)
+[RUNNING] `rustc --crate-name foo [..]`
+[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
+
+"#]])
+ .run();
+}
+
+#[cargo_test(nightly, reason = "-Zhint-mostly-unused is unstable")]
+fn workspace_profile_package_wildcard() {
+ let p = project()
+ .file(
+ "Cargo.toml",
+ r#"
+[workspace]
+members = ["foo"]
+
+[profile.dev.package."*"]
+hint-mostly-unused = true
+"#,
+ )
+ .file(
+ "foo/Cargo.toml",
+ r#"
+[package]
+name = "foo"
+version = "0.0.1"
+edition = "2015"
+authors = []
+"#,
+ )
+ .file("foo/src/lib.rs", "")
+ .build();
+
+ p.cargo("check -Zprofile-hint-mostly-unused -v")
+ .masquerade_as_nightly_cargo(&["profile-hint-mostly-unused", "cargo-lints"])
+ .with_stderr_data(str![[r#"
+[WARNING] `hint-mostly-unused` is being blanket applied to all dependencies
+ --> Cargo.toml:5:22
+ |
+5 | [profile.dev.package."*"]
+ | ^^^
+6 | hint-mostly-unused = true
+ | -------------------------
+ |
+ = [HELP] scope `hint-mostly-unused` to specific packages with a lot of unused object code
+ = [NOTE] `cargo::blanket_hint_mostly_unused` is set to `warn` by default
+[CHECKING] foo v0.0.1 ([ROOT]/foo/foo)
+[RUNNING] `rustc --crate-name foo [..]`
+[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
+
+"#]])
+ .run();
+}
diff --git a/tests/testsuite/lints/mod.rs b/tests/testsuite/lints/mod.rs
index 908663a..ac92d79 100644
--- a/tests/testsuite/lints/mod.rs
+++ b/tests/testsuite/lints/mod.rs
@@ -3,6 +3,7 @@
use cargo_test_support::registry::Package;
use cargo_test_support::str;
+mod blanket_hint_mostly_unused;
mod error;
mod inherited;
mod unknown_lints;
