Skip to content

Commit

Permalink
Auto merge of #13812 - Muscraft:dont-always-inherit-workspace-lints, …
Browse files Browse the repository at this point in the history
…r=epage

fix(cargo-lints): Don't always inherit workspace lints

When working on changes for #13805, I noticed that we always passed the contents of `[workspace.lints.cargo]` into the currently implemented lints,  even if `[lints]` was not specified or did not contain `workspace = true`. This PR makes it so we only pass in the workspace cargo lints if `[lints]` contains `workspace = true`.

You can verify this change by looking at the first commit, where I added a test showing the current behavior, and looking at the second commit and seeing the test output no longer shows a warning about specifying `im-a-teapot`.
  • Loading branch information
bors committed Apr 27, 2024
2 parents c4e19cc + cf197fc commit eee4ea2
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 26 deletions.
55 changes: 35 additions & 20 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1147,11 +1147,26 @@ impl<'gctx> Workspace<'gctx> {
}

pub fn emit_warnings(&self) -> 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();

for (path, maybe_pkg) in &self.packages.packages {
let path = path.join("Cargo.toml");
if let MaybePackage::Package(pkg) = maybe_pkg {
if self.gctx.cli_unstable().cargo_lints {
self.emit_lints(pkg, &path)?
self.emit_lints(pkg, &path, &ws_cargo_lints)?
}
}
let warnings = match maybe_pkg {
Expand Down Expand Up @@ -1179,22 +1194,12 @@ impl<'gctx> Workspace<'gctx> {
Ok(())
}

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();

pub fn emit_lints(
&self,
pkg: &Package,
path: &Path,
ws_cargo_lints: &manifest::TomlToolLints,
) -> CargoResult<()> {
let mut error_count = 0;
let toml_lints = pkg
.manifest()
Expand All @@ -1212,27 +1217,37 @@ impl<'gctx> Workspace<'gctx> {
.map(|(name, lint)| (name.replace('-', "_"), lint))
.collect();

// We should only be using workspace lints if the `[lints]` table is
// present in the manifest, and `workspace` is set to `true`
let ws_cargo_lints = pkg
.manifest()
.resolved_toml()
.lints
.as_ref()
.is_some_and(|l| l.workspace)
.then(|| ws_cargo_lints);

check_im_a_teapot(
pkg,
&path,
&normalized_lints,
&ws_cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
check_implicit_features(
pkg,
&path,
&normalized_lints,
&ws_cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
unused_dependencies(
pkg,
&path,
&normalized_lints,
&ws_cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
Expand Down
12 changes: 6 additions & 6 deletions src/cargo/util/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl Lint {
pub fn level(
&self,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
edition: Edition,
) -> (LintLevel, LintLevelReason) {
self.groups
Expand Down Expand Up @@ -188,7 +188,7 @@ fn level_priority(
default_level: LintLevel,
edition_lint_opts: Option<(Edition, LintLevel)>,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
edition: Edition,
) -> (LintLevel, LintLevelReason, i8) {
let (unspecified_level, reason) = if let Some(level) = edition_lint_opts
Expand All @@ -211,7 +211,7 @@ fn level_priority(
LintLevelReason::Package,
defined_level.priority(),
)
} else if let Some(defined_level) = ws_lints.get(name) {
} else if let Some(defined_level) = ws_lints.and_then(|l| l.get(name)) {
(
defined_level.level().into(),
LintLevelReason::Workspace,
Expand All @@ -234,7 +234,7 @@ pub fn check_im_a_teapot(
pkg: &Package,
path: &Path,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
Expand Down Expand Up @@ -306,7 +306,7 @@ pub fn check_implicit_features(
pkg: &Package,
path: &Path,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
Expand Down Expand Up @@ -390,7 +390,7 @@ pub fn unused_dependencies(
pkg: &Package,
path: &Path,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
Expand Down
40 changes: 40 additions & 0 deletions tests/testsuite/lints_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -982,3 +982,43 @@ error: `im_a_teapot` is specified
)
.run();
}

#[cargo_test]
fn dont_always_inherit_workspace_lints() {
let p = project()
.file(
"Cargo.toml",
r#"
[workspace]
members = ["foo"]
[workspace.lints.cargo]
im-a-teapot = "warn"
"#,
)
.file(
"foo/Cargo.toml",
r#"
cargo-features = ["test-dummy-unstable"]
[package]
name = "foo"
version = "0.0.1"
edition = "2015"
authors = []
im-a-teapot = true
"#,
)
.file("foo/src/lib.rs", "")
.build();

p.cargo("check -Zcargo-lints")
.masquerade_as_nightly_cargo(&["cargo-lints"])
.with_stderr(
"\
[CHECKING] foo v0.0.1 ([CWD]/foo)
[FINISHED] [..]
",
)
.run();
}

0 comments on commit eee4ea2

Please sign in to comment.