Lint against some unexpected target cfgs

with the help of the Cargo lints infra

Author: Urgau

src/cargo/core/workspace.rs

@@ -25,7 +25,8 @@ use crate::util::edit_distance;
 use crate::util::errors::{CargoResult, ManifestError};
 use crate::util::interning::InternedString;
 use crate::util::lints::{
-    analyze_cargo_lints_table, check_im_a_teapot, check_implicit_features, unused_dependencies,
+    analyze_cargo_lints_table, check_im_a_teapot, check_implicit_features, unexpected_target_cfgs,
+    unused_dependencies,
 };
 use crate::util::toml::{read_manifest, InheritableFields};
 use crate::util::{
@@ -1238,6 +1239,7 @@ impl<'gctx> Workspace<'gctx> {
         check_im_a_teapot(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
         check_implicit_features(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
         unused_dependencies(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
+        unexpected_target_cfgs(self, pkg, &path, &mut error_count, self.gctx)?;
         if error_count > 0 {
             Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
                 "encountered {error_count} errors(s) while running lints"

src/cargo/util/lints.rs

@@ -1,9 +1,11 @@
+use crate::core::compiler::{CompileKind, TargetInfo};
 use crate::core::dependency::DepKind;
 use crate::core::FeatureValue::Dep;
-use crate::core::{Edition, Feature, FeatureValue, Features, Manifest, Package};
+use crate::core::{Edition, Feature, FeatureValue, Features, Manifest, Package, Workspace};
 use crate::util::interning::InternedString;
 use crate::{CargoResult, GlobalContext};
 use annotate_snippets::{Level, Snippet};
+use cargo_platform::{Cfg, CfgExpr, CheckCfg, ExpectedValues, Platform};
 use cargo_util_schemas::manifest::{TomlLintLevel, TomlToolLints};
 use pathdiff::diff_paths;
 use std::collections::HashSet;
@@ -880,6 +882,111 @@ pub fn unused_dependencies(
     Ok(())
 }
 
+pub fn unexpected_target_cfgs(
+    ws: &Workspace<'_>,
+    pkg: &Package,
+    path: &Path,
+    error_count: &mut usize,
+    gctx: &GlobalContext,
+) -> CargoResult<()> {
+    fn warn_on_unexpected_cfgs(
+        gctx: &GlobalContext,
+        check_cfg: &CheckCfg,
+        cfg_expr: &CfgExpr,
+        lint_level: LintLevel,
+        error_count: &mut usize,
+        path: Option<&Path>,
+        suffix: &str,
+    ) -> CargoResult<()> {
+        let prefix = if let Some(path) = path {
+            format!("{path}: ", path = path.display())
+        } else {
+            "".to_string()
+        };
+
+        let mut emit_lint = |msg| match lint_level {
+            LintLevel::Warn => gctx.shell().warn(msg),
+            LintLevel::Deny | LintLevel::Forbid => {
+                *error_count += 1;
+                gctx.shell().error(msg)
+            }
+            LintLevel::Allow => Ok(()),
+        };
+
+        cfg_expr.try_for_each(|cfg| -> CargoResult<()> {
+            let (name, value) = match cfg {
+                Cfg::Name(name) => (name, None),
+                Cfg::KeyPair(name, value) => (name, Some(value.to_string())),
+            };
+
+            match check_cfg.expecteds.get(name) {
+                Some(ExpectedValues::Some(values)) if !values.contains(&value) => {
+                    let value = if let Some(ref value) = value {
+                        value
+                    } else {
+                        "(none)"
+                    };
+                    emit_lint(format!(
+                        "{prefix}unexpected `cfg` condition value: `{value}` for `{cfg}` in `[target.'cfg({cfg_expr})'{suffix}]`"
+                    ))?;
+                }
+                None => {
+                    emit_lint(format!(
+                        "{prefix}unexpected `cfg` condition name: `{name}`{cfg} in `[target.'cfg({cfg_expr})'{suffix}]`",
+                        cfg = if value.is_some() {
+                            format!(" for `{cfg}`")
+                        } else {
+                            format!("")
+                        }
+                    ))?;
+                }
+                _ => { /* not unexpected */ }
+            }
+            Ok(())
+        })
+    }
+
+    let rustc = gctx.load_global_rustc(Some(ws))?;
+    // FIXME: While it doesn't doesn't really matter for `--print=check-cfg`, wee should
+    // still pass the actual requested targets instead of an empty array so that the
+    // target info can be de-duplicated.
+    let compile_kinds = CompileKind::from_requested_targets(gctx, &[])?;
+    let target_info = TargetInfo::new(gctx, &compile_kinds, &rustc, CompileKind::Host)?;
+
+    let Some(global_check_cfg) = target_info.check_cfg() else {
+        return Ok(());
+    };
+
+    for dep in pkg.summary().dependencies() {
+        let Some(platform) = dep.platform() else {
+            continue;
+        };
+        let Platform::Cfg(cfg_expr) = platform else {
+            continue;
+        };
+
+        // FIXME: If the `[lints.rust.unexpected_cfgs.check-cfg]` config is set we should
+        // re-fetch the check-cfg informations with those extra args
+
+        if !global_check_cfg.exhaustive {
+            continue;
+        }
+
+        warn_on_unexpected_cfgs(
+            gctx,
+            &global_check_cfg,
+            cfg_expr,
+            // FIXME: We should get the lint level from `[lints.rust.unexpected_cfgs]`
+            LintLevel::Warn,
+            error_count,
+            Some(path),
+            ".dependencies",
+        )?;
+    }
+
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use itertools::Itertools;

src/doc/src/reference/unstable.md

@@ -1741,7 +1741,7 @@ This feature checks for unexpected cfgs in `[target.'cfg(...)']` entries, based
 on `rustc --print=check-cfg`.
 
 ```sh
-cargo check -Zcheck-target-cfgs
+cargo check -Zcargo-lints -Zcheck-target-cfgs
 ```
 
 It follows the lint Rust `unexpected_cfgs` lint configuration:

tests/testsuite/cfg.rs

@@ -564,10 +564,14 @@ fn unexpected_cfgs_target() {
         .file("c/src/lib.rs", "")
         .build();
 
-    p.cargo("check -Zcheck-target-cfgs")
+    p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
         .masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
-        // FIXME: We should warn on multiple cfgs
         .with_stderr_data(str![[r#"
+[WARNING] [ROOT]/foo/Cargo.toml: unexpected `cfg` condition name: `foo` in `[target.'cfg(any(foo, all(bar)))'.dependencies]`
+[WARNING] [ROOT]/foo/Cargo.toml: unexpected `cfg` condition name: `bar` in `[target.'cfg(any(foo, all(bar)))'.dependencies]`
+[WARNING] [ROOT]/foo/Cargo.toml: unexpected `cfg` condition value: `` for `windows = ""` in `[target.'cfg(not(windows = ""))'.dependencies]`
+[WARNING] [ROOT]/foo/Cargo.toml: unexpected `cfg` condition value: `zoo` for `unix = "zoo"` in `[target.'cfg(unix = "zoo")'.dependencies]`
+[WARNING] [ROOT]/foo/Cargo.toml: unexpected `cfg` condition value: `zoo` for `unix = "zoo"` in `[target.'cfg(unix = "zoo")'.dependencies]`
 [LOCKING] 2 packages to latest compatible versions
 [CHECKING] b v0.0.1 ([ROOT]/foo/b)
 [CHECKING] a v0.0.1 ([ROOT]/foo)
@@ -614,10 +618,13 @@ fn unexpected_cfgs_target_with_lint() {
         .file("b/src/lib.rs", "")
         .build();
 
-    p.cargo("check -Zcheck-target-cfgs")
+    p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
         .masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
-        // FIXME: We should warn on multiple cfgs
+        // FIXME: We should not warn on `cfg(foo = "foo")` but we currently do
         .with_stderr_data(str![[r#"
+[WARNING] [ROOT]/foo/Cargo.toml: unexpected `cfg` condition name: `bar` in `[target.'cfg(bar)'.dependencies]`
+[WARNING] [ROOT]/foo/Cargo.toml: unexpected `cfg` condition name: `foo` for `foo = "foo"` in `[target.'cfg(foo = "foo")'.dependencies]`
+[WARNING] [ROOT]/foo/Cargo.toml: unexpected `cfg` condition name: `foo` in `[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
@@ -650,10 +657,11 @@ fn unexpected_cfgs_target_lint_level_allow() {
         .file("b/src/lib.rs", "")
         .build();
 
-    p.cargo("check -Zcheck-target-cfgs")
+    p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
         .masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
-        // FIXME: We should warn on multiple cfgs
+        // FIXME: We shouldn't warn any target cfgs because of the level="allow"
         .with_stderr_data(str![[r#"
+[WARNING] [ROOT]/foo/Cargo.toml: unexpected `cfg` condition name: `foo` in `[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
@@ -686,9 +694,10 @@ fn unexpected_cfgs_target_lint_level_deny() {
         .file("b/src/lib.rs", "")
         .build();
 
-    p.cargo("check -Zcheck-target-cfgs")
+    p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
         .masquerade_as_nightly_cargo(&["requires -Zcheck-target-cfgs"])
         .with_stderr_data(str![[r#"
+[WARNING] [ROOT]/foo/Cargo.toml: unexpected `cfg` condition name: `foo` in `[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
@@ -724,9 +733,11 @@ fn unexpected_cfgs_target_cfg_any() {
         .file("b/src/lib.rs", "")
         .build();
 
-    p.cargo("check -Zcheck-target-cfgs")
+    p.cargo("check -Zcargo-lints -Zcheck-target-cfgs")
         .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] [ROOT]/foo/Cargo.toml: unexpected `cfg` condition name: `foo` in `[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