Rollup merge of rust-lang#125412 - Urgau:check-cfg-less-build-rs, r=w…

…esleywiser Don't suggest adding the unexpected cfgs to the build-script it-self This PR adds a check to avoid suggesting to add the unexpected cfgs inside the build-script when building the build-script it-self, as it won't have any effect, since build-scripts applies to their descended target. Fixes rust-lang#125368
GuillaumeGomez · May 23, 2024 · 56427d3 · 56427d3
2 parents 1e4bde1 + 1c7859e
commit 56427d3
Show file tree

Hide file tree

Showing 4 changed files with 107 additions and 24 deletions.
diff --git a/compiler/rustc_lint/src/context/diagnostics/check_cfg.rs b/compiler/rustc_lint/src/context/diagnostics/check_cfg.rs
@@ -42,6 +42,22 @@ fn to_check_cfg_arg(name: Symbol, value: Option<Symbol>, quotes: EscapeQuotes) -
     }
 }
 
+fn cargo_help_sub(
+    sess: &Session,
+    inst: &impl Fn(EscapeQuotes) -> String,
+) -> lints::UnexpectedCfgCargoHelp {
+    // We don't want to suggest the `build.rs` way to expected cfgs if we are already in a
+    // `build.rs`. We therefor do a best effort check (looking if the `--crate-name` is
+    // `build_script_build`) to try to figure out if we are building a Cargo build script
+
+    let unescaped = &inst(EscapeQuotes::No);
+    if matches!(&sess.opts.crate_name, Some(crate_name) if crate_name == "build_script_build") {
+        lints::UnexpectedCfgCargoHelp::lint_cfg(unescaped)
+    } else {
+        lints::UnexpectedCfgCargoHelp::lint_cfg_and_build_rs(unescaped, &inst(EscapeQuotes::Yes))
+    }
+}
+
 pub(super) fn unexpected_cfg_name(
     sess: &Session,
     (name, name_span): (Symbol, Span),
@@ -162,14 +178,7 @@ pub(super) fn unexpected_cfg_name(
     let inst = |escape_quotes| to_check_cfg_arg(name, value.map(|(v, _s)| v), escape_quotes);
 
     let invocation_help = if is_from_cargo {
-        let sub = if !is_feature_cfg {
-            Some(lints::UnexpectedCfgCargoHelp::new(
-                &inst(EscapeQuotes::No),
-                &inst(EscapeQuotes::Yes),
-            ))
-        } else {
-            None
-        };
+        let sub = if !is_feature_cfg { Some(cargo_help_sub(sess, &inst)) } else { None };
         lints::unexpected_cfg_name::InvocationHelp::Cargo { sub }
     } else {
         lints::unexpected_cfg_name::InvocationHelp::Rustc(lints::UnexpectedCfgRustcHelp::new(
@@ -267,10 +276,7 @@ pub(super) fn unexpected_cfg_value(
                 Some(lints::unexpected_cfg_value::CargoHelp::DefineFeatures)
             }
         } else if !is_cfg_a_well_know_name {
-            Some(lints::unexpected_cfg_value::CargoHelp::Other(lints::UnexpectedCfgCargoHelp::new(
-                &inst(EscapeQuotes::No),
-                &inst(EscapeQuotes::Yes),
-            )))
+            Some(lints::unexpected_cfg_value::CargoHelp::Other(cargo_help_sub(sess, &inst)))
         } else {
             None
         };

diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs
@@ -1962,21 +1962,33 @@ pub struct UnitBindingsDiag {
 pub struct BuiltinNamedAsmLabel;
 
 #[derive(Subdiagnostic)]
-#[help(lint_unexpected_cfg_add_cargo_feature)]
-#[help(lint_unexpected_cfg_add_cargo_toml_lint_cfg)]
-#[help(lint_unexpected_cfg_add_build_rs_println)]
-pub struct UnexpectedCfgCargoHelp {
-    pub build_rs_println: String,
-    pub cargo_toml_lint_cfg: String,
+pub enum UnexpectedCfgCargoHelp {
+    #[help(lint_unexpected_cfg_add_cargo_feature)]
+    #[help(lint_unexpected_cfg_add_cargo_toml_lint_cfg)]
+    LintCfg { cargo_toml_lint_cfg: String },
+    #[help(lint_unexpected_cfg_add_cargo_feature)]
+    #[help(lint_unexpected_cfg_add_cargo_toml_lint_cfg)]
+    #[help(lint_unexpected_cfg_add_build_rs_println)]
+    LintCfgAndBuildRs { cargo_toml_lint_cfg: String, build_rs_println: String },
 }
 
 impl UnexpectedCfgCargoHelp {
-    pub fn new(unescaped: &str, escaped: &str) -> Self {
-        Self {
-            cargo_toml_lint_cfg: format!(
-                "\n [lints.rust]\n unexpected_cfgs = {{ level = \"warn\", check-cfg = ['{unescaped}'] }}",
-            ),
-            build_rs_println: format!("println!(\"cargo::rustc-check-cfg={escaped}\");",),
+    fn cargo_toml_lint_cfg(unescaped: &str) -> String {
+        format!(
+            "\n [lints.rust]\n unexpected_cfgs = {{ level = \"warn\", check-cfg = ['{unescaped}'] }}"
+        )
+    }
+
+    pub fn lint_cfg(unescaped: &str) -> Self {
+        UnexpectedCfgCargoHelp::LintCfg {
+            cargo_toml_lint_cfg: Self::cargo_toml_lint_cfg(unescaped),
+        }
+    }
+
+    pub fn lint_cfg_and_build_rs(unescaped: &str, escaped: &str) -> Self {
+        UnexpectedCfgCargoHelp::LintCfgAndBuildRs {
+            cargo_toml_lint_cfg: Self::cargo_toml_lint_cfg(unescaped),
+            build_rs_println: format!("println!(\"cargo::rustc-check-cfg={escaped}\");"),
         }
     }
 }

diff --git a/tests/ui/check-cfg/cargo-build-script.rs b/tests/ui/check-cfg/cargo-build-script.rs
@@ -0,0 +1,22 @@
+// This test checks that when we are building a build script provided
+// by Cargo we only suggest expecting the unexpected cfgs in the Cargo.toml.
+//
+//@ check-pass
+//@ no-auto-check-cfg
+//@ rustc-env:CARGO_CRATE_NAME=build_script_build
+//@ compile-flags:--crate-name=build_script_build
+//@ compile-flags:--check-cfg=cfg(has_bar)
+
+#[cfg(has_foo)]
+//~^ WARNING unexpected `cfg` condition name
+fn foo() {}
+
+#[cfg(has_foo = "yes")]
+//~^ WARNING unexpected `cfg` condition name
+fn foo() {}
+
+#[cfg(has_bar = "yes")]
+//~^ WARNING unexpected `cfg` condition value
+fn has_bar() {}
+
+fn main() {}
diff --git a/tests/ui/check-cfg/cargo-build-script.stderr b/tests/ui/check-cfg/cargo-build-script.stderr
@@ -0,0 +1,43 @@
+warning: unexpected `cfg` condition name: `has_foo`
+  --> $DIR/cargo-build-script.rs:10:7
+   |
+LL | #[cfg(has_foo)]
+   |       ^^^^^^^
+   |
+   = help: expected names are: `clippy`, `debug_assertions`, `doc`, `doctest`, `has_bar`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `rustfmt`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `ub_checks`, `unix`, and `windows`
+   = help: consider using a Cargo feature instead
+   = help: or consider adding in `Cargo.toml` the `check-cfg` lint config for the lint:
+            [lints.rust]
+            unexpected_cfgs = { level = "warn", check-cfg = ['cfg(has_foo)'] }
+   = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
+   = note: `#[warn(unexpected_cfgs)]` on by default
+
+warning: unexpected `cfg` condition name: `has_foo`
+  --> $DIR/cargo-build-script.rs:14:7
+   |
+LL | #[cfg(has_foo = "yes")]
+   |       ^^^^^^^^^^^^^^^
+   |
+   = help: consider using a Cargo feature instead
+   = help: or consider adding in `Cargo.toml` the `check-cfg` lint config for the lint:
+            [lints.rust]
+            unexpected_cfgs = { level = "warn", check-cfg = ['cfg(has_foo, values("yes"))'] }
+   = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
+
+warning: unexpected `cfg` condition value: `yes`
+  --> $DIR/cargo-build-script.rs:18:7
+   |
+LL | #[cfg(has_bar = "yes")]
+   |       ^^^^^^^--------
+   |              |
+   |              help: remove the value
+   |
+   = note: no expected value for `has_bar`
+   = help: consider using a Cargo feature instead
+   = help: or consider adding in `Cargo.toml` the `check-cfg` lint config for the lint:
+            [lints.rust]
+            unexpected_cfgs = { level = "warn", check-cfg = ['cfg(has_bar, values("yes"))'] }
+   = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
+
+warning: 3 warnings emitted
+