Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add external macros specific diagnostics for check-cfg #133221

Merged
merged 3 commits into from
Dec 14, 2024
Merged
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
4 changes: 4 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -803,10 +803,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),
Comment on lines +85 to +86
Copy link
Member

Choose a reason for hiding this comment

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

Question: is this still problematic?

Copy link
Member

Choose a reason for hiding this comment

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

Answer: yes, crate_name is on tcx, early ctxt doesn't have access to a cstore

Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine.

})
} 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 @@ -2131,6 +2131,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 @@ -2235,10 +2254,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 @@ -2341,9 +2367,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.

Loading