From d5bc35d84454fd4653b5ffe546e86363803b0a1b Mon Sep 17 00:00:00 2001 From: Scott Schafer Date: Wed, 24 Apr 2024 13:52:43 -0600 Subject: [PATCH] refactor(lints): Keep workspace and package lints separate --- src/cargo/core/workspace.rs | 42 ++++++++++++++++++++++++++++++++++--- src/cargo/util/lints.rs | 41 ++++++++++++++++++++++++++---------- src/cargo/util/toml/mod.rs | 36 +++++++++++++------------------ 3 files changed, 83 insertions(+), 36 deletions(-) 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 c5790929d82..453d8bd0c55 100644 --- a/src/cargo/util/lints.rs +++ b/src/cargo/util/lints.rs @@ -85,13 +85,25 @@ pub struct Lint { } impl Lint { - pub fn level(&self, lints: &TomlToolLints, edition: Edition) -> LintLevel { + pub fn level( + &self, + pkg_lints: &TomlToolLints, + ws_lints: &TomlToolLints, + edition: Edition, + ) -> LintLevel { self.groups .iter() .map(|g| { ( g.name, - level_priority(g.name, g.default_level, g.edition_lint_opts, lints, edition), + level_priority( + g.name, + g.default_level, + g.edition_lint_opts, + pkg_lints, + ws_lints, + edition, + ), ) }) .chain(std::iter::once(( @@ -100,7 +112,8 @@ impl Lint { self.name, self.default_level, self.edition_lint_opts, - lints, + pkg_lints, + ws_lints, edition, ), ))) @@ -155,7 +168,8 @@ fn level_priority( name: &str, default_level: LintLevel, edition_lint_opts: Option<(Edition, LintLevel)>, - lints: &TomlToolLints, + pkg_lints: &TomlToolLints, + ws_lints: &TomlToolLints, edition: Edition, ) -> (LintLevel, i8) { let unspecified_level = if let Some(level) = edition_lint_opts @@ -172,7 +186,9 @@ fn level_priority( return (unspecified_level, 0); } - if let Some(defined_level) = lints.get(name) { + if let Some(defined_level) = pkg_lints.get(name) { + (defined_level.level().into(), defined_level.priority()) + } else if let Some(defined_level) = ws_lints.get(name) { (defined_level.level().into(), defined_level.priority()) } else { (unspecified_level, 0) @@ -190,12 +206,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 = IM_A_TEAPOT.level(pkg_lints, ws_lints, manifest.edition()); if lint_level == LintLevel::Allow { return Ok(()); } @@ -258,7 +275,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<()> { @@ -269,7 +287,7 @@ pub fn check_implicit_features( return Ok(()); } - let lint_level = IMPLICIT_FEATURES.level(lints, edition); + let lint_level = IMPLICIT_FEATURES.level(pkg_lints, ws_lints, edition); if lint_level == LintLevel::Allow { return Ok(()); } @@ -341,7 +359,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<()> { @@ -351,7 +370,7 @@ pub fn unused_dependencies( return Ok(()); } - let lint_level = UNUSED_OPTIONAL_DEPENDENCY.level(lints, edition); + let lint_level = UNUSED_OPTIONAL_DEPENDENCY.level(pkg_lints, ws_lints, edition); if lint_level == LintLevel::Allow { return Ok(()); } 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