Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/attr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ impl MetaItem {
}
}

fn from_tokens(iter: &mut TokenStreamIter<'_>) -> Option<MetaItem> {
pub fn from_tokens(iter: &mut TokenStreamIter<'_>) -> Option<MetaItem> {
// FIXME: Share code with `parse_path`.
let tt = iter.next().map(|tt| TokenTree::uninterpolate(tt));
let path = match tt.as_deref() {
Expand Down
18 changes: 18 additions & 0 deletions compiler/rustc_attr_parsing/src/attributes/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,24 @@ pub fn parse_cfg_entry<S: Stage>(
cx: &mut AcceptContext<'_, '_, S>,
item: &MetaItemOrLitParser,
) -> Result<CfgEntry, ErrorGuaranteed> {
let entry = parse_cfg_entry_inner(cx, item)?;
let mut spans = Vec::new();
let total = misleading_cfgs(&entry, &mut spans);
if !spans.is_empty() {
cx.emit_lint(
UNEXPECTED_CFGS,
AttributeLintKind::UnexpectedCfgName((name, name_span), value),
span,
);
}
Ok(entry)
}

fn parse_cfg_entry_inner<S: Stage>(
cx: &mut AcceptContext<'_, '_, S>,
item: &MetaItemOrLitParser,
) -> Result<CfgEntry, ErrorGuaranteed> {

Ok(match item {
MetaItemOrLitParser::MetaItemParser(meta) => match meta.args() {
ArgParser::List(list) => match meta.path().word_sym() {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/early/diagnostics/check_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ fn cargo_help_sub(
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.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);
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ pub mod lifetime_syntax;
mod lints;
mod macro_expr_fragment_specifier_2024_migration;
mod map_unit_fn;
mod misleading_cfg_in_build_script;
mod multiple_supertrait_upcastable;
mod non_ascii_idents;
mod non_fmt_panic;
Expand Down Expand Up @@ -101,6 +102,7 @@ use let_underscore::*;
use lifetime_syntax::*;
use macro_expr_fragment_specifier_2024_migration::*;
use map_unit_fn::*;
use misleading_cfg_in_build_script::MisleadingCfgInBuildScript;
use multiple_supertrait_upcastable::*;
use non_ascii_idents::*;
use non_fmt_panic::NonPanicFmt;
Expand Down Expand Up @@ -157,6 +159,7 @@ early_lint_methods!(
pub BuiltinCombinedPreExpansionLintPass,
[
KeywordIdents: KeywordIdents,
MisleadingCfgInBuildScript: MisleadingCfgInBuildScript,
]
]
);
Expand Down
275 changes: 275 additions & 0 deletions compiler/rustc_lint/src/misleading_cfg_in_build_script.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,275 @@
use rustc_ast::ast::{Attribute, MacCall};
use rustc_ast::tokenstream::TokenStream;
use rustc_ast::{MetaItem, MetaItemInner, MetaItemKind};
use rustc_errors::{Applicability, DiagDecorator};
use rustc_session::{declare_lint, declare_lint_pass};
use rustc_span::{Span, Symbol, sym};

use crate::{EarlyContext, EarlyLintPass, LintContext};

declare_lint! {
/// Checks for usage of `#[cfg]`/`#[cfg_attr]`/`cfg!()` in `build.rs` scripts.
///
/// ### Explanation
///
/// It checks the `cfg` values for the *host*, not the target. For example, `cfg!(windows)` is
/// true when compiling on Windows, so it will give the wrong answer if you are cross compiling.
/// This is because build scripts run on the machine performing compilation, rather than on the
/// target.
///
/// ### Example
///
/// ```rust,ignore (should only be run in cargo build scripts)
/// if cfg!(windows) {}
/// ```
///
/// Use instead:
///
/// ```rust
/// if std::env::var("CARGO_CFG_WINDOWS").is_ok() {}
/// ```
pub MISLEADING_CFG_IN_BUILD_SCRIPT,
Allow,
"use of host configs in `build.rs` scripts"
}
Comment on lines +10 to +34
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be moved to rustc_lint_defs.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most lints are defined directly in their file in this crate, so I'll keep the same logic.


declare_lint_pass!(MisleadingCfgInBuildScript => [MISLEADING_CFG_IN_BUILD_SCRIPT]);

/// Represents the AST of `cfg` attribute and `cfg!` macro.
#[derive(Debug)]
enum CfgAst {
/// Represents an OS family such as "unix" or "windows".
OsFamily(Symbol),
/// The `any()` attribute.
Any(Vec<CfgAst>),
/// The `all()` attribute.
All(Vec<CfgAst>),
/// The `not()` attribute.
Not(Box<CfgAst>),
/// Any `target_* = ""` key/value attribute.
TargetKeyValue(Symbol, Symbol),
/// the `feature = ""` attribute with its associated value.
Feature(Symbol),
}

impl CfgAst {
fn has_only_features(&self) -> bool {
match self {
Self::OsFamily(_) | Self::TargetKeyValue(_, _) => false,
Self::Any(v) | Self::All(v) => v.is_empty() || v.iter().all(CfgAst::has_only_features),
Self::Not(v) => v.has_only_features(),
Self::Feature(_) => true,
}
}

fn generate_replacement(&self) -> String {
self.generate_replacement_inner(true, false)
}

fn generate_replacement_inner(&self, is_top_level: bool, parent_is_not: bool) -> String {
match self {
Self::OsFamily(os) => format!(
"std::env::var(\"CARGO_CFG_{}\"){}",
os.as_str().to_uppercase(),
if parent_is_not { ".is_err()" } else { ".is_ok()" },
),
Self::TargetKeyValue(cfg_target, s) => format!(
"{}std::env::var(\"CARGO_CFG_{}\").unwrap_or_default() == \"{s}\"",
if parent_is_not { "!" } else { "" },
cfg_target.as_str().to_uppercase(),
),
Self::Any(v) => {
if v.is_empty() {
if parent_is_not { "true" } else { "false" }.to_string()
} else if v.len() == 1 {
v[0].generate_replacement_inner(is_top_level, parent_is_not)
} else {
format!(
"{not}{open_paren}{cond}{closing_paren}",
not = if parent_is_not { "!" } else { "" },
open_paren = if !parent_is_not && is_top_level { "" } else { "(" },
cond = v
.iter()
.map(|i| i.generate_replacement_inner(false, false))
.collect::<Vec<_>>()
.join(" || "),
closing_paren = if !parent_is_not && is_top_level { "" } else { ")" },
)
}
}
Self::All(v) => {
if v.is_empty() {
if parent_is_not { "false" } else { "true" }.to_string()
} else if v.len() == 1 {
v[0].generate_replacement_inner(is_top_level, parent_is_not)
} else {
format!(
"{not}{open_paren}{cond}{closing_paren}",
not = if parent_is_not { "!" } else { "" },
open_paren = if !parent_is_not && is_top_level { "" } else { "(" },
cond = v
.iter()
.map(|i| i.generate_replacement_inner(false, false))
.collect::<Vec<_>>()
.join(" && "),
closing_paren = if !parent_is_not && is_top_level { "" } else { ")" },
)
}
}
Self::Not(i) => i.generate_replacement_inner(is_top_level, true),
Self::Feature(s) => format!(
"cfg!({}feature = {s}{})",
if parent_is_not { "not(" } else { "" },
if parent_is_not { ")" } else { "" },
),
}
}
}

fn parse_meta_item(meta: MetaItem, has_unknown: &mut bool, out: &mut Vec<CfgAst>) {
let Some(name) = meta.name() else {
*has_unknown = true;
return;
};
match meta.kind {
MetaItemKind::Word => {
if [sym::windows, sym::unix].contains(&name) {
out.push(CfgAst::OsFamily(name));
return;
}
}
MetaItemKind::NameValue(item) => {
if name == sym::feature {
out.push(CfgAst::Feature(item.symbol));
return;
} else if name.as_str().starts_with("target_") {
out.push(CfgAst::TargetKeyValue(name, item.symbol));
return;
}
}
MetaItemKind::List(item) => {
if !*has_unknown && [sym::any, sym::not, sym::all].contains(&name) {
let mut sub_out = Vec::new();

for sub in item {
if let MetaItemInner::MetaItem(item) = sub {
parse_meta_item(item, has_unknown, &mut sub_out);
if *has_unknown {
return;
}
}
}
if name == sym::any {
out.push(CfgAst::Any(sub_out));
return;
} else if name == sym::all {
out.push(CfgAst::All(sub_out));
return;
// `not()` can only have one argument.
} else if sub_out.len() == 1 {
out.push(CfgAst::Not(Box::new(sub_out.pop().unwrap())));
return;
}
}
}
}
*has_unknown = true;
}

fn parse_macro_args(tokens: &TokenStream, has_unknown: &mut bool, out: &mut Vec<CfgAst>) {
if let Some(meta) = MetaItem::from_tokens(&mut tokens.iter()) {
parse_meta_item(meta, has_unknown, out);
}
}

fn get_invalid_cfg_attrs(attr: &MetaItem, spans: &mut Vec<Span>, has_unknown: &mut bool) {
let Some(ident) = attr.ident() else { return };
if [sym::unix, sym::windows].contains(&ident.name) {
spans.push(attr.span);
} else if attr.value_str().is_some() && ident.name.as_str().starts_with("target_") {
spans.push(attr.span);
} else if let Some(sub_attrs) = attr.meta_item_list() {
for sub_attr in sub_attrs {
if let Some(meta) = sub_attr.meta_item() {
get_invalid_cfg_attrs(meta, spans, has_unknown);
}
}
} else {
*has_unknown = true;
}
}
Comment on lines +185 to +200
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be moved in parse_name_value:

pub(crate) fn parse_name_value<S: Stage>(


const ERROR_MESSAGE: &str = "target-based cfg should be avoided in build scripts";

impl EarlyLintPass for MisleadingCfgInBuildScript {
fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
match attr.name() {
Some(sym::cfg) if let Some(meta) = attr.meta() => {
get_invalid_cfg_attrs(&meta, &mut spans, &mut has_unknown);
}
Some(sym::cfg_attr)
if let Some(sub_attrs) = attr.meta_item_list()
&& let Some(meta) = sub_attrs.first().and_then(|a| a.meta_item()) =>
{
get_invalid_cfg_attrs(meta, &mut spans, &mut has_unknown);
}
_ => return,
}
if !spans.is_empty() {
if has_unknown {
// If the `cfg`/`cfg_attr` attribute contains not only invalid items, we display
// spans of all invalid items.
cx.emit_span_lint(
MISLEADING_CFG_IN_BUILD_SCRIPT,
spans,
DiagDecorator(|diag| {
diag.primary_message(ERROR_MESSAGE);
}),
);
} else {
// No "good" item in the `cfg`/`cfg_attr` attribute so we can use the span of the
// whole attribute directly.
cx.emit_span_lint(
MISLEADING_CFG_IN_BUILD_SCRIPT,
attr.span,
DiagDecorator(|diag| {
diag.primary_message(ERROR_MESSAGE);
}),
);
}
}
}

fn check_mac(&mut self, cx: &EarlyContext<'_>, call: &MacCall) {
if call.path.segments.len() == 1 && call.path.segments[0].ident.name == sym::cfg {
let mut ast = Vec::new();
let mut has_unknown = false;
parse_macro_args(&call.args.tokens, &mut has_unknown, &mut ast);
if !has_unknown && ast.len() > 1 {
cx.emit_span_lint(
MISLEADING_CFG_IN_BUILD_SCRIPT,
call.span(),
DiagDecorator(|diag| {
diag.primary_message(ERROR_MESSAGE);
}),
);
} else if let Some(ast) = ast.get(0)
&& !ast.has_only_features()
{
let span = call.span();
cx.emit_span_lint(
MISLEADING_CFG_IN_BUILD_SCRIPT,
span,
DiagDecorator(|diag| {
diag.primary_message(ERROR_MESSAGE).span_suggestion(
span,
"use cargo environment variables if possible",
ast.generate_replacement(),
Applicability::MaybeIncorrect,
);
}),
);
}
}
}
}
29 changes: 28 additions & 1 deletion compiler/rustc_session/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_feature::{GateIssue, UnstableFeatures, find_feature_issue};
use rustc_span::edition::Edition;
use rustc_span::hygiene::ExpnId;
use rustc_span::source_map::{FilePathMapping, SourceMap};
use rustc_span::{Span, Symbol, sym};
use rustc_span::{ErrorGuaranteed, Span, Symbol, sym};

use crate::Session;
use crate::config::{Cfg, CheckCfg};
Expand Down Expand Up @@ -379,3 +379,30 @@ impl ParseSess {
self.dcx.handle()
}
}

pub fn parse_cfg(parser: &mut rustc_parse::Parser<'_>, sess: &rustc_session::Session<'_>, span: Span, tts: TokenStream) -> Result<CfgEntry, ErrorGuaranteed> {
let meta = MetaItemOrLitParser::parse_single(
&mut parser,
ShouldEmit::Nothing,
AllowExprMetavar::Yes,
)
.map_err(|_| diag.emit())?;
AttributeParser::parse_single_args(
sess,
span,
span,
AttrStyle::Inner,
AttrPath { segments: vec![sym::cfg].into_boxed_slice(), span },
None,
ParsedDescription::Macro,
span,
cx.current_expansion.lint_node_id,
// Doesn't matter what the target actually is here.
Target::Crate,
Some(cx.ecfg.features),
ShouldEmit::Nothing,
&meta,
parse_cfg_entry,
&CFG_TEMPLATE,
)
}
1 change: 1 addition & 0 deletions src/tools/lint-docs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ impl<'a> LintExtractor<'a> {
lint.name.as_str(),
"unused_features" // broken lint
| "soft_unstable" // cannot have a stable example
| "misleading_cfg_in_build_script" // only run in cargo build scripts
) {
return Ok(());
}
Expand Down
Loading