Skip to content

Commit dfc28f4

Browse files
UrgauUrgau
committed
Lint against some unexpected target cfgs
with the help of the Cargo lints infra
1 parent 02b2621 commit dfc28f4

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
@@ -24,7 +24,7 @@ use crate::sources::{PathSource, SourceConfigMap, CRATES_IO_INDEX, CRATES_IO_REG
2424
use crate::util::edit_distance;
2525
use crate::util::errors::{CargoResult, ManifestError};
2626
use crate::util::interning::InternedString;
27-
use crate::util::lints::{analyze_cargo_lints_table, check_im_a_teapot};
27+
use crate::util::lints::{analyze_cargo_lints_table, check_im_a_teapot, unexpected_target_cfgs};
2828
use crate::util::toml::{read_manifest, InheritableFields};
2929
use crate::util::{
3030
context::CargoResolverConfig, context::ConfigRelativePath, context::IncompatibleRustVersions,
@@ -1219,6 +1219,7 @@ impl<'gctx> Workspace<'gctx> {
12191219
self.gctx,
12201220
)?;
12211221
check_im_a_teapot(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
1222+
unexpected_target_cfgs(self, pkg, &path, &mut error_count, self.gctx)?;
12221223
if error_count > 0 {
12231224
Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
12241225
"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
@@ -1752,7 +1752,7 @@ This feature checks for unexpected cfgs in `[target.'cfg(...)']` entries, based
17521752
on `rustc --print=check-cfg`.
17531753

17541754
```sh
1755-
cargo check -Zcheck-target-cfgs
1755+
cargo check -Zcargo-lints -Zcheck-target-cfgs
17561756
```
17571757

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

‎tests/testsuite/cfg.rs

+87-14
Original file line numberDiff line numberDiff line change
@@ -702,10 +702,39 @@ fn unexpected_cfgs_target() {
702702
.file("c/src/lib.rs", "")
703703
.build();
704704

705-
p.cargo("check -Zcheck-target-cfgs")
705+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
706706
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
707-
// FIXME: We should warn on multiple cfgs
708707
.with_stderr_data(str![[r#"
708+
[WARNING] unexpected `cfg` condition name: foo
709+
--> Cargo.toml:11:25
710+
|
711+
11 | [target."cfg(any(foo, all(bar)))".dependencies]
712+
| -------------------------
713+
|
714+
[WARNING] unexpected `cfg` condition name: bar
715+
--> Cargo.toml:11:25
716+
|
717+
11 | [target."cfg(any(foo, all(bar)))".dependencies]
718+
| -------------------------
719+
|
720+
[WARNING] unexpected `cfg` condition value: `` for `windows = ""`
721+
--> Cargo.toml:18:25
722+
|
723+
18 | [target.'cfg(not(windows = ""))'.dependencies]
724+
| ------------------------
725+
|
726+
[WARNING] unexpected `cfg` condition value: `zoo` for `unix = "zoo"`
727+
--> Cargo.toml:14:25
728+
|
729+
14 | [target.'cfg(unix = "zoo")'.dependencies]
730+
| -------------------
731+
|
732+
[WARNING] unexpected `cfg` condition value: `zoo` for `unix = "zoo"`
733+
--> Cargo.toml:14:25
734+
|
735+
14 | [target.'cfg(unix = "zoo")'.dependencies]
736+
| -------------------
737+
|
709738
[LOCKING] 2 packages to latest compatible versions
710739
[CHECKING] b v0.0.1 ([ROOT]/foo/b)
711740
[CHECKING] a v0.0.1 ([ROOT]/foo)
@@ -752,10 +781,28 @@ fn unexpected_cfgs_target_with_lint() {
752781
.file("b/src/lib.rs", "")
753782
.build();
754783

755-
p.cargo("check -Zcheck-target-cfgs")
784+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
756785
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
757-
// FIXME: We should warn on multiple cfgs
786+
// FIXME: We should not warn on `cfg(foo = "foo")` but we currently do
758787
.with_stderr_data(str![[r#"
788+
[WARNING] unexpected `cfg` condition name: bar
789+
--> Cargo.toml:14:25
790+
|
791+
14 | [target."cfg(bar)".dependencies]
792+
| ----------
793+
|
794+
[WARNING] unexpected `cfg` condition name: foo for `foo = "foo"`
795+
--> Cargo.toml:11:25
796+
|
797+
11 | [target.'cfg(foo = "foo")'.dependencies] # should not warn here
798+
| ------------------
799+
|
800+
[WARNING] unexpected `cfg` condition name: foo
801+
--> Cargo.toml:8:25
802+
|
803+
8 | [target."cfg(foo)".dependencies] # should not warn here
804+
| ----------
805+
|
759806
[LOCKING] 1 package to latest compatible version
760807
[CHECKING] a v0.0.1 ([ROOT]/foo)
761808
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -776,10 +823,10 @@ fn unexpected_cfgs_target_diagnostics() {
776823
edition = "2015"
777824
authors = []
778825
779-
[target."cfg(target_pointer_width)".dependencies]
826+
[target."cfg(target_pointer_width)".dependencies] # expect (none) as value
780827
b = { path = 'b' }
781828
782-
[target.'cfg( all(foo , bar))'.dependencies]
829+
[target.'cfg( all(foo , bar))'.dependencies] # no snippet due to weird formatting
783830
b = { path = 'b' }
784831
"#,
785832
)
@@ -788,9 +835,19 @@ fn unexpected_cfgs_target_diagnostics() {
788835
.file("b/src/lib.rs", "")
789836
.build();
790837

791-
p.cargo("check -Zcheck-target-cfgs")
838+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
792839
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
793840
.with_stderr_data(str![[r#"
841+
[WARNING] unexpected `cfg` condition name: foo in `[target.'cfg(all(foo, bar))']`
842+
= [HELP] occurred in `[ROOT]/foo/Cargo.toml`
843+
[WARNING] unexpected `cfg` condition name: bar in `[target.'cfg(all(foo, bar))']`
844+
= [HELP] occurred in `[ROOT]/foo/Cargo.toml`
845+
[WARNING] unexpected `cfg` condition value: (none) for `target_pointer_width`
846+
--> Cargo.toml:8:25
847+
|
848+
8 | [target."cfg(target_pointer_width)".dependencies] # expect (none) as value
849+
| ---------------------------
850+
|
794851
[LOCKING] 1 package to latest compatible version
795852
[CHECKING] a v0.0.1 ([ROOT]/foo)
796853
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -823,10 +880,16 @@ fn unexpected_cfgs_target_lint_level_allow() {
823880
.file("b/src/lib.rs", "")
824881
.build();
825882

826-
p.cargo("check -Zcheck-target-cfgs")
883+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
827884
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
828-
// FIXME: We should warn on multiple cfgs
885+
// FIXME: We shouldn't warn any target cfgs because of the level="allow"
829886
.with_stderr_data(str![[r#"
887+
[WARNING] unexpected `cfg` condition name: foo
888+
--> Cargo.toml:8:25
889+
|
890+
8 | [target."cfg(foo)".dependencies]
891+
| ----------
892+
|
830893
[LOCKING] 1 package to latest compatible version
831894
[CHECKING] a v0.0.1 ([ROOT]/foo)
832895
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -859,9 +922,15 @@ fn unexpected_cfgs_target_lint_level_deny() {
859922
.file("b/src/lib.rs", "")
860923
.build();
861924

862-
p.cargo("check -Zcheck-target-cfgs")
925+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
863926
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
864927
.with_stderr_data(str![[r#"
928+
[WARNING] unexpected `cfg` condition name: foo
929+
--> Cargo.toml:8:25
930+
|
931+
8 | [target."cfg(foo)".dependencies]
932+
| ----------
933+
|
865934
[LOCKING] 1 package to latest compatible version
866935
[CHECKING] a v0.0.1 ([ROOT]/foo)
867936
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -897,9 +966,16 @@ fn unexpected_cfgs_target_cfg_any() {
897966
.file("b/src/lib.rs", "")
898967
.build();
899968

900-
p.cargo("check -Zcheck-target-cfgs")
969+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
901970
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
971+
// FIXME: We shouldn't be linting `cfg(foo)` because of the `cfg(any())`
902972
.with_stderr_data(str![[r#"
973+
[WARNING] unexpected `cfg` condition name: foo
974+
--> Cargo.toml:8:25
975+
|
976+
8 | [target."cfg(foo)".dependencies]
977+
| ----------
978+
|
903979
[LOCKING] 1 package to latest compatible version
904980
[CHECKING] a v0.0.1 ([ROOT]/foo)
905981
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -952,9 +1028,6 @@ fn no_unexpected_cfgs_target() {
9521028
p.cargo("check -Zcargo-lints")
9531029
.masquerade_as_nightly_cargo(&["requires -Zcargo-lints"])
9541030
.with_stderr_data(str![[r#"
955-
[LOCKING] 1 package to latest compatible version
956-
[CHECKING] b v0.0.1 ([ROOT]/foo/b)
957-
[CHECKING] a v0.0.1 ([ROOT]/foo)
9581031
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
9591032
9601033
"#]])

0 commit comments

Comments
 (0)