about summary refs log tree commit diff
diff options
context:
space:
mode:
authorLoïc BRANSTETT <lolo.branstett@numericable.fr>2022-03-03 15:56:19 +0100
committerLoïc BRANSTETT <lolo.branstett@numericable.fr>2022-03-05 12:11:05 +0100
commit92544f43b052b4c6b1d224c7ff73f68c0a19bd5c (patch)
treeb89ad1cb40935aefc84e61a58468b267d445f6e8
parent86067bb461d044ee30e7880ab6f3b34d5070f1db (diff)
downloadrust-92544f43b052b4c6b1d224c7ff73f68c0a19bd5c.tar.gz
rust-92544f43b052b4c6b1d224c7ff73f68c0a19bd5c.zip
Improve unexpected_cfgs lint when their is no value expected
-rw-r--r--compiler/rustc_attr/src/builtin.rs7
-rw-r--r--compiler/rustc_lint/src/context.rs46
-rw-r--r--compiler/rustc_lint_defs/src/lib.rs2
-rw-r--r--src/test/ui/check-cfg/empty-values.stderr4
-rw-r--r--src/test/ui/check-cfg/no-values.rs6
-rw-r--r--src/test/ui/check-cfg/no-values.stderr12
-rw-r--r--src/test/ui/check-cfg/well-known-values.stderr4
7 files changed, 52 insertions, 29 deletions
diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs
index 846abce9d6a..061fee7f569 100644
--- a/compiler/rustc_attr/src/builtin.rs
+++ b/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())),
                                 ),
                             );
                         }
diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs
index c34e9f1ba78..c6bbf769b23 100644
--- a/compiler/rustc_lint/src/context.rs
+++ b/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);
+                        }
                     }
                 },
             }
diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs
index c78428765bb..e7f6dfa67b9 100644
--- a/compiler/rustc_lint_defs/src/lib.rs
+++ b/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
diff --git a/src/test/ui/check-cfg/empty-values.stderr b/src/test/ui/check-cfg/empty-values.stderr
index 834b28f1244..10dab503489 100644
--- a/src/test/ui/check-cfg/empty-values.stderr
+++ b/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`
diff --git a/src/test/ui/check-cfg/no-values.rs b/src/test/ui/check-cfg/no-values.rs
index 2440757e52d..8c80f56cb5a 100644
--- a/src/test/ui/check-cfg/no-values.rs
+++ b/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() {}
diff --git a/src/test/ui/check-cfg/no-values.stderr b/src/test/ui/check-cfg/no-values.stderr
index ea1c9107d4c..7025b4cd7ba 100644
--- a/src/test/ui/check-cfg/no-values.stderr
+++ b/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
 
diff --git a/src/test/ui/check-cfg/well-known-values.stderr b/src/test/ui/check-cfg/well-known-values.stderr
index 05b2a8af0ee..8eefd6aaf35 100644
--- a/src/test/ui/check-cfg/well-known-values.stderr
+++ b/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`