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

Suggest name value cfg when only value is used for check-cfg #120435

Merged
merged 3 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
50 changes: 40 additions & 10 deletions compiler/rustc_lint/src/context/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,23 @@ pub(super) fn builtin(
BuiltinLintDiagnostics::UnexpectedCfgName((name, name_span), value) => {
chenyukang marked this conversation as resolved.
Show resolved Hide resolved
let possibilities: Vec<Symbol> =
sess.parse_sess.check_config.expecteds.keys().copied().collect();

let mut names_possibilities: Vec<_> = if value.is_none() {
// We later sort and display all the possibilities, so the order here does not matter.
#[allow(rustc::potential_query_instability)]
sess.parse_sess
.check_config
.expecteds
.iter()
.filter_map(|(k, v)| match v {
ExpectedValues::Some(v) if v.contains(&Some(name)) => Some(k),
_ => None,
})
.collect()
} else {
Vec::new()
};

let is_from_cargo = std::env::var_os("CARGO").is_some();
let mut is_feature_cfg = name == sym::feature;

Expand Down Expand Up @@ -261,17 +278,30 @@ pub(super) fn builtin(
}

is_feature_cfg |= best_match == sym::feature;
} else if !possibilities.is_empty() {
let mut possibilities =
possibilities.iter().map(Symbol::as_str).collect::<Vec<_>>();
possibilities.sort();
let possibilities = possibilities.join("`, `");
} else {
if !names_possibilities.is_empty() {
names_possibilities.sort();
for cfg_name in names_possibilities.iter() {
db.span_suggestion(
chenyukang marked this conversation as resolved.
Show resolved Hide resolved
name_span,
"found config with similar value",
format!("{cfg_name} = \"{name}\""),
Applicability::MaybeIncorrect,
);
}
}
if !possibilities.is_empty() {
let mut possibilities =
possibilities.iter().map(Symbol::as_str).collect::<Vec<_>>();
possibilities.sort();
let possibilities = possibilities.join("`, `");

// The list of expected names can be long (even by default) and
// so the diagnostic produced can take a lot of space. To avoid
// cloging the user output we only want to print that diagnostic
// once.
db.help_once(format!("expected names are: `{possibilities}`"));
// The list of expected names can be long (even by default) and
// so the diagnostic produced can take a lot of space. To avoid
// cloging the user output we only want to print that diagnostic
// once.
db.help_once(format!("expected names are: `{possibilities}`"));
}
}

let inst = if let Some((value, _value_span)) = value {
Expand Down
12 changes: 12 additions & 0 deletions tests/ui/check-cfg/cfg-value-for-cfg-name-multiple.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// #120427
// This test checks that when a single cfg has a value for user's specified name
//
// check-pass
// compile-flags: -Z unstable-options
// compile-flags: --check-cfg=cfg(foo,values("my_value")) --check-cfg=cfg(bar,values("my_value"))

#[cfg(my_value)]
//~^ WARNING unexpected `cfg` condition name: `my_value`
fn x() {}

fn main() {}
21 changes: 21 additions & 0 deletions tests/ui/check-cfg/cfg-value-for-cfg-name-multiple.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
warning: unexpected `cfg` condition name: `my_value`
--> $DIR/cfg-value-for-cfg-name-multiple.rs:8:7
|
LL | #[cfg(my_value)]
| ^^^^^^^^
|
= help: expected names are: `bar`, `debug_assertions`, `doc`, `doctest`, `foo`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `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`, `unix`, `windows`
= help: to expect this configuration use `--check-cfg=cfg(my_value)`
= note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default
help: found config with similar value
|
LL | #[cfg(foo = "my_value")]
| ~~~~~~~~~~~~~~~~
help: found config with similar value
|
LL | #[cfg(bar = "my_value")]
| ~~~~~~~~~~~~~~~~

warning: 1 warning emitted

18 changes: 18 additions & 0 deletions tests/ui/check-cfg/cfg-value-for-cfg-name.rs
chenyukang marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// #120427
// This test checks that when a single cfg has a value for user's specified name
// suggest to use `#[cfg(target_os = "linux")]` instead of `#[cfg(linux)]`
//
// check-pass
// compile-flags: -Z unstable-options
// compile-flags: --check-cfg=cfg()

#[cfg(linux)]
//~^ WARNING unexpected `cfg` condition name: `linux`
fn x() {}

// will not suggest if the cfg has a value
#[cfg(linux = "os-name")]
//~^ WARNING unexpected `cfg` condition name: `linux`
fn y() {}

fn main() {}
22 changes: 22 additions & 0 deletions tests/ui/check-cfg/cfg-value-for-cfg-name.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
warning: unexpected `cfg` condition name: `linux`
--> $DIR/cfg-value-for-cfg-name.rs:9:7
|
LL | #[cfg(linux)]
| ^^^^^ help: found config with similar value: `target_os = "linux"`
|
= help: expected names are: `debug_assertions`, `doc`, `doctest`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `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`, `unix`, `windows`
= help: to expect this configuration use `--check-cfg=cfg(linux)`
= note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition name: `linux`
--> $DIR/cfg-value-for-cfg-name.rs:14:7
|
LL | #[cfg(linux = "os-name")]
| ^^^^^^^^^^^^^^^^^
|
= help: to expect this configuration use `--check-cfg=cfg(linux, values("os-name"))`
= note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration

warning: 2 warnings emitted

Loading