Auto merge of #94561 - Urgau:check-cfg-lint-help-remove, r=petrochenkov

Improve unexpected_cfgs lint when their is no value expected

This pull-request improve the `unexpected_cfgs` when their is no value expected by suggesting to remove the value.

I also took the liberty to special case it for `feature` as it seems wrong to suggest to remove the value when the problem is most probably the absence of value(s) and also the fact that it doesn't make sense to only have `feature` without a value.

r? `@petrochenkov`

Author: bors

compiler/rustc_attr/src/builtin.rs

@@ -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),
                         );
                     }
                 }
@@ -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())),
                                 ),
                             );
                         }

compiler/rustc_lint/src/context.rs

@@ -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);
+                        }
                     }
                 },
             }

compiler/rustc_lint_defs/src/lib.rs

@@ -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

src/test/ui/check-cfg/empty-values.stderr

@@ -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`

src/test/ui/check-cfg/no-values.rs

@@ -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() {}

src/test/ui/check-cfg/no-values.stderr

@@ -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
 

src/test/ui/check-cfg/well-known-values.stderr

@@ -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`