Skip to content

Improve unexpected_cfgs lint when their is no value expected #94561

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

Merged
merged 1 commit into from
Mar 5, 2022
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
7 changes: 3 additions & 4 deletions compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ pub fn cfg_matches(
cfg.span,
lint_node_id,
"unexpected `cfg` condition name",
BuiltinLintDiagnostics::UnexpectedCfg(ident.span, name, None),
BuiltinLintDiagnostics::UnexpectedCfg((name, ident.span), None),
);
}
}
Expand All @@ -489,9 +489,8 @@ pub fn cfg_matches(
lint_node_id,
"unexpected `cfg` condition value",
BuiltinLintDiagnostics::UnexpectedCfg(
cfg.name_value_literal_span().unwrap(),
name,
Some(value),
(name, ident.span),
Some((value, cfg.name_value_literal_span().unwrap())),
),
);
}
Expand Down
46 changes: 26 additions & 20 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -779,37 +779,43 @@ pub trait LintContext: Sized {
db.help(&help);
db.note("see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information");
},
BuiltinLintDiagnostics::UnexpectedCfg(span, name, value) => {
let possibilities: Vec<Symbol> = if value.is_some() {
let Some(values) = &sess.parse_sess.check_config.values_valid.get(&name) else {
bug!("it shouldn't be possible to have a diagnostic on a value whose name is not in values");
};
values.iter().map(|&s| s).collect()
} else {
let Some(names_valid) = &sess.parse_sess.check_config.names_valid else {
bug!("it shouldn't be possible to have a diagnostic on a name if name checking is not enabled");
};
names_valid.iter().map(|s| *s).collect()
BuiltinLintDiagnostics::UnexpectedCfg((name, name_span), None) => {
let Some(names_valid) = &sess.parse_sess.check_config.names_valid else {
bug!("it shouldn't be possible to have a diagnostic on a name if name checking is not enabled");
};
let possibilities: Vec<Symbol> = names_valid.iter().map(|s| *s).collect();

// Suggest the most probable if we found one
if let Some(best_match) = find_best_match_for_name(&possibilities, name, None) {
db.span_suggestion(name_span, "did you mean", format!("{best_match}"), Applicability::MaybeIncorrect);
}
},
BuiltinLintDiagnostics::UnexpectedCfg((name, name_span), Some((value, value_span))) => {
let Some(values) = &sess.parse_sess.check_config.values_valid.get(&name) else {
bug!("it shouldn't be possible to have a diagnostic on a value whose name is not in values");
};
let possibilities: Vec<Symbol> = values.iter().map(|&s| s).collect();

// 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
if value.is_some() {
if !possibilities.is_empty() {
if !possibilities.is_empty() {
{
let mut possibilities = possibilities.iter().map(Symbol::as_str).collect::<Vec<_>>();
possibilities.sort();

let possibilities = possibilities.join(", ");
db.note(&format!("expected values for `{name}` are: {possibilities}"));
} else {
db.note(&format!("no expected value for `{name}`"));
}
}

// Suggest the most probable if we found one
if let Some(best_match) = find_best_match_for_name(&possibilities, value.unwrap_or(name), None) {
let punctuation = if value.is_some() { "\"" } else { "" };
db.span_suggestion(span, "did you mean", format!("{punctuation}{best_match}{punctuation}"), Applicability::MaybeIncorrect);
// Suggest the most probable if we found one
if let Some(best_match) = find_best_match_for_name(&possibilities, value, None) {
db.span_suggestion(value_span, "did you mean", format!("\"{best_match}\""), Applicability::MaybeIncorrect);
}
} else {
db.note(&format!("no expected value for `{name}`"));
if name != sym::feature {
db.span_suggestion(name_span.shrink_to_hi().to(value_span), "remove the value", String::new(), Applicability::MaybeIncorrect);
}
}
},
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ pub enum BuiltinLintDiagnostics {
BreakWithLabelAndLoop(Span),
NamedAsmLabel(String),
UnicodeTextFlow(Span, String),
UnexpectedCfg(Span, Symbol, Option<Symbol>),
UnexpectedCfg((Symbol, Span), Option<(Symbol, Span)>),
}

/// Lints that are buffered up early on in the `Session` before the
Expand Down
4 changes: 3 additions & 1 deletion src/test/ui/check-cfg/empty-values.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ warning: unexpected `cfg` condition value
--> $DIR/empty-values.rs:6:7
|
LL | #[cfg(test = "value")]
| ^^^^^^^^^^^^^^
| ^^^^----------
| |
| help: remove the value
|
= note: `#[warn(unexpected_cfgs)]` on by default
= note: no expected value for `test`
Expand Down
6 changes: 5 additions & 1 deletion src/test/ui/check-cfg/no-values.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
// Check that we detect unexpected value when none are allowed
//
// check-pass
// compile-flags: --check-cfg=values(feature) -Z unstable-options
// compile-flags: --check-cfg=values(test) --check-cfg=values(feature) -Z unstable-options

#[cfg(feature = "foo")]
//~^ WARNING unexpected `cfg` condition value
fn do_foo() {}

#[cfg(test = "foo")]
//~^ WARNING unexpected `cfg` condition value
fn do_foo() {}

fn main() {}
12 changes: 11 additions & 1 deletion src/test/ui/check-cfg/no-values.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,15 @@ LL | #[cfg(feature = "foo")]
= note: `#[warn(unexpected_cfgs)]` on by default
= note: no expected value for `feature`

warning: 1 warning emitted
warning: unexpected `cfg` condition value
--> $DIR/no-values.rs:10:7
|
LL | #[cfg(test = "foo")]
| ^^^^--------
| |
| help: remove the value
|
= note: no expected value for `test`

warning: 2 warnings emitted

4 changes: 3 additions & 1 deletion src/test/ui/check-cfg/well-known-values.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ warning: unexpected `cfg` condition value
--> $DIR/well-known-values.rs:21:7
|
LL | #[cfg(unix = "aa")]
| ^^^^^^^^^^^
| ^^^^-------
| |
| help: remove the value
|
= note: no expected value for `unix`

Expand Down