Skip to content

Commit

Permalink
Respect [lints.rust.unexpected_cfgs] lint level
Browse files Browse the repository at this point in the history
  • Loading branch information
Urgau committed Oct 2, 2024
1 parent 0715b1c commit 655bedd
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 23 deletions.
6 changes: 5 additions & 1 deletion src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,10 @@ impl<'gctx> Workspace<'gctx> {
.get("cargo")
.cloned()
.unwrap_or(manifest::TomlToolLints::default());
let rust_lints = toml_lints
.get("rust")
.cloned()
.unwrap_or(manifest::TomlToolLints::default());

let ws_contents = match self.root_maybe() {
MaybePackage::Package(pkg) => pkg.manifest().contents(),
Expand All @@ -1238,7 +1242,7 @@ impl<'gctx> Workspace<'gctx> {
self.gctx,
)?;
check_im_a_teapot(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
unexpected_target_cfgs(self, pkg, &path, &mut error_count, self.gctx)?;
unexpected_target_cfgs(self, pkg, &path, &rust_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"
Expand Down
24 changes: 21 additions & 3 deletions src/cargo/util/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::path::Path;
use toml_edit::ImDocument;

const LINT_GROUPS: &[LintGroup] = &[TEST_DUMMY_UNSTABLE];
pub const LINTS: &[Lint] = &[IM_A_TEAPOT, UNKNOWN_LINTS];
pub const LINTS: &[Lint] = &[IM_A_TEAPOT, UNKNOWN_LINTS, UNEXPECTED_CFGS];

pub fn analyze_cargo_lints_table(
pkg: &Package,
Expand Down Expand Up @@ -608,17 +608,35 @@ fn output_unknown_lints(
Ok(())
}

// FIXME: This lint is only used for the Cargo infra, it's actually defined in rustc
// it-self, which is broken is several ways, as it doesn't take into account
// `rustc` flags ('via `RUSTFLAGS`), nor the possible `rustc` lints groups, ...
const UNEXPECTED_CFGS: Lint = Lint {
name: "unexpected_cfgs",
desc: "lint on unexpected target cfgs",
groups: &[],
default_level: LintLevel::Warn,
edition_lint_opts: None,
feature_gate: None,
docs: None,
};

pub fn unexpected_target_cfgs(
ws: &Workspace<'_>,
pkg: &Package,
path: &Path,
rust_lints: &TomlToolLints,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
let manifest = pkg.manifest();

// FIXME: We should get the lint level from `[lints.rust.unexpected_cfgs]`
let lint_level = LintLevel::Warn;
let (lint_level, _lint_reason) =
UNEXPECTED_CFGS.level(rust_lints, manifest.edition(), manifest.unstable_features());

if lint_level == LintLevel::Allow {
return Ok(());
}

let rustc = gctx.load_global_rustc(Some(ws))?;
// FIXME: While it doesn't doesn't really matter for `--print=check-cfg`, we should
Expand Down
26 changes: 7 additions & 19 deletions tests/testsuite/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -744,14 +744,7 @@ fn unexpected_cfgs_target_lint_level_allow() {

p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
// FIXME: We shouldn't warn any target cfgs because of the level="allow"
.with_stderr_data(str![[r#"
[WARNING] unexpected `cfg` condition name: foo
--> Cargo.toml:8:25
|
8 | [target."cfg(foo)".dependencies]
| ----------
|
[LOCKING] 1 package to latest compatible version
[CHECKING] a v0.0.1 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
Expand Down Expand Up @@ -787,19 +780,15 @@ fn unexpected_cfgs_target_lint_level_deny() {
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
.with_stderr_data(str![[r#"
[WARNING] unexpected `cfg` condition name: foo
[ERROR] unexpected `cfg` condition name: foo
--> Cargo.toml:8:25
|
8 | [target."cfg(foo)".dependencies]
| ----------
| ^^^^^^^^^^
|
[LOCKING] 1 package to latest compatible version
[CHECKING] a v0.0.1 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
"#]])
// FIXME: this test should fail
// .with_status(101)
.with_status(101)
.run();
}

Expand Down Expand Up @@ -832,17 +821,16 @@ fn unexpected_cfgs_target_cfg_any() {
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
// FIXME: We shouldn't be linting `cfg(foo)` because of the `cfg(any())`
.with_stderr_data(str![[r#"
[WARNING] unexpected `cfg` condition name: foo
[ERROR] unexpected `cfg` condition name: foo
--> Cargo.toml:8:25
|
8 | [target."cfg(foo)".dependencies]
| ----------
| ^^^^^^^^^^
|
[LOCKING] 1 package to latest compatible version
[CHECKING] a v0.0.1 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
"#]])
// nor should we error out because of the level="deny"
.with_status(101)
.run();
}

Expand Down

0 comments on commit 655bedd

Please sign in to comment.