Skip to content

Commit 592ab9e

Browse files
UrgauUrgau
committed
Lint against some unexpected target cfgs
with the help of the Cargo lints infra
1 parent 0ecf43a commit 592ab9e

File tree

4 files changed

+220
-17
lines changed

4 files changed

+220
-17
lines changed

‎src/cargo/core/workspace.rs

+2-1
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, &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

+130-1
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
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;
@@ -605,6 +608,132 @@ fn output_unknown_lints(
605608
Ok(())
606609
}
607610

611+
pub fn unexpected_target_cfgs(
612+
ws: &Workspace<'_>,
613+
pkg: &Package,
614+
path: &Path,
615+
error_count: &mut usize,
616+
gctx: &GlobalContext,
617+
) -> CargoResult<()> {
618+
let manifest = pkg.manifest();
619+
620+
// FIXME: We should get the lint level from `[lints.rust.unexpected_cfgs]`
621+
let lint_level = LintLevel::Warn;
622+
623+
let rustc = gctx.load_global_rustc(Some(ws))?;
624+
// FIXME: While it doesn't doesn't really matter for `--print=check-cfg`, we should
625+
// still pass the actual requested targets instead of an empty array so that the
626+
// target info can always be de-duplicated.
627+
// FIXME: Move the target info creation before any linting is done and re-use it for
628+
// all the packages.
629+
let compile_kinds = CompileKind::from_requested_targets(gctx, &[])?;
630+
let target_info = TargetInfo::new(gctx, &compile_kinds, &rustc, CompileKind::Host)?;
631+
632+
let Some(global_check_cfg) = target_info.check_cfg() else {
633+
return Ok(());
634+
};
635+
636+
if !global_check_cfg.exhaustive {
637+
return Ok(());
638+
}
639+
640+
// FIXME: If the `[lints.rust.unexpected_cfgs.check-cfg]` config is set we should
641+
// re-fetch the check-cfg informations with those extra args
642+
643+
for dep in pkg.summary().dependencies() {
644+
let Some(platform) = dep.platform() else {
645+
continue;
646+
};
647+
let Platform::Cfg(cfg_expr) = platform else {
648+
continue;
649+
};
650+
651+
cfg_expr.walk(|cfg| -> CargoResult<()> {
652+
let (name, value) = match cfg {
653+
Cfg::Name(name) => (name, None),
654+
Cfg::KeyPair(name, value) => (name, Some(value.to_string())),
655+
};
656+
657+
match global_check_cfg.expecteds.get(name.as_str()) {
658+
Some(ExpectedValues::Some(values)) if !values.contains(&value) => {
659+
let level = lint_level.to_diagnostic_level();
660+
if lint_level == LintLevel::Forbid || lint_level == LintLevel::Deny {
661+
*error_count += 1;
662+
}
663+
664+
let value = match value {
665+
Some(value) => Cow::from(format!("`{value}`")),
666+
None => Cow::from("(none)"),
667+
};
668+
669+
// Retrieving the span can fail if our pretty-priting doesn't match what the
670+
// user wrote: `unix="foo"` and `unix = "foo"`, for that reason we don't fail
671+
// and just produce a slightly worth diagnostic.
672+
if let Some(span) = get_span(manifest.document(), &["target", &*format!("cfg({cfg_expr})")], false) {
673+
let manifest_path = rel_cwd_manifest_path(path, gctx);
674+
let title = format!(
675+
"unexpected `cfg` condition value: {value} for `{cfg}`"
676+
);
677+
let diag = level.title(&*title).snippet(
678+
Snippet::source(manifest.contents())
679+
.origin(&manifest_path)
680+
.annotation(level.span(span))
681+
.fold(true)
682+
);
683+
gctx.shell().print_message(diag)?;
684+
} else {
685+
let title = format!(
686+
"unexpected `cfg` condition value: {value} for `{cfg}` in `[target.'cfg({cfg_expr})']`"
687+
);
688+
let help_where = format!("occurred in `{path}`", path = path.display());
689+
let diag = level.title(&*title).footer(Level::Help.title(&*help_where));
690+
gctx.shell().print_message(diag)?;
691+
}
692+
}
693+
None => {
694+
let level = lint_level.to_diagnostic_level();
695+
if lint_level == LintLevel::Forbid || lint_level == LintLevel::Deny {
696+
*error_count += 1;
697+
}
698+
699+
let for_cfg = match value {
700+
Some(_value) => Cow::from(format!(" for `{cfg}`")),
701+
None => Cow::from(""),
702+
};
703+
704+
// Retrieving the span can fail if our pretty-priting doesn't match what the
705+
// user wrote: `unix="foo"` and `unix = "foo"`, for that reason we don't fail
706+
// and just produce a slightly worth diagnostic.
707+
if let Some(span) = get_span(manifest.document(), &["target", &*format!("cfg({cfg_expr})")], false) {
708+
let manifest_path = rel_cwd_manifest_path(path, gctx);
709+
let title = format!(
710+
"unexpected `cfg` condition name: {name}{for_cfg}"
711+
);
712+
let diag = level.title(&*title).snippet(
713+
Snippet::source(manifest.contents())
714+
.origin(&manifest_path)
715+
.annotation(level.span(span))
716+
.fold(true)
717+
);
718+
gctx.shell().print_message(diag)?;
719+
} else {
720+
let title = format!(
721+
"unexpected `cfg` condition name: {name}{for_cfg} in `[target.'cfg({cfg_expr})']`"
722+
);
723+
let help_where = format!("occurred in `{path}`", path = path.display());
724+
let diag = level.title(&*title).footer(Level::Help.title(&*help_where));
725+
gctx.shell().print_message(diag)?;
726+
}
727+
}
728+
_ => { /* not unexpected */ }
729+
}
730+
Ok(())
731+
})?;
732+
}
733+
734+
Ok(())
735+
}
736+
608737
#[cfg(test)]
609738
mod tests {
610739
use itertools::Itertools;

‎src/doc/src/reference/unstable.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -1925,7 +1925,7 @@ This feature checks for unexpected cfgs in `[target.'cfg(...)']` entries, based
19251925
on `rustc --print=check-cfg`.
19261926

19271927
```sh
1928-
cargo check -Zcheck-target-cfgs
1928+
cargo check -Zcargo-lints -Zcheck-target-cfgs
19291929
```
19301930

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

‎tests/testsuite/cfg.rs

+87-14
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
@@ -981,10 +1038,16 @@ fn unexpected_cfgs_target_lint_level_allow() {
9811038
.file("b/src/lib.rs", "")
9821039
.build();
9831040

984-
p.cargo("check -Zcheck-target-cfgs")
1041+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
9851042
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
986-
// FIXME: We should warn on multiple cfgs
1043+
// FIXME: We shouldn't warn any target cfgs because of the level="allow"
9871044
.with_stderr_data(str![[r#"
1045+
[WARNING] unexpected `cfg` condition name: foo
1046+
--> Cargo.toml:8:25
1047+
|
1048+
8 | [target."cfg(foo)".dependencies]
1049+
| ----------
1050+
|
9881051
[LOCKING] 1 package to latest compatible version
9891052
[CHECKING] a v0.0.1 ([ROOT]/foo)
9901053
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -1017,9 +1080,15 @@ fn unexpected_cfgs_target_lint_level_deny() {
10171080
.file("b/src/lib.rs", "")
10181081
.build();
10191082

1020-
p.cargo("check -Zcheck-target-cfgs")
1083+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
10211084
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
10221085
.with_stderr_data(str![[r#"
1086+
[WARNING] unexpected `cfg` condition name: foo
1087+
--> Cargo.toml:8:25
1088+
|
1089+
8 | [target."cfg(foo)".dependencies]
1090+
| ----------
1091+
|
10231092
[LOCKING] 1 package to latest compatible version
10241093
[CHECKING] a v0.0.1 ([ROOT]/foo)
10251094
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -1055,9 +1124,16 @@ fn unexpected_cfgs_target_cfg_any() {
10551124
.file("b/src/lib.rs", "")
10561125
.build();
10571126

1058-
p.cargo("check -Zcheck-target-cfgs")
1127+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
10591128
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
1129+
// FIXME: We shouldn't be linting `cfg(foo)` because of the `cfg(any())`
10601130
.with_stderr_data(str![[r#"
1131+
[WARNING] unexpected `cfg` condition name: foo
1132+
--> Cargo.toml:8:25
1133+
|
1134+
8 | [target."cfg(foo)".dependencies]
1135+
| ----------
1136+
|
10611137
[LOCKING] 1 package to latest compatible version
10621138
[CHECKING] a v0.0.1 ([ROOT]/foo)
10631139
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -1110,9 +1186,6 @@ fn no_unexpected_cfgs_target() {
11101186
p.cargo("check -Zcargo-lints")
11111187
.masquerade_as_nightly_cargo(&["requires -Zcargo-lints"])
11121188
.with_stderr_data(str![[r#"
1113-
[LOCKING] 1 package to latest compatible version
1114-
[CHECKING] b v0.0.1 ([ROOT]/foo/b)
1115-
[CHECKING] a v0.0.1 ([ROOT]/foo)
11161189
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
11171190
11181191
"#]])

0 commit comments

Comments
 (0)