Skip to content

Commit

Permalink
Rollup merge of #133221 - Urgau:check-cfg-macro-diag, r=jieyouxu
Browse files Browse the repository at this point in the history
Add external macros specific diagnostics for check-cfg

This PR adds specific check-cfg diagnostics for unexpected cfg in external macros.

As well as hiding the some of the Cargo specific help/suggestions as they distraction for external macros and are generally not the right solution.

Follow-up to #132577

`@rustbot` label +L-unexpected_cfgs
r? compiler
  • Loading branch information
matthiaskrgr authored Dec 14, 2024
2 parents f5079d0 + e2fbeec commit f96fdab
Show file tree
Hide file tree
Showing 8 changed files with 211 additions and 29 deletions.
4 changes: 4 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -806,10 +806,14 @@ lint_unexpected_cfg_add_build_rs_println = or consider adding `{$build_rs_printl
lint_unexpected_cfg_add_cargo_feature = consider using a Cargo feature instead
lint_unexpected_cfg_add_cargo_toml_lint_cfg = or consider adding in `Cargo.toml` the `check-cfg` lint config for the lint:{$cargo_toml_lint_cfg}
lint_unexpected_cfg_add_cmdline_arg = to expect this configuration use `{$cmdline_arg}`
lint_unexpected_cfg_cargo_update = the {$macro_kind} `{$macro_name}` may come from an old version of it's defining crate, try updating your dependencies with `cargo update`
lint_unexpected_cfg_define_features = consider defining some features in `Cargo.toml`
lint_unexpected_cfg_doc_cargo = see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
lint_unexpected_cfg_doc_rustc = see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration
lint_unexpected_cfg_from_external_macro_origin = using a cfg inside a {$macro_kind} will use the cfgs from the destination crate and not the ones from the defining crate
lint_unexpected_cfg_from_external_macro_refer = try refering to `{$macro_name}` crate for guidance on how handle this unexpected cfg
lint_unexpected_cfg_name = unexpected `cfg` condition name: `{$name}`
lint_unexpected_cfg_name_expected_names = expected names are: {$possibilities}{$and_more ->
[0] {""}
Expand Down
68 changes: 58 additions & 10 deletions compiler/rustc_lint/src/context/diagnostics/check_cfg.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use rustc_hir::def_id::LOCAL_CRATE;
use rustc_middle::bug;
use rustc_session::Session;
use rustc_session::config::ExpectedValues;
use rustc_span::edit_distance::find_best_match_for_name;
use rustc_span::symbol::Ident;
use rustc_span::{Span, Symbol, sym};
use rustc_span::{ExpnKind, Span, Symbol, sym};

use crate::lints;

Expand Down Expand Up @@ -60,6 +61,35 @@ fn cargo_help_sub(
}
}

fn rustc_macro_help(span: Span) -> Option<lints::UnexpectedCfgRustcMacroHelp> {
let oexpn = span.ctxt().outer_expn_data();
if let Some(def_id) = oexpn.macro_def_id
&& let ExpnKind::Macro(macro_kind, macro_name) = oexpn.kind
&& def_id.krate != LOCAL_CRATE
{
Some(lints::UnexpectedCfgRustcMacroHelp { macro_kind: macro_kind.descr(), macro_name })
} else {
None
}
}

fn cargo_macro_help(span: Span) -> Option<lints::UnexpectedCfgCargoMacroHelp> {
let oexpn = span.ctxt().outer_expn_data();
if let Some(def_id) = oexpn.macro_def_id
&& let ExpnKind::Macro(macro_kind, macro_name) = oexpn.kind
&& def_id.krate != LOCAL_CRATE
{
Some(lints::UnexpectedCfgCargoMacroHelp {
macro_kind: macro_kind.descr(),
macro_name,
// FIXME: Get access to a `TyCtxt` from an `EarlyContext`
// crate_name: cx.tcx.crate_name(def_id.krate),
})
} else {
None
}
}

pub(super) fn unexpected_cfg_name(
sess: &Session,
(name, name_span): (Symbol, Span),
Expand All @@ -85,6 +115,7 @@ pub(super) fn unexpected_cfg_name(
};

let is_from_cargo = rustc_session::utils::was_invoked_from_cargo();
let is_from_external_macro = rustc_middle::lint::in_external_macro(sess, name_span);
let mut is_feature_cfg = name == sym::feature;

let code_sugg = if is_feature_cfg && is_from_cargo {
Expand Down Expand Up @@ -185,12 +216,21 @@ pub(super) fn unexpected_cfg_name(
};

let invocation_help = if is_from_cargo {
let sub = if !is_feature_cfg { Some(cargo_help_sub(sess, &inst)) } else { None };
lints::unexpected_cfg_name::InvocationHelp::Cargo { sub }
let help = if !is_feature_cfg && !is_from_external_macro {
Some(cargo_help_sub(sess, &inst))
} else {
None
};
lints::unexpected_cfg_name::InvocationHelp::Cargo {
help,
macro_help: cargo_macro_help(name_span),
}
} else {
lints::unexpected_cfg_name::InvocationHelp::Rustc(lints::UnexpectedCfgRustcHelp::new(
&inst(EscapeQuotes::No),
))
let help = lints::UnexpectedCfgRustcHelp::new(&inst(EscapeQuotes::No));
lints::unexpected_cfg_name::InvocationHelp::Rustc {
help,
macro_help: rustc_macro_help(name_span),
}
};

lints::UnexpectedCfgName { code_sugg, invocation_help, name }
Expand All @@ -216,7 +256,9 @@ pub(super) fn unexpected_cfg_value(
.copied()
.flatten()
.collect();

let is_from_cargo = rustc_session::utils::was_invoked_from_cargo();
let is_from_external_macro = rustc_middle::lint::in_external_macro(sess, name_span);

// Show the full list if all possible values for a given name, but don't do it
// for names as the possibilities could be very long
Expand Down Expand Up @@ -284,25 +326,31 @@ pub(super) fn unexpected_cfg_value(
};

let invocation_help = if is_from_cargo {
let help = if name == sym::feature {
let help = if name == sym::feature && !is_from_external_macro {
if let Some((value, _value_span)) = value {
Some(lints::unexpected_cfg_value::CargoHelp::AddFeature { value })
} else {
Some(lints::unexpected_cfg_value::CargoHelp::DefineFeatures)
}
} else if can_suggest_adding_value {
} else if can_suggest_adding_value && !is_from_external_macro {
Some(lints::unexpected_cfg_value::CargoHelp::Other(cargo_help_sub(sess, &inst)))
} else {
None
};
lints::unexpected_cfg_value::InvocationHelp::Cargo(help)
lints::unexpected_cfg_value::InvocationHelp::Cargo {
help,
macro_help: cargo_macro_help(name_span),
}
} else {
let help = if can_suggest_adding_value {
Some(lints::UnexpectedCfgRustcHelp::new(&inst(EscapeQuotes::No)))
} else {
None
};
lints::unexpected_cfg_value::InvocationHelp::Rustc(help)
lints::unexpected_cfg_value::InvocationHelp::Rustc {
help,
macro_help: rustc_macro_help(name_span),
}
};

lints::UnexpectedCfgValue {
Expand Down
44 changes: 40 additions & 4 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2172,6 +2172,25 @@ impl UnexpectedCfgRustcHelp {
}
}

#[derive(Subdiagnostic)]
#[note(lint_unexpected_cfg_from_external_macro_origin)]
#[help(lint_unexpected_cfg_from_external_macro_refer)]
pub(crate) struct UnexpectedCfgRustcMacroHelp {
pub macro_kind: &'static str,
pub macro_name: Symbol,
}

#[derive(Subdiagnostic)]
#[note(lint_unexpected_cfg_from_external_macro_origin)]
#[help(lint_unexpected_cfg_from_external_macro_refer)]
#[help(lint_unexpected_cfg_cargo_update)]
pub(crate) struct UnexpectedCfgCargoMacroHelp {
pub macro_kind: &'static str,
pub macro_name: Symbol,
// FIXME: Figure out a way to get the crate name
// crate_name: String,
}

#[derive(LintDiagnostic)]
#[diag(lint_unexpected_cfg_name)]
pub(crate) struct UnexpectedCfgName {
Expand Down Expand Up @@ -2276,10 +2295,17 @@ pub(crate) mod unexpected_cfg_name {
#[note(lint_unexpected_cfg_doc_cargo)]
Cargo {
#[subdiagnostic]
sub: Option<super::UnexpectedCfgCargoHelp>,
macro_help: Option<super::UnexpectedCfgCargoMacroHelp>,
#[subdiagnostic]
help: Option<super::UnexpectedCfgCargoHelp>,
},
#[note(lint_unexpected_cfg_doc_rustc)]
Rustc(#[subdiagnostic] super::UnexpectedCfgRustcHelp),
Rustc {
#[subdiagnostic]
macro_help: Option<super::UnexpectedCfgRustcMacroHelp>,
#[subdiagnostic]
help: super::UnexpectedCfgRustcHelp,
},
}
}

Expand Down Expand Up @@ -2382,9 +2408,19 @@ pub(crate) mod unexpected_cfg_value {
#[derive(Subdiagnostic)]
pub(crate) enum InvocationHelp {
#[note(lint_unexpected_cfg_doc_cargo)]
Cargo(#[subdiagnostic] Option<CargoHelp>),
Cargo {
#[subdiagnostic]
help: Option<CargoHelp>,
#[subdiagnostic]
macro_help: Option<super::UnexpectedCfgCargoMacroHelp>,
},
#[note(lint_unexpected_cfg_doc_rustc)]
Rustc(#[subdiagnostic] Option<super::UnexpectedCfgRustcHelp>),
Rustc {
#[subdiagnostic]
help: Option<super::UnexpectedCfgRustcHelp>,
#[subdiagnostic]
macro_help: Option<super::UnexpectedCfgRustcMacroHelp>,
},
}

#[derive(Subdiagnostic)]
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/check-cfg/auxiliary/cfg_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,19 @@ macro_rules! my_lib_macro {
$crate::my_lib_func()
};
}

#[macro_export]
macro_rules! my_lib_macro_value {
() => {
#[cfg(panic = "UNEXPECTED_VALUE")]
$crate::my_lib_func()
};
}

#[macro_export]
macro_rules! my_lib_macro_feature {
() => {
#[cfg(feature = "UNEXPECTED_FEATURE")]
$crate::my_lib_func()
};
}
42 changes: 42 additions & 0 deletions tests/ui/check-cfg/report-in-external-macros.cargo.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
warning: unexpected `cfg` condition name: `my_lib_cfg`
--> $DIR/report-in-external-macros.rs:13:5
|
LL | cfg_macro::my_lib_macro!();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: expected names are: `clippy`, `debug_assertions`, `doc`, `doctest`, `feature`, `fmt_debug`, `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`
= note: using a cfg inside a macro will use the cfgs from the destination crate and not the ones from the defining crate
= help: try refering to `cfg_macro::my_lib_macro` crate for guidance on how handle this unexpected cfg
= help: the macro `cfg_macro::my_lib_macro` may come from an old version of it's defining crate, try updating your dependencies with `cargo update`
= 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
= note: this warning originates in the macro `cfg_macro::my_lib_macro` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: unexpected `cfg` condition value: `UNEXPECTED_VALUE`
--> $DIR/report-in-external-macros.rs:16:5
|
LL | cfg_macro::my_lib_macro_value!();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: expected values for `panic` are: `abort` and `unwind`
= note: using a cfg inside a macro will use the cfgs from the destination crate and not the ones from the defining crate
= help: try refering to `cfg_macro::my_lib_macro_value` crate for guidance on how handle this unexpected cfg
= help: the macro `cfg_macro::my_lib_macro_value` may come from an old version of it's defining crate, try updating your dependencies with `cargo update`
= note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
= note: this warning originates in the macro `cfg_macro::my_lib_macro_value` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: unexpected `cfg` condition value: `UNEXPECTED_FEATURE`
--> $DIR/report-in-external-macros.rs:19:5
|
LL | cfg_macro::my_lib_macro_feature!();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: no expected values for `feature`
= note: using a cfg inside a macro will use the cfgs from the destination crate and not the ones from the defining crate
= help: try refering to `cfg_macro::my_lib_macro_feature` crate for guidance on how handle this unexpected cfg
= help: the macro `cfg_macro::my_lib_macro_feature` may come from an old version of it's defining crate, try updating your dependencies with `cargo update`
= note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
= note: this warning originates in the macro `cfg_macro::my_lib_macro_feature` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: 3 warnings emitted

11 changes: 10 additions & 1 deletion tests/ui/check-cfg/report-in-external-macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,19 @@

//@ check-pass
//@ no-auto-check-cfg
//@ revisions: cargo rustc
//@ [rustc]unset-rustc-env:CARGO_CRATE_NAME
//@ [cargo]rustc-env:CARGO_CRATE_NAME=foo
//@ aux-crate: cfg_macro=cfg_macro.rs
//@ compile-flags: --check-cfg=cfg()
//@ compile-flags: --check-cfg=cfg(feature,values())

fn main() {
cfg_macro::my_lib_macro!();
//~^ WARNING unexpected `cfg` condition name

cfg_macro::my_lib_macro_value!();
//~^ WARNING unexpected `cfg` condition value

cfg_macro::my_lib_macro_feature!();
//~^ WARNING unexpected `cfg` condition value
}
41 changes: 41 additions & 0 deletions tests/ui/check-cfg/report-in-external-macros.rustc.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
warning: unexpected `cfg` condition name: `my_lib_cfg`
--> $DIR/report-in-external-macros.rs:13:5
|
LL | cfg_macro::my_lib_macro!();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: expected names are: `clippy`, `debug_assertions`, `doc`, `doctest`, `feature`, `fmt_debug`, `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`
= note: using a cfg inside a macro will use the cfgs from the destination crate and not the ones from the defining crate
= help: try refering to `cfg_macro::my_lib_macro` crate for guidance on how handle this unexpected cfg
= help: to expect this configuration use `--check-cfg=cfg(my_lib_cfg)`
= note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default
= note: this warning originates in the macro `cfg_macro::my_lib_macro` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: unexpected `cfg` condition value: `UNEXPECTED_VALUE`
--> $DIR/report-in-external-macros.rs:16:5
|
LL | cfg_macro::my_lib_macro_value!();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: expected values for `panic` are: `abort` and `unwind`
= note: using a cfg inside a macro will use the cfgs from the destination crate and not the ones from the defining crate
= help: try refering to `cfg_macro::my_lib_macro_value` crate for guidance on how handle this unexpected cfg
= note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration
= note: this warning originates in the macro `cfg_macro::my_lib_macro_value` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: unexpected `cfg` condition value: `UNEXPECTED_FEATURE`
--> $DIR/report-in-external-macros.rs:19:5
|
LL | cfg_macro::my_lib_macro_feature!();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: no expected values for `feature`
= help: to expect this configuration use `--check-cfg=cfg(feature, values("UNEXPECTED_FEATURE"))`
= note: using a cfg inside a macro will use the cfgs from the destination crate and not the ones from the defining crate
= help: try refering to `cfg_macro::my_lib_macro_feature` crate for guidance on how handle this unexpected cfg
= note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration
= note: this warning originates in the macro `cfg_macro::my_lib_macro_feature` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: 3 warnings emitted

14 changes: 0 additions & 14 deletions tests/ui/check-cfg/report-in-external-macros.stderr

This file was deleted.

0 comments on commit f96fdab

Please sign in to comment.