Skip to content

Commit b9958a3

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

File tree

4 files changed

+220
-17
lines changed

4 files changed

+220
-17
lines changed

‎src/cargo/core/workspace.rs

Lines changed: 2 additions & 1 deletion
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

Lines changed: 130 additions & 1 deletion
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) {
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

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

17811781
```sh
1782-
cargo check -Zcheck-target-cfgs
1782+
cargo check -Zcargo-lints -Zcheck-target-cfgs
17831783
```
17841784

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

‎tests/testsuite/cfg.rs

Lines changed: 87 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -564,10 +564,39 @@ fn unexpected_cfgs_target() {
564564
.file("c/src/lib.rs", "")
565565
.build();
566566

567-
p.cargo("check -Zcheck-target-cfgs")
567+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
568568
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
569-
// FIXME: We should warn on multiple cfgs
570569
.with_stderr_data(str![[r#"
570+
[WARNING] unexpected `cfg` condition name: foo
571+
--> Cargo.toml:11:25
572+
|
573+
11 | [target."cfg(any(foo, all(bar)))".dependencies]
574+
| -------------------------
575+
|
576+
[WARNING] unexpected `cfg` condition name: bar
577+
--> Cargo.toml:11:25
578+
|
579+
11 | [target."cfg(any(foo, all(bar)))".dependencies]
580+
| -------------------------
581+
|
582+
[WARNING] unexpected `cfg` condition value: `` for `windows = ""`
583+
--> Cargo.toml:18:25
584+
|
585+
18 | [target.'cfg(not(windows = ""))'.dependencies]
586+
| ------------------------
587+
|
588+
[WARNING] unexpected `cfg` condition value: `zoo` for `unix = "zoo"`
589+
--> Cargo.toml:14:25
590+
|
591+
14 | [target.'cfg(unix = "zoo")'.dependencies]
592+
| -------------------
593+
|
594+
[WARNING] unexpected `cfg` condition value: `zoo` for `unix = "zoo"`
595+
--> Cargo.toml:14:25
596+
|
597+
14 | [target.'cfg(unix = "zoo")'.dependencies]
598+
| -------------------
599+
|
571600
[LOCKING] 2 packages to latest compatible versions
572601
[CHECKING] b v0.0.1 ([ROOT]/foo/b)
573602
[CHECKING] a v0.0.1 ([ROOT]/foo)
@@ -614,10 +643,28 @@ fn unexpected_cfgs_target_with_lint() {
614643
.file("b/src/lib.rs", "")
615644
.build();
616645

617-
p.cargo("check -Zcheck-target-cfgs")
646+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
618647
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
619-
// FIXME: We should warn on multiple cfgs
648+
// FIXME: We should not warn on `cfg(foo = "foo")` but we currently do
620649
.with_stderr_data(str![[r#"
650+
[WARNING] unexpected `cfg` condition name: bar
651+
--> Cargo.toml:14:25
652+
|
653+
14 | [target."cfg(bar)".dependencies]
654+
| ----------
655+
|
656+
[WARNING] unexpected `cfg` condition name: foo for `foo = "foo"`
657+
--> Cargo.toml:11:25
658+
|
659+
11 | [target.'cfg(foo = "foo")'.dependencies] # should not warn here
660+
| ------------------
661+
|
662+
[WARNING] unexpected `cfg` condition name: foo
663+
--> Cargo.toml:8:25
664+
|
665+
8 | [target."cfg(foo)".dependencies] # should not warn here
666+
| ----------
667+
|
621668
[LOCKING] 1 package to latest compatible version
622669
[CHECKING] a v0.0.1 ([ROOT]/foo)
623670
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -638,10 +685,10 @@ fn unexpected_cfgs_target_diagnostics() {
638685
edition = "2015"
639686
authors = []
640687
641-
[target."cfg(target_pointer_width)".dependencies]
688+
[target."cfg(target_pointer_width)".dependencies] # expect (none) as value
642689
b = { path = 'b' }
643690
644-
[target.'cfg( all(foo , bar))'.dependencies]
691+
[target.'cfg( all(foo , bar))'.dependencies] # no snippet due to weird formatting
645692
b = { path = 'b' }
646693
"#,
647694
)
@@ -650,9 +697,19 @@ fn unexpected_cfgs_target_diagnostics() {
650697
.file("b/src/lib.rs", "")
651698
.build();
652699

653-
p.cargo("check -Zcheck-target-cfgs")
700+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
654701
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
655702
.with_stderr_data(str![[r#"
703+
[WARNING] unexpected `cfg` condition name: foo in `[target.'cfg(all(foo, bar))']`
704+
= [HELP] occurred in `[ROOT]/foo/Cargo.toml`
705+
[WARNING] unexpected `cfg` condition name: bar in `[target.'cfg(all(foo, bar))']`
706+
= [HELP] occurred in `[ROOT]/foo/Cargo.toml`
707+
[WARNING] unexpected `cfg` condition value: (none) for `target_pointer_width`
708+
--> Cargo.toml:8:25
709+
|
710+
8 | [target."cfg(target_pointer_width)".dependencies] # expect (none) as value
711+
| ---------------------------
712+
|
656713
[LOCKING] 1 package to latest compatible version
657714
[CHECKING] a v0.0.1 ([ROOT]/foo)
658715
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -685,10 +742,16 @@ fn unexpected_cfgs_target_lint_level_allow() {
685742
.file("b/src/lib.rs", "")
686743
.build();
687744

688-
p.cargo("check -Zcheck-target-cfgs")
745+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
689746
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
690-
// FIXME: We should warn on multiple cfgs
747+
// FIXME: We shouldn't warn any target cfgs because of the level="allow"
691748
.with_stderr_data(str![[r#"
749+
[WARNING] unexpected `cfg` condition name: foo
750+
--> Cargo.toml:8:25
751+
|
752+
8 | [target."cfg(foo)".dependencies]
753+
| ----------
754+
|
692755
[LOCKING] 1 package to latest compatible version
693756
[CHECKING] a v0.0.1 ([ROOT]/foo)
694757
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -721,9 +784,15 @@ fn unexpected_cfgs_target_lint_level_deny() {
721784
.file("b/src/lib.rs", "")
722785
.build();
723786

724-
p.cargo("check -Zcheck-target-cfgs")
787+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
725788
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
726789
.with_stderr_data(str![[r#"
790+
[WARNING] unexpected `cfg` condition name: foo
791+
--> Cargo.toml:8:25
792+
|
793+
8 | [target."cfg(foo)".dependencies]
794+
| ----------
795+
|
727796
[LOCKING] 1 package to latest compatible version
728797
[CHECKING] a v0.0.1 ([ROOT]/foo)
729798
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -759,9 +828,16 @@ fn unexpected_cfgs_target_cfg_any() {
759828
.file("b/src/lib.rs", "")
760829
.build();
761830

762-
p.cargo("check -Zcheck-target-cfgs")
831+
p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
763832
.masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
833+
// FIXME: We shouldn't be linting `cfg(foo)` because of the `cfg(any())`
764834
.with_stderr_data(str![[r#"
835+
[WARNING] unexpected `cfg` condition name: foo
836+
--> Cargo.toml:8:25
837+
|
838+
8 | [target."cfg(foo)".dependencies]
839+
| ----------
840+
|
765841
[LOCKING] 1 package to latest compatible version
766842
[CHECKING] a v0.0.1 ([ROOT]/foo)
767843
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -814,9 +890,6 @@ fn no_unexpected_cfgs_target() {
814890
p.cargo("check -Zcargo-lints")
815891
.masquerade_as_nightly_cargo(&["requires -Zcargo-lints"])
816892
.with_stderr_data(str![[r#"
817-
[LOCKING] 1 package to latest compatible version
818-
[CHECKING] b v0.0.1 ([ROOT]/foo/b)
819-
[CHECKING] a v0.0.1 ([ROOT]/foo)
820893
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
821894
822895
"#]])

0 commit comments

Comments
 (0)