diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 1db728d81ca..f2e8397756d 100644 --- a/src/cargo/core/workspace.rs +++ b/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> { + 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 0763be70e28..ca43ff0b658 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 @@ 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.", + )), + ), + ); + } 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 d1f1244569a..8393efb9ec5 100644 --- a/src/doc/src/reference/lints.md +++ b/src/doc/src/reference/lints.md @@ -5,8 +5,37 @@ Note: [Cargo's linting system is unstable](unstable.md#lintscargo) and can only ## 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 00000000000..18e55a8447b --- /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.] + | +++++++++++++++++++ +[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 908663af8fa..ac92d791274 100644 --- a/tests/testsuite/lints/mod.rs +++ b/tests/testsuite/lints/mod.rs @@ -3,6 +3,7 @@ use cargo_test_support::project; use cargo_test_support::registry::Package; use cargo_test_support::str; +mod blanket_hint_mostly_unused; mod error; mod inherited; mod unknown_lints;