diff --git a/compiler/rustc_ast/src/attr/mod.rs b/compiler/rustc_ast/src/attr/mod.rs index 0374a86d3eb1c..fa8f3376f24d5 100644 --- a/compiler/rustc_ast/src/attr/mod.rs +++ b/compiler/rustc_ast/src/attr/mod.rs @@ -481,7 +481,7 @@ impl MetaItem { } } - fn from_tokens(iter: &mut TokenStreamIter<'_>) -> Option { + pub fn from_tokens(iter: &mut TokenStreamIter<'_>) -> Option { // FIXME: Share code with `parse_path`. let tt = iter.next().map(|tt| TokenTree::uninterpolate(tt)); let path = match tt.as_deref() { diff --git a/compiler/rustc_attr_parsing/src/attributes/cfg.rs b/compiler/rustc_attr_parsing/src/attributes/cfg.rs index 4cc07ceaf231f..2b455766b9f70 100644 --- a/compiler/rustc_attr_parsing/src/attributes/cfg.rs +++ b/compiler/rustc_attr_parsing/src/attributes/cfg.rs @@ -9,6 +9,7 @@ use rustc_feature::{ }; use rustc_hir::attrs::CfgEntry; use rustc_hir::{AttrPath, RustcVersion, Target}; +use rustc_lint_defs::builtin::MISLEADING_CFG_IN_BUILD_SCRIPT; use rustc_parse::parser::{ForceCollect, Parser, Recovery}; use rustc_parse::{exp, parse_in}; use rustc_session::Session; @@ -79,9 +80,148 @@ pub fn parse_cfg(cx: &mut AcceptContext<'_, '_>, args: &ArgParser) -> Option String { + generate_cfg_replacement_inner(entry, true, false) +} + +fn generate_cfg_replacement_inner( + entry: &CfgEntry, + is_top_level: bool, + parent_is_not: bool, +) -> String { + match entry { + CfgEntry::NameValue { name, value, .. } => { + let name = name.as_str(); + match value { + Some(value) => format!( + "{}std::env::var(\"CARGO_CFG_{}\").unwrap_or_default() == \"{value}\"", + if parent_is_not { "!" } else { "" }, + name.to_uppercase(), + ), + None => format!( + "std::env::var(\"CARGO_CFG_{}\"){}", + name.to_uppercase(), + if parent_is_not { ".is_err()" } else { ".is_ok()" }, + ), + } + } + CfgEntry::Any(entries, _) => match entries.as_slice() { + [] => if parent_is_not { "true" } else { "false" }.to_string(), + [entry] => generate_cfg_replacement_inner(&entry, is_top_level, parent_is_not), + _ => format!( + "{not}{open_paren}{cond}{closing_paren}", + not = if parent_is_not { "!" } else { "" }, + open_paren = if !parent_is_not && is_top_level { "" } else { "(" }, + cond = entries + .iter() + .map(|cfg| generate_cfg_replacement_inner(cfg, false, false)) + .collect::>() + .join(" || "), + closing_paren = if !parent_is_not && is_top_level { "" } else { ")" }, + ), + }, + CfgEntry::All(entries, _) => match entries.as_slice() { + [] => if parent_is_not { "false" } else { "true" }.to_string(), + [entry] => generate_cfg_replacement_inner(&entry, is_top_level, parent_is_not), + _ => format!( + "{not}{open_paren}{cond}{closing_paren}", + not = if parent_is_not { "!" } else { "" }, + open_paren = if !parent_is_not && is_top_level { "" } else { "(" }, + cond = entries + .iter() + .map(|cfg| generate_cfg_replacement_inner(cfg, false, false)) + .collect::>() + .join(" && "), + closing_paren = if !parent_is_not && is_top_level { "" } else { ")" }, + ), + }, + CfgEntry::Not(cfg, _) => generate_cfg_replacement_inner(cfg, is_top_level, true), + _ => String::new(), + } +} + +fn misleading_cfgs(entry: &CfgEntry, spans: &mut Vec, has_ok_cfgs: &mut bool) { + match entry { + CfgEntry::All(entries, _) | CfgEntry::Any(entries, _) => { + for entry in entries { + misleading_cfgs(entry, spans, has_ok_cfgs); + } + } + CfgEntry::Not(entry, _) => misleading_cfgs(entry, spans, has_ok_cfgs), + CfgEntry::Bool(..) | CfgEntry::Version(..) => { + *has_ok_cfgs = true; + } + CfgEntry::NameValue { name, value, span } => match value { + Some(_) => { + let name = name.as_str(); + if name.starts_with("target_") { + spans.push(*span); + } else { + *has_ok_cfgs = true; + } + } + None => { + if [sym::windows, sym::unix].contains(&name) { + spans.push(*span); + } else { + *has_ok_cfgs = true; + } + } + }, + } +} + +fn check_unexpected_cfgs( + cx: &mut AcceptContext<'_, '_>, + entry: &CfgEntry, + _span: Span, + is_cfg_macro: bool, +) { + let mut spans = Vec::new(); + let mut has_ok_cfgs = false; + misleading_cfgs(entry, &mut spans, &mut has_ok_cfgs); + if spans.is_empty() { + return; + } + if !is_cfg_macro || has_ok_cfgs { + cx.emit_lint( + MISLEADING_CFG_IN_BUILD_SCRIPT, + crate::errors::UnexpectedCfg { span: None, suggestion_message: String::new() }, + spans, + ); + return; + } + let replacement = generate_cfg_replacement(entry); + // The span including the `cfg!()` macro. + let span = cx.attr_span; + cx.emit_lint( + MISLEADING_CFG_IN_BUILD_SCRIPT, + crate::errors::UnexpectedCfg { span: Some(span), suggestion_message: replacement }, + span, + ); +} + +pub fn parse_cfg_entry_macro( + cx: &mut AcceptContext<'_, '_>, + item: &MetaItemOrLitParser, +) -> Result { + let entry = parse_cfg_entry_inner(cx, item)?; + check_unexpected_cfgs(cx, &entry, item.span(), true); + Ok(entry) +} + pub fn parse_cfg_entry( cx: &mut AcceptContext<'_, '_>, item: &MetaItemOrLitParser, +) -> Result { + let entry = parse_cfg_entry_inner(cx, item)?; + check_unexpected_cfgs(cx, &entry, item.span(), false); + Ok(entry) +} + +fn parse_cfg_entry_inner( + cx: &mut AcceptContext<'_, '_>, + item: &MetaItemOrLitParser, ) -> Result { Ok(match item { MetaItemOrLitParser::MetaItemParser(meta) => match meta.args() { @@ -90,14 +230,14 @@ pub fn parse_cfg_entry( let Some(single) = list.as_single() else { return Err(cx.adcx().expected_single_argument(list.span, list.len())); }; - CfgEntry::Not(Box::new(parse_cfg_entry(cx, single)?), list.span) + CfgEntry::Not(Box::new(parse_cfg_entry_inner(cx, single)?), list.span) } Some(sym::any) => CfgEntry::Any( - list.mixed().flat_map(|sub_item| parse_cfg_entry(cx, sub_item)).collect(), + list.mixed().flat_map(|sub_item| parse_cfg_entry_inner(cx, sub_item)).collect(), list.span, ), Some(sym::all) => CfgEntry::All( - list.mixed().flat_map(|sub_item| parse_cfg_entry(cx, sub_item)).collect(), + list.mixed().flat_map(|sub_item| parse_cfg_entry_inner(cx, sub_item)).collect(), list.span, ), Some(sym::target) => parse_cfg_entry_target(cx, list, meta.span())?, diff --git a/compiler/rustc_attr_parsing/src/attributes/diagnostic/check_cfg.rs b/compiler/rustc_attr_parsing/src/attributes/diagnostic/check_cfg.rs index 33db99c6bc520..0690592928b69 100644 --- a/compiler/rustc_attr_parsing/src/attributes/diagnostic/check_cfg.rs +++ b/compiler/rustc_attr_parsing/src/attributes/diagnostic/check_cfg.rs @@ -63,7 +63,7 @@ fn cargo_help_sub( inst: &impl Fn(EscapeQuotes) -> String, ) -> errors::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.rs`. We therefore 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); diff --git a/compiler/rustc_attr_parsing/src/errors.rs b/compiler/rustc_attr_parsing/src/errors.rs index d2c9c1b1eb807..af74f9bd75d59 100644 --- a/compiler/rustc_attr_parsing/src/errors.rs +++ b/compiler/rustc_attr_parsing/src/errors.rs @@ -326,6 +326,18 @@ pub(crate) struct IncorrectDoNotRecommendLocation { pub target_span: Span, } +#[derive(Diagnostic)] +#[diag("target-based cfg should be avoided in build scripts")] +pub(crate) struct UnexpectedCfg { + #[suggestion( + "use cargo environment variables if possible", + code = "{suggestion_message}", + applicability = "maybe-incorrect" + )] + pub span: Option, + pub suggestion_message: String, +} + #[derive(Diagnostic)] #[diag("malformed `doc` attribute input")] #[warning( diff --git a/compiler/rustc_attr_parsing/src/lib.rs b/compiler/rustc_attr_parsing/src/lib.rs index 24e10e19ca976..82808df372977 100644 --- a/compiler/rustc_attr_parsing/src/lib.rs +++ b/compiler/rustc_attr_parsing/src/lib.rs @@ -116,6 +116,7 @@ pub mod validate_attr; pub use attributes::AttributeSafety; pub use attributes::cfg::{ CFG_TEMPLATE, EvalConfigResult, eval_config_entry, parse_cfg, parse_cfg_attr, parse_cfg_entry, + parse_cfg_entry_macro, }; pub use attributes::cfg_select::*; pub use attributes::util::{is_builtin_attr, parse_version}; diff --git a/compiler/rustc_builtin_macros/src/cfg.rs b/compiler/rustc_builtin_macros/src/cfg.rs index 2872cff0fdc7a..892f7326c4e7b 100644 --- a/compiler/rustc_builtin_macros/src/cfg.rs +++ b/compiler/rustc_builtin_macros/src/cfg.rs @@ -7,7 +7,7 @@ use rustc_ast::{AttrStyle, token}; use rustc_attr_parsing::parser::{AllowExprMetavar, MetaItemOrLitParser}; use rustc_attr_parsing::{ self as attr, AttributeParser, AttributeSafety, CFG_TEMPLATE, ParsedDescription, ShouldEmit, - parse_cfg_entry, + parse_cfg_entry_macro, }; use rustc_expand::base::{DummyResult, ExpandResult, ExtCtxt, MacEager, MacroExpanderResult}; use rustc_hir::attrs::CfgEntry; @@ -63,7 +63,7 @@ fn parse_cfg(cx: &ExtCtxt<'_>, span: Span, tts: TokenStream) -> Result LintExtractor<'a> { if matches!( lint.name.as_str(), "unused_features" // broken lint + | "misleading_cfg_in_build_script" // only run in cargo build scripts ) { return Ok(()); } diff --git a/tests/ui/lint/misleading_cfg_in_build_script.rs b/tests/ui/lint/misleading_cfg_in_build_script.rs new file mode 100644 index 0000000000000..e7c840d596ac5 --- /dev/null +++ b/tests/ui/lint/misleading_cfg_in_build_script.rs @@ -0,0 +1,47 @@ +// This test checks the `cfg()` attributes/macros in cargo build scripts. +// +//@ no-auto-check-cfg + +#![deny(misleading_cfg_in_build_script)] +#![allow(dead_code)] + +#[cfg(windows)] +//~^ ERROR: misleading_cfg_in_build_script +fn unused_windows_fn() {} +#[cfg(not(windows))] +//~^ ERROR: misleading_cfg_in_build_script +fn unused_not_windows_fn() {} +#[cfg(any(windows, feature = "yellow", unix))] +//~^ ERROR: misleading_cfg_in_build_script +fn pink() {} + +// Should not lint. +#[cfg(feature = "green")] +fn pink() {} +#[cfg(bob)] +fn bob() {} + +fn main() { + if cfg!(windows) {} + //~^ ERROR: misleading_cfg_in_build_script + if cfg!(not(windows)) {} + //~^ ERROR: misleading_cfg_in_build_script + if cfg!(target_os = "freebsd") {} + //~^ ERROR: misleading_cfg_in_build_script + if cfg!(any(target_os = "freebsd", windows)) {} + //~^ ERROR: misleading_cfg_in_build_script + if cfg!(not(any(target_os = "freebsd", windows))) {} + //~^ ERROR: misleading_cfg_in_build_script + if cfg!(all(target_os = "freebsd", windows)) {} + //~^ ERROR: misleading_cfg_in_build_script + if cfg!(not(all(target_os = "freebsd", windows))) {} + //~^ ERROR: misleading_cfg_in_build_script + if cfg!(all(target_os = "freebsd", any(windows, not(feature = "red")))) {} + //~^ ERROR: misleading_cfg_in_build_script + + // Should not warn. + if cfg!(any()) {} + if cfg!(all()) {} + if cfg!(feature = "blue") {} + if cfg!(bob) {} +} diff --git a/tests/ui/lint/misleading_cfg_in_build_script.stderr b/tests/ui/lint/misleading_cfg_in_build_script.stderr new file mode 100644 index 0000000000000..e2c75999db929 --- /dev/null +++ b/tests/ui/lint/misleading_cfg_in_build_script.stderr @@ -0,0 +1,74 @@ +error: target-based cfg should be avoided in build scripts + --> $DIR/misleading_cfg_in_build_script.rs:8:1 + | +LL | #[cfg(windows)] + | ^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/misleading_cfg_in_build_script.rs:5:9 + | +LL | #![deny(misleading_cfg_in_build_script)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: target-based cfg should be avoided in build scripts + --> $DIR/misleading_cfg_in_build_script.rs:11:1 + | +LL | #[cfg(not(windows))] + | ^^^^^^^^^^^^^^^^^^^^ + +error: target-based cfg should be avoided in build scripts + --> $DIR/misleading_cfg_in_build_script.rs:14:11 + | +LL | #[cfg(any(windows, feature = "yellow", unix))] + | ^^^^^^^ ^^^^ + +error: target-based cfg should be avoided in build scripts + --> $DIR/misleading_cfg_in_build_script.rs:25:8 + | +LL | if cfg!(windows) {} + | ^^^^^^^^^^^^^ help: use cargo environment variables if possible: `std::env::var("CARGO_CFG_WINDOWS").is_ok()` + +error: target-based cfg should be avoided in build scripts + --> $DIR/misleading_cfg_in_build_script.rs:27:8 + | +LL | if cfg!(not(windows)) {} + | ^^^^^^^^^^^^^^^^^^ help: use cargo environment variables if possible: `std::env::var("CARGO_CFG_WINDOWS").is_err()` + +error: target-based cfg should be avoided in build scripts + --> $DIR/misleading_cfg_in_build_script.rs:29:8 + | +LL | if cfg!(target_os = "freebsd") {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use cargo environment variables if possible: `std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default() == "freebsd"` + +error: target-based cfg should be avoided in build scripts + --> $DIR/misleading_cfg_in_build_script.rs:31:8 + | +LL | if cfg!(any(target_os = "freebsd", windows)) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use cargo environment variables if possible: `std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default() == "freebsd" || std::env::var("CARGO_CFG_WINDOWS").is_ok()` + +error: target-based cfg should be avoided in build scripts + --> $DIR/misleading_cfg_in_build_script.rs:33:8 + | +LL | if cfg!(not(any(target_os = "freebsd", windows))) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use cargo environment variables if possible: `!(std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default() == "freebsd" || std::env::var("CARGO_CFG_WINDOWS").is_ok())` + +error: target-based cfg should be avoided in build scripts + --> $DIR/misleading_cfg_in_build_script.rs:35:8 + | +LL | if cfg!(all(target_os = "freebsd", windows)) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use cargo environment variables if possible: `std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default() == "freebsd" && std::env::var("CARGO_CFG_WINDOWS").is_ok()` + +error: target-based cfg should be avoided in build scripts + --> $DIR/misleading_cfg_in_build_script.rs:37:8 + | +LL | if cfg!(not(all(target_os = "freebsd", windows))) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use cargo environment variables if possible: `!(std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default() == "freebsd" && std::env::var("CARGO_CFG_WINDOWS").is_ok())` + +error: target-based cfg should be avoided in build scripts + --> $DIR/misleading_cfg_in_build_script.rs:39:8 + | +LL | if cfg!(all(target_os = "freebsd", any(windows, not(feature = "red")))) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use cargo environment variables if possible: `std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default() == "freebsd" && (std::env::var("CARGO_CFG_WINDOWS").is_ok() || cfg!(not(feature = red)))` + +error: aborting due to 11 previous errors +