Skip to content

Commit 48cb321

Browse files
committed
Lint against some unexpected target cfgs
with the help of the Cargo lints infra
1 parent 4dc3a8d commit 48cb321

File tree

4 files changed

+240
-23
lines changed

4 files changed

+240
-23
lines changed

src/cargo/core/workspace.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use crate::util::context::FeatureUnification;
2525
use crate::util::edit_distance;
2626
use crate::util::errors::{CargoResult, ManifestError};
2727
use crate::util::interning::InternedString;
28-
use crate::util::lints::{analyze_cargo_lints_table, check_im_a_teapot};
28+
use crate::util::lints::{analyze_cargo_lints_table, check_im_a_teapot, unexpected_target_cfgs};
2929
use crate::util::toml::{read_manifest, InheritableFields};
3030
use crate::util::{
3131
context::CargoResolverConfig, context::ConfigRelativePath, context::IncompatibleRustVersions,
@@ -1261,6 +1261,7 @@ impl<'gctx> Workspace<'gctx> {
12611261
self.gctx,
12621262
)?;
12631263
check_im_a_teapot(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
1264+
unexpected_target_cfgs(self, pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
12641265
if error_count > 0 {
12651266
Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
12661267
"encountered {error_count} errors(s) while running lints"

src/cargo/util/lints.rs

Lines changed: 163 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
1-
use crate::core::{Edition, Feature, Features, Manifest, Package};
1+
use crate::core::compiler::{CompileKind, TargetInfo};
2+
use crate::core::{Edition, Feature, Features, Manifest, Package, Workspace};
23
use crate::{CargoResult, GlobalContext};
34
use annotate_snippets::{Level, Snippet};
5+
use cargo_platform::{Cfg, ExpectedValues, Platform};
46
use cargo_util_schemas::manifest::{TomlLintLevel, TomlToolLints};
57
use pathdiff::diff_paths;
8+
use std::borrow::Cow;
69
use std::fmt::Display;
710
use std::ops::Range;
811
use std::path::Path;
912
use toml_edit::ImDocument;
1013

1114
const LINT_GROUPS: &[LintGroup] = &[TEST_DUMMY_UNSTABLE];
12-
pub const LINTS: &[Lint] = &[IM_A_TEAPOT, UNKNOWN_LINTS];
15+
pub const LINTS: &[Lint] = &[IM_A_TEAPOT, UNEXPECTED_CFGS, UNKNOWN_LINTS];
1316

1417
pub fn analyze_cargo_lints_table(
1518
pkg: &Package,
@@ -605,6 +608,164 @@ fn output_unknown_lints(
605608
Ok(())
606609
}
607610

611+
const UNEXPECTED_CFGS: Lint = Lint {
612+
name: "unexpected_cfgs",
613+
desc: "lint on unexpected target cfgs",
614+
groups: &[],
615+
default_level: LintLevel::Warn,
616+
edition_lint_opts: None,
617+
feature_gate: None,
618+
docs: Some(
619+
r#"
620+
### What it does
621+
Checks for unexpected cfgs in `[target.'cfg(...)']`
622+
623+
### Why it is bad
624+
The lint helps with verifying that the crate is correctly handling conditional
625+
compilation for different target platforms. It ensures that the cfg settings are
626+
consistent between what is intended and what is used, helping to
627+
catch potential bugs or errors early in the development process.
628+
629+
### Example
630+
```toml
631+
[lints.cargo]
632+
unexpected_cfgs = "warn"
633+
```
634+
"#,
635+
),
636+
};
637+
638+
pub fn unexpected_target_cfgs(
639+
ws: &Workspace<'_>,
640+
pkg: &Package,
641+
path: &Path,
642+
pkg_lints: &TomlToolLints,
643+
error_count: &mut usize,
644+
gctx: &GlobalContext,
645+
) -> CargoResult<()> {
646+
let manifest = pkg.manifest();
647+
648+
let (lint_level, _lint_reason) =
649+
UNEXPECTED_CFGS.level(pkg_lints, manifest.edition(), manifest.unstable_features());
650+
651+
if lint_level == LintLevel::Allow {
652+
return Ok(());
653+
}
654+
655+
let rustc = gctx.load_global_rustc(Some(ws))?;
656+
// FIXME: While it doesn't doesn't really matter for `--print=check-cfg`, we should
657+
// still pass the actual requested targets instead of an empty array so that the
658+
// target info can always be de-duplicated.
659+
// FIXME: Move the target info creation before any linting is done and re-use it for
660+
// all the packages.
661+
let compile_kinds = CompileKind::from_requested_targets(gctx, &[])?;
662+
let target_info = TargetInfo::new(gctx, &compile_kinds, &rustc, CompileKind::Host)?;
663+
664+
let Some(global_check_cfg) = target_info.check_cfg() else {
665+
return Ok(());
666+
};
667+
668+
if !global_check_cfg.exhaustive {
669+
return Ok(());
670+
}
671+
672+
// FIXME: If the `[lints.rust.unexpected_cfgs.check-cfg]` config is set we should
673+
// re-fetch the check-cfg informations with those extra args
674+
675+
for dep in pkg.summary().dependencies() {
676+
let Some(platform) = dep.platform() else {
677+
continue;
678+
};
679+
let Platform::Cfg(cfg_expr) = platform else {
680+
continue;
681+
};
682+
683+
cfg_expr.walk(|cfg| -> CargoResult<()> {
684+
let (name, value) = match cfg {
685+
Cfg::Name(name) => (name, None),
686+
Cfg::KeyPair(name, value) => (name, Some(value.to_string())),
687+
};
688+
689+
match global_check_cfg.expecteds.get(name.as_str()) {
690+
Some(ExpectedValues::Some(values)) if !values.contains(&value) => {
691+
let level = lint_level.to_diagnostic_level();
692+
if lint_level == LintLevel::Forbid || lint_level == LintLevel::Deny {
693+
*error_count += 1;
694+
}
695+
696+
let value = match value {
697+
Some(value) => Cow::from(format!("`{value}`")),
698+
None => Cow::from("(none)"),
699+
};
700+
701+
// Retrieving the span can fail if our pretty-priting doesn't match what the
702+
// user wrote: `unix="foo"` and `unix = "foo"`, for that reason we don't fail
703+
// and just produce a slightly worth diagnostic.
704+
if let Some(span) = get_span(manifest.document(), &["target", &*format!("cfg({cfg_expr})")], false) {
705+
let manifest_path = rel_cwd_manifest_path(path, gctx);
706+
let title = format!(
707+
"unexpected `cfg` condition value: {value} for `{cfg}`"
708+
);
709+
let diag = level.title(&*title).snippet(
710+
Snippet::source(manifest.contents())
711+
.origin(&manifest_path)
712+
.annotation(level.span(span))
713+
.fold(true)
714+
);
715+
gctx.shell().print_message(diag)?;
716+
} else {
717+
let title = format!(
718+
"unexpected `cfg` condition value: {value} for `{cfg}` in `[target.'cfg({cfg_expr})']`"
719+
);
720+
let help_where = format!("occurred in `{path}`", path = path.display());
721+
let diag = level.title(&*title).footer(Level::Help.title(&*help_where));
722+
gctx.shell().print_message(diag)?;
723+
}
724+
}
725+
None => {
726+
let level = lint_level.to_diagnostic_level();
727+
if lint_level == LintLevel::Forbid || lint_level == LintLevel::Deny {
728+
*error_count += 1;
729+
}
730+
731+
let for_cfg = match value {
732+
Some(_value) => Cow::from(format!(" for `{cfg}`")),
733+
None => Cow::from(""),
734+
};
735+
736+
// Retrieving the span can fail if our pretty-priting doesn't match what the
737+
// user wrote: `unix="foo"` and `unix = "foo"`, for that reason we don't fail
738+
// and just produce a slightly worth diagnostic.
739+
if let Some(span) = get_span(manifest.document(), &["target", &*format!("cfg({cfg_expr})")], false) {
740+
let manifest_path = rel_cwd_manifest_path(path, gctx);
741+
let title = format!(
742+
"unexpected `cfg` condition name: {name}{for_cfg}"
743+
);
744+
let diag = level.title(&*title).snippet(
745+
Snippet::source(manifest.contents())
746+
.origin(&manifest_path)
747+
.annotation(level.span(span))
748+
.fold(true)
749+
);
750+
gctx.shell().print_message(diag)?;
751+
} else {
752+
let title = format!(
753+
"unexpected `cfg` condition name: {name}{for_cfg} in `[target.'cfg({cfg_expr})']`"
754+
);
755+
let help_where = format!("occurred in `{path}`", path = path.display());
756+
let diag = level.title(&*title).footer(Level::Help.title(&*help_where));
757+
gctx.shell().print_message(diag)?;
758+
}
759+
}
760+
_ => { /* not unexpected */ }
761+
}
762+
Ok(())
763+
})?;
764+
}
765+
766+
Ok(())
767+
}
768+
608769
#[cfg(test)]
609770
mod tests {
610771
use itertools::Itertools;

src/doc/src/reference/unstable.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1924,7 +1924,7 @@ This feature checks for unexpected cfgs in `[target.'cfg(...)']` entries, based
19241924
on `rustc --print=check-cfg`.
19251925

19261926
```sh
1927-
cargo check -Zcheck-target-cfgs
1927+
cargo check -Zcargo-lints -Zcheck-target-cfgs
19281928
```
19291929

19301930
It follows the lint Rust `unexpected_cfgs` lint configuration:

tests/testsuite/cfg.rs

Lines changed: 74 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -860,10 +860,39 @@ fn unexpected_cfgs_target() {
860860
.file("c/src/lib.rs", "")
861861
.build();
862862

863-
p.cargo("check -Zcheck-target-cfgs")
863+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
864864
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
865-
// FIXME: We should warn on multiple cfgs
866865
.with_stderr_data(str![[r#"
866+
[WARNING] unexpected `cfg` condition name: foo
867+
--> Cargo.toml:11:25
868+
|
869+
11 | [target."cfg(any(foo, all(bar)))".dependencies]
870+
| -------------------------
871+
|
872+
[WARNING] unexpected `cfg` condition name: bar
873+
--> Cargo.toml:11:25
874+
|
875+
11 | [target."cfg(any(foo, all(bar)))".dependencies]
876+
| -------------------------
877+
|
878+
[WARNING] unexpected `cfg` condition value: `` for `windows = ""`
879+
--> Cargo.toml:18:25
880+
|
881+
18 | [target.'cfg(not(windows = ""))'.dependencies]
882+
| ------------------------
883+
|
884+
[WARNING] unexpected `cfg` condition value: `zoo` for `unix = "zoo"`
885+
--> Cargo.toml:14:25
886+
|
887+
14 | [target.'cfg(unix = "zoo")'.dependencies]
888+
| -------------------
889+
|
890+
[WARNING] unexpected `cfg` condition value: `zoo` for `unix = "zoo"`
891+
--> Cargo.toml:14:25
892+
|
893+
14 | [target.'cfg(unix = "zoo")'.dependencies]
894+
| -------------------
895+
|
867896
[LOCKING] 2 packages to latest compatible versions
868897
[CHECKING] b v0.0.1 ([ROOT]/foo/b)
869898
[CHECKING] a v0.0.1 ([ROOT]/foo)
@@ -910,10 +939,28 @@ fn unexpected_cfgs_target_with_lint() {
910939
.file("b/src/lib.rs", "")
911940
.build();
912941

913-
p.cargo("check -Zcheck-target-cfgs")
942+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
914943
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
915-
// FIXME: We should warn on multiple cfgs
944+
// FIXME: We should not warn on `cfg(foo = "foo")` but we currently do
916945
.with_stderr_data(str![[r#"
946+
[WARNING] unexpected `cfg` condition name: bar
947+
--> Cargo.toml:14:25
948+
|
949+
14 | [target."cfg(bar)".dependencies]
950+
| ----------
951+
|
952+
[WARNING] unexpected `cfg` condition name: foo for `foo = "foo"`
953+
--> Cargo.toml:11:25
954+
|
955+
11 | [target.'cfg(foo = "foo")'.dependencies] # should not warn here
956+
| ------------------
957+
|
958+
[WARNING] unexpected `cfg` condition name: foo
959+
--> Cargo.toml:8:25
960+
|
961+
8 | [target."cfg(foo)".dependencies] # should not warn here
962+
| ----------
963+
|
917964
[LOCKING] 1 package to latest compatible version
918965
[CHECKING] a v0.0.1 ([ROOT]/foo)
919966
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -934,10 +981,10 @@ fn unexpected_cfgs_target_diagnostics() {
934981
edition = "2015"
935982
authors = []
936983
937-
[target."cfg(target_pointer_width)".dependencies]
984+
[target."cfg(target_pointer_width)".dependencies] # expect (none) as value
938985
b = { path = 'b' }
939986
940-
[target.'cfg( all(foo , bar))'.dependencies]
987+
[target.'cfg( all(foo , bar))'.dependencies] # no snippet due to weird formatting
941988
b = { path = 'b' }
942989
"#,
943990
)
@@ -946,9 +993,19 @@ fn unexpected_cfgs_target_diagnostics() {
946993
.file("b/src/lib.rs", "")
947994
.build();
948995

949-
p.cargo("check -Zcheck-target-cfgs")
996+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
950997
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
951998
.with_stderr_data(str![[r#"
999+
[WARNING] unexpected `cfg` condition name: foo in `[target.'cfg(all(foo, bar))']`
1000+
= [HELP] occurred in `[ROOT]/foo/Cargo.toml`
1001+
[WARNING] unexpected `cfg` condition name: bar in `[target.'cfg(all(foo, bar))']`
1002+
= [HELP] occurred in `[ROOT]/foo/Cargo.toml`
1003+
[WARNING] unexpected `cfg` condition value: (none) for `target_pointer_width`
1004+
--> Cargo.toml:8:25
1005+
|
1006+
8 | [target."cfg(target_pointer_width)".dependencies] # expect (none) as value
1007+
| ---------------------------
1008+
|
9521009
[LOCKING] 1 package to latest compatible version
9531010
[CHECKING] a v0.0.1 ([ROOT]/foo)
9541011
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -984,9 +1041,8 @@ fn unexpected_cfgs_target_lint_level_allow() {
9841041
.file("b/src/lib.rs", "")
9851042
.build();
9861043

987-
p.cargo("check -Zcheck-target-cfgs")
1044+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
9881045
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
989-
// FIXME: We should warn on multiple cfgs
9901046
.with_stderr_data(str![[r#"
9911047
[LOCKING] 1 package to latest compatible version
9921048
[CHECKING] a v0.0.1 ([ROOT]/foo)
@@ -1023,16 +1079,18 @@ fn unexpected_cfgs_target_lint_level_deny() {
10231079
.file("b/src/lib.rs", "")
10241080
.build();
10251081

1026-
p.cargo("check -Zcheck-target-cfgs")
1082+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
10271083
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
10281084
.with_stderr_data(str![[r#"
1029-
[LOCKING] 1 package to latest compatible version
1030-
[CHECKING] a v0.0.1 ([ROOT]/foo)
1031-
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
1085+
[ERROR] unexpected `cfg` condition name: foo
1086+
--> Cargo.toml:8:25
1087+
|
1088+
8 | [target."cfg(foo)".dependencies]
1089+
| ^^^^^^^^^^
1090+
|
10321091
10331092
"#]])
1034-
// FIXME: this test should fail
1035-
// .with_status(101)
1093+
.with_status(101)
10361094
.run();
10371095
}
10381096

@@ -1061,7 +1119,7 @@ fn unexpected_cfgs_target_cfg_any() {
10611119
.file("b/src/lib.rs", "")
10621120
.build();
10631121

1064-
p.cargo("check -Zcheck-target-cfgs")
1122+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
10651123
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
10661124
.with_stderr_data(str![[r#"
10671125
[LOCKING] 1 package to latest compatible version
@@ -1116,9 +1174,6 @@ fn no_unexpected_cfgs_target() {
11161174
p.cargo("check -Zcargo-lints")
11171175
.masquerade_as_nightly_cargo(&["requires -Zcargo-lints"])
11181176
.with_stderr_data(str![[r#"
1119-
[LOCKING] 1 package to latest compatible version
1120-
[CHECKING] b v0.0.1 ([ROOT]/foo/b)
1121-
[CHECKING] a v0.0.1 ([ROOT]/foo)
11221177
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
11231178
11241179
"#]])

0 commit comments

Comments
 (0)