From 1c7859e117b77134650663b4c470dcfea7fe1468 Mon Sep 17 00:00:00 2001 From: Urgau Date: Wed, 22 May 2024 13:27:35 +0200 Subject: [PATCH] Don't suggest adding the unexpected cfgs the build-script it-self --- .../src/context/diagnostics/check_cfg.rs | 30 +++++++------ compiler/rustc_lint/src/lints.rs | 36 ++++++++++------ tests/ui/check-cfg/cargo-build-script.rs | 22 ++++++++++ tests/ui/check-cfg/cargo-build-script.stderr | 43 +++++++++++++++++++ 4 files changed, 107 insertions(+), 24 deletions(-) create mode 100644 tests/ui/check-cfg/cargo-build-script.rs create mode 100644 tests/ui/check-cfg/cargo-build-script.stderr diff --git a/compiler/rustc_lint/src/context/diagnostics/check_cfg.rs b/compiler/rustc_lint/src/context/diagnostics/check_cfg.rs index 020ca1753cf..c69e680cb64 100644 --- 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, 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 index 3bd6faca379..8339107a16b 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1961,21 +1961,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 new file mode 100644 index 00000000000..a3ba8791441 --- /dev/null +++ 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 new file mode 100644 index 00000000000..9ab3290ef22 --- /dev/null +++ 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 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 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 for more information about checking conditional configuration + +warning: 3 warnings emitted +