diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 55bfb7a10c6..2ef3a1a1a75 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -1180,6 +1180,21 @@ impl<'gctx> Workspace<'gctx> { } pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> { + let ws_lints = self + .root_maybe() + .workspace_config() + .inheritable() + .and_then(|i| i.lints().ok()) + .unwrap_or_default(); + + let ws_cargo_lints = ws_lints + .get("cargo") + .cloned() + .unwrap_or_default() + .into_iter() + .map(|(k, v)| (k.replace('-', "_"), v)) + .collect(); + let mut error_count = 0; let toml_lints = pkg .manifest() @@ -1197,9 +1212,30 @@ impl<'gctx> Workspace<'gctx> { .map(|(name, lint)| (name.replace('-', "_"), lint)) .collect(); - check_im_a_teapot(pkg, &path, &normalized_lints, &mut error_count, self.gctx)?; - check_implicit_features(pkg, &path, &normalized_lints, &mut error_count, self.gctx)?; - unused_dependencies(pkg, &path, &normalized_lints, &mut error_count, self.gctx)?; + check_im_a_teapot( + pkg, + &path, + &normalized_lints, + &ws_cargo_lints, + &mut error_count, + self.gctx, + )?; + check_implicit_features( + pkg, + &path, + &normalized_lints, + &ws_cargo_lints, + &mut error_count, + self.gctx, + )?; + unused_dependencies( + pkg, + &path, + &normalized_lints, + &ws_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" diff --git a/src/cargo/util/lints.rs b/src/cargo/util/lints.rs index cf752fa7f34..f177dac48e4 100644 --- a/src/cargo/util/lints.rs +++ b/src/cargo/util/lints.rs @@ -85,40 +85,41 @@ pub struct Lint { } impl Lint { - pub fn level(&self, lints: &TomlToolLints, edition: Edition) -> LintLevel { - let edition_level = self - .edition_lint_opts - .filter(|(e, _)| edition >= *e) - .map(|(_, l)| l); - - if self.default_level == LintLevel::Forbid || edition_level == Some(LintLevel::Forbid) { - return LintLevel::Forbid; - } - - let level = self - .groups + pub fn level( + &self, + pkg_lints: &TomlToolLints, + ws_lints: &TomlToolLints, + edition: Edition, + ) -> (LintLevel, LintLevelReason) { + self.groups .iter() - .map(|g| g.name) - .chain(std::iter::once(self.name)) - .filter_map(|n| lints.get(n).map(|l| (n, l))) - .max_by_key(|(n, l)| { + .map(|g| { ( - l.level() == TomlLintLevel::Forbid, - l.priority(), - std::cmp::Reverse(*n), + g.name, + level_priority( + g.name, + g.default_level, + g.edition_lint_opts, + pkg_lints, + ws_lints, + edition, + ), ) - }); - - match level { - Some((_, toml_lint)) => toml_lint.level().into(), - None => { - if let Some(level) = edition_level { - level - } else { - self.default_level - } - } - } + }) + .chain(std::iter::once(( + self.name, + level_priority( + self.name, + self.default_level, + self.edition_lint_opts, + pkg_lints, + ws_lints, + edition, + ), + ))) + .max_by_key(|(n, (l, _, p))| (l == &LintLevel::Forbid, *p, std::cmp::Reverse(*n))) + .map(|(_, (l, r, _))| (l, r)) + .unwrap() } } @@ -163,6 +164,64 @@ impl From for LintLevel { } } +#[derive(Copy, Clone, Debug)] +pub enum LintLevelReason { + Default, + Edition(Edition), + Package, + Workspace, +} + +impl Display for LintLevelReason { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + LintLevelReason::Default => write!(f, "by default"), + LintLevelReason::Edition(edition) => write!(f, "in edition {}", edition), + LintLevelReason::Package => write!(f, "in `[lints]`"), + LintLevelReason::Workspace => write!(f, "in `[workspace.lints]`"), + } + } +} + +fn level_priority( + name: &str, + default_level: LintLevel, + edition_lint_opts: Option<(Edition, LintLevel)>, + pkg_lints: &TomlToolLints, + ws_lints: &TomlToolLints, + edition: Edition, +) -> (LintLevel, LintLevelReason, i8) { + let (unspecified_level, reason) = if let Some(level) = edition_lint_opts + .filter(|(e, _)| edition >= *e) + .map(|(_, l)| l) + { + (level, LintLevelReason::Edition(edition)) + } else { + (default_level, LintLevelReason::Default) + }; + + // Don't allow the group to be overridden if the level is `Forbid` + if unspecified_level == LintLevel::Forbid { + return (unspecified_level, reason, 0); + } + + if let Some(defined_level) = pkg_lints.get(name) { + ( + defined_level.level().into(), + LintLevelReason::Package, + defined_level.priority(), + ) + } else if let Some(defined_level) = ws_lints.get(name) { + ( + defined_level.level().into(), + LintLevelReason::Workspace, + defined_level.priority(), + ) + } else { + (unspecified_level, reason, 0) + } +} + const IM_A_TEAPOT: Lint = Lint { name: "im_a_teapot", desc: "`im_a_teapot` is specified", @@ -174,12 +233,13 @@ const IM_A_TEAPOT: Lint = Lint { pub fn check_im_a_teapot( pkg: &Package, path: &Path, - lints: &TomlToolLints, + pkg_lints: &TomlToolLints, + ws_lints: &TomlToolLints, error_count: &mut usize, gctx: &GlobalContext, ) -> CargoResult<()> { let manifest = pkg.manifest(); - let lint_level = IM_A_TEAPOT.level(lints, manifest.edition()); + let (lint_level, reason) = IM_A_TEAPOT.level(pkg_lints, ws_lints, manifest.edition()); if lint_level == LintLevel::Allow { return Ok(()); } @@ -194,7 +254,10 @@ pub fn check_im_a_teapot( } let level = lint_level.to_diagnostic_level(); let manifest_path = rel_cwd_manifest_path(path, gctx); - let emitted_reason = format!("`cargo::{}` is set to `{lint_level}`", IM_A_TEAPOT.name); + let emitted_reason = format!( + "`cargo::{}` is set to `{lint_level}` {reason}", + IM_A_TEAPOT.name + ); let key_span = get_span(manifest.document(), &["package", "im-a-teapot"], false).unwrap(); let value_span = get_span(manifest.document(), &["package", "im-a-teapot"], true).unwrap(); @@ -242,7 +305,8 @@ const IMPLICIT_FEATURES: Lint = Lint { pub fn check_implicit_features( pkg: &Package, path: &Path, - lints: &TomlToolLints, + pkg_lints: &TomlToolLints, + ws_lints: &TomlToolLints, error_count: &mut usize, gctx: &GlobalContext, ) -> CargoResult<()> { @@ -253,7 +317,7 @@ pub fn check_implicit_features( return Ok(()); } - let lint_level = IMPLICIT_FEATURES.level(lints, edition); + let (lint_level, reason) = IMPLICIT_FEATURES.level(pkg_lints, ws_lints, edition); if lint_level == LintLevel::Allow { return Ok(()); } @@ -298,7 +362,7 @@ pub fn check_implicit_features( ); if emitted_source.is_none() { emitted_source = Some(format!( - "`cargo::{}` is set to `{lint_level}`", + "`cargo::{}` is set to `{lint_level}` {reason}", IMPLICIT_FEATURES.name )); message = message.footer(Level::Note.title(emitted_source.as_ref().unwrap())); @@ -325,7 +389,8 @@ const UNUSED_OPTIONAL_DEPENDENCY: Lint = Lint { pub fn unused_dependencies( pkg: &Package, path: &Path, - lints: &TomlToolLints, + pkg_lints: &TomlToolLints, + ws_lints: &TomlToolLints, error_count: &mut usize, gctx: &GlobalContext, ) -> CargoResult<()> { @@ -335,7 +400,7 @@ pub fn unused_dependencies( return Ok(()); } - let lint_level = UNUSED_OPTIONAL_DEPENDENCY.level(lints, edition); + let (lint_level, reason) = UNUSED_OPTIONAL_DEPENDENCY.level(pkg_lints, ws_lints, edition); if lint_level == LintLevel::Allow { return Ok(()); } @@ -401,7 +466,7 @@ pub fn unused_dependencies( ); if emitted_source.is_none() { emitted_source = Some(format!( - "`cargo::{}` is set to `{lint_level}`", + "`cargo::{}` is set to `{lint_level}` {reason}", UNUSED_OPTIONAL_DEPENDENCY.name )); message = diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 16cba3cd3a2..0234df9a22a 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -411,15 +411,7 @@ fn resolve_toml( } resolved_toml.target = (!resolved_target.is_empty()).then_some(resolved_target); - let resolved_lints = original_toml - .lints - .clone() - .map(|value| lints_inherit_with(value, || inherit()?.lints())) - .transpose()?; - resolved_toml.lints = resolved_lints.map(|lints| manifest::InheritableLints { - workspace: false, - lints, - }); + resolved_toml.lints = original_toml.lints.clone(); let resolved_badges = original_toml .badges @@ -803,7 +795,7 @@ impl InheritableFields { } /// Gets the field `workspace.lint`. - fn lints(&self) -> CargoResult { + pub fn lints(&self) -> CargoResult { let Some(val) = &self.lints else { bail!("`workspace.lints` was not defined"); }; @@ -1284,18 +1276,18 @@ fn to_real_manifest( } } - verify_lints( - resolved_toml.resolved_lints().expect("previously resolved"), - gctx, - warnings, - )?; - let default = manifest::TomlLints::default(); - let rustflags = lints_to_rustflags( - resolved_toml - .resolved_lints() - .expect("previously resolved") - .unwrap_or(&default), - ); + let resolved_lints = resolved_toml + .lints + .clone() + .map(|value| { + lints_inherit_with(value, || { + load_inheritable_fields(gctx, manifest_file, &workspace_config)?.lints() + }) + }) + .transpose()?; + + verify_lints(resolved_lints.as_ref(), gctx, warnings)?; + let rustflags = lints_to_rustflags(&resolved_lints.unwrap_or_default()); let metadata = ManifestMetadata { description: resolved_package diff --git a/tests/testsuite/lints/implicit_features/edition_2021_warn/stderr.term.svg b/tests/testsuite/lints/implicit_features/edition_2021_warn/stderr.term.svg index 75040932d82..11affcc2ee0 100644 --- a/tests/testsuite/lints/implicit_features/edition_2021_warn/stderr.term.svg +++ b/tests/testsuite/lints/implicit_features/edition_2021_warn/stderr.term.svg @@ -33,7 +33,7 @@ | - = note: `cargo::implicit_features` is set to `warn` + = note: `cargo::implicit_features` is set to `warn` in `[lints]` warning: implicit features for optional dependencies is deprecated and will be unavailable in the 2024 edition diff --git a/tests/testsuite/lints/unused_optional_dependencies/edition_2024/stderr.term.svg b/tests/testsuite/lints/unused_optional_dependencies/edition_2024/stderr.term.svg index 1b93aa050e5..84eddf6de57 100644 --- a/tests/testsuite/lints/unused_optional_dependencies/edition_2024/stderr.term.svg +++ b/tests/testsuite/lints/unused_optional_dependencies/edition_2024/stderr.term.svg @@ -34,7 +34,7 @@ | - = note: `cargo::unused_optional_dependency` is set to `warn` + = note: `cargo::unused_optional_dependency` is set to `warn` by default = help: remove the dependency or activate it in a feature with `dep:bar` diff --git a/tests/testsuite/lints/unused_optional_dependencies/renamed_deps/stderr.term.svg b/tests/testsuite/lints/unused_optional_dependencies/renamed_deps/stderr.term.svg index 4d76be9bfbe..2f1c99bec30 100644 --- a/tests/testsuite/lints/unused_optional_dependencies/renamed_deps/stderr.term.svg +++ b/tests/testsuite/lints/unused_optional_dependencies/renamed_deps/stderr.term.svg @@ -34,7 +34,7 @@ | - = note: `cargo::unused_optional_dependency` is set to `warn` + = note: `cargo::unused_optional_dependency` is set to `warn` by default = help: remove the dependency or activate it in a feature with `dep:bar` diff --git a/tests/testsuite/lints_table.rs b/tests/testsuite/lints_table.rs index 451a3dc59c9..75d0de4ee27 100644 --- a/tests/testsuite/lints_table.rs +++ b/tests/testsuite/lints_table.rs @@ -852,7 +852,7 @@ warning: `im_a_teapot` is specified 9 | im-a-teapot = true | ------------------ | - = note: `cargo::im_a_teapot` is set to `warn` + = note: `cargo::im_a_teapot` is set to `warn` in `[lints]` [CHECKING] foo v0.0.1 ([CWD]) [FINISHED] [..] ", @@ -892,7 +892,7 @@ warning: `im_a_teapot` is specified 9 | im-a-teapot = true | ------------------ | - = note: `cargo::im_a_teapot` is set to `warn` + = note: `cargo::im_a_teapot` is set to `warn` in `[lints]` [CHECKING] foo v0.0.1 ([CWD]) [FINISHED] [..] ", @@ -934,7 +934,50 @@ error: `im_a_teapot` is specified 9 | im-a-teapot = true | ^^^^^^^^^^^^^^^^^^ | - = note: `cargo::im_a_teapot` is set to `forbid` + = note: `cargo::im_a_teapot` is set to `forbid` in `[lints]` +", + ) + .run(); +} + +#[cargo_test] +fn workspace_cargo_lints() { + let p = project() + .file( + "Cargo.toml", + r#" +cargo-features = ["test-dummy-unstable"] + +[workspace.lints.cargo] +im-a-teapot = { level = "warn", priority = 10 } +test-dummy-unstable = { level = "forbid", priority = -1 } + +[package] +name = "foo" +version = "0.0.1" +edition = "2015" +authors = [] +im-a-teapot = true + +[lints] +workspace = true + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints", "test-dummy-unstable"]) + .with_status(101) + .with_stderr( + "\ +error: `im_a_teapot` is specified + --> Cargo.toml:13:1 + | +13 | im-a-teapot = true + | ^^^^^^^^^^^^^^^^^^ + | + = note: `cargo::im_a_teapot` is set to `forbid` in `[workspace.lints]` ", ) .run();