Auto merge of #116666 - Urgau:check-cfg-pre-mcp636, r=petrochenkov

Improve check-cfg diagnostics This PR tries to improve some of the diagnostics of check-cfg. The main changes is the unexpected name or value being added to the main diagnostic: ```diff - warning: unexpected `cfg` condition name + warning: unexpected `cfg` condition name: `widnows` ``` It also cherry-pick the better sensible logic for when we print the list of expected values when we have a matching value for a very similar name. Address #111072 (comment) r? `@petrochenkov`
rust-lang · Oct 13, 2023 · a4a10bd · a4a10bd
2 parents 2763ca5 + ed922d8
commit a4a10bd
Show file tree

Hide file tree

Showing 21 changed files with 87 additions and 62 deletions.
diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs
@@ -548,7 +548,11 @@ pub fn cfg_matches(
                     UNEXPECTED_CFGS,
                     cfg.span,
                     lint_node_id,
-                    "unexpected `cfg` condition value",
+                    if let Some(value) = cfg.value {
+                        format!("unexpected `cfg` condition value: `{value}`")
+                    } else {
+                        format!("unexpected `cfg` condition value: (none)")
+                    },
                     BuiltinLintDiagnostics::UnexpectedCfgValue(
                         (cfg.name, cfg.name_span),
                         cfg.value.map(|v| (v, cfg.value_span.unwrap())),
@@ -560,7 +564,7 @@ pub fn cfg_matches(
                     UNEXPECTED_CFGS,
                     cfg.span,
                     lint_node_id,
-                    "unexpected `cfg` condition name",
+                    format!("unexpected `cfg` condition name: `{}`", cfg.name),
                     BuiltinLintDiagnostics::UnexpectedCfgName(
                         (cfg.name, cfg.name_span),
                         cfg.value.map(|v| (v, cfg.value_span.unwrap())),

diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs
@@ -659,6 +659,7 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
         msg: impl Into<SubdiagnosticMessage>,
     ) -> &mut Self);
     forward!(pub fn help(&mut self, msg: impl Into<SubdiagnosticMessage>) -> &mut Self);
+    forward!(pub fn help_once(&mut self, msg: impl Into<SubdiagnosticMessage>) -> &mut Self);
     forward!(pub fn span_help(
         &mut self,
         sp: impl Into<MultiSpan>,

diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs
@@ -727,11 +727,14 @@ pub trait LintContext: Sized {
                                 .collect::<Vec<_>>();
                             possibilities.sort();
 
+                            let mut should_print_possibilities = true;
                             if let Some((value, value_span)) = value {
                                 if best_match_values.contains(&Some(value)) {
                                     db.span_suggestion(name_span, "there is a config with a similar name and value", best_match, Applicability::MaybeIncorrect);
+                                    should_print_possibilities = false;
                                 } else if best_match_values.contains(&None) {
                                     db.span_suggestion(name_span.to(value_span), "there is a config with a similar name and no value", best_match, Applicability::MaybeIncorrect);
+                                    should_print_possibilities = false;
                                 } else if let Some(first_value) = possibilities.first() {
                                     db.span_suggestion(name_span.to(value_span), "there is a config with a similar name and different values", format!("{best_match} = \"{first_value}\""), Applicability::MaybeIncorrect);
                                 } else {
@@ -741,13 +744,25 @@ pub trait LintContext: Sized {
                                 db.span_suggestion(name_span, "there is a config with a similar name", best_match, Applicability::MaybeIncorrect);
                             }
 
-                            if !possibilities.is_empty() {
+                            if !possibilities.is_empty() && should_print_possibilities {
                                 let possibilities = possibilities.join("`, `");
                                 db.help(format!("expected values for `{best_match}` are: `{possibilities}`"));
                             }
                         } else {
                             db.span_suggestion(name_span, "there is a config with a similar name", best_match, Applicability::MaybeIncorrect);
                         }
+                    } else 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}`"));
                     }
                 },
                 BuiltinLintDiagnostics::UnexpectedCfgValue((name, name_span), value) => {

diff --git a/tests/rustdoc-ui/check-cfg/check-cfg.stderr b/tests/rustdoc-ui/check-cfg/check-cfg.stderr
@@ -1,4 +1,4 @@
-warning: unexpected `cfg` condition name
+warning: unexpected `cfg` condition name: `uniz`
   --> $DIR/check-cfg.rs:5:7
    |
 LL | #[cfg(uniz)]

diff --git a/tests/rustdoc-ui/doctest/check-cfg-test.stderr b/tests/rustdoc-ui/doctest/check-cfg-test.stderr
@@ -1,4 +1,4 @@
-warning: unexpected `cfg` condition value
+warning: unexpected `cfg` condition value: `invalid`
   --> $DIR/check-cfg-test.rs:9:7
    |
 LL | #[cfg(feature = "invalid")]

diff --git a/tests/ui/check-cfg/allow-same-level.stderr b/tests/ui/check-cfg/allow-same-level.stderr
@@ -1,9 +1,10 @@
-warning: unexpected `cfg` condition name
+warning: unexpected `cfg` condition name: `FALSE`
   --> $DIR/allow-same-level.rs:7:7
    |
 LL | #[cfg(FALSE)]
    |       ^^^^^
    |
+   = help: expected names are: `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `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`
    = note: `#[warn(unexpected_cfgs)]` on by default
 
 warning: 1 warning emitted

diff --git a/tests/ui/check-cfg/compact-names.stderr b/tests/ui/check-cfg/compact-names.stderr
@@ -1,9 +1,10 @@
-warning: unexpected `cfg` condition name
+warning: unexpected `cfg` condition name: `target_architecture`
   --> $DIR/compact-names.rs:11:28
    |
 LL | #[cfg(target(os = "linux", architecture = "arm"))]
    |                            ^^^^^^^^^^^^^^^^^^^^
    |
+   = help: expected names are: `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `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`
    = note: `#[warn(unexpected_cfgs)]` on by default
 
 warning: 1 warning emitted

diff --git a/tests/ui/check-cfg/compact-values.stderr b/tests/ui/check-cfg/compact-values.stderr
@@ -1,4 +1,4 @@
-warning: unexpected `cfg` condition value
+warning: unexpected `cfg` condition value: `X`
   --> $DIR/compact-values.rs:11:28
    |
 LL | #[cfg(target(os = "linux", arch = "X"))]

diff --git a/tests/ui/check-cfg/diagnotics.stderr b/tests/ui/check-cfg/diagnotics.stderr
@@ -1,4 +1,4 @@
-warning: unexpected `cfg` condition name
+warning: unexpected `cfg` condition name: `featur`
   --> $DIR/diagnotics.rs:4:7
    |
 LL | #[cfg(featur)]
@@ -7,19 +7,18 @@ LL | #[cfg(featur)]
    = help: expected values for `feature` are: `foo`
    = note: `#[warn(unexpected_cfgs)]` on by default
 
-warning: unexpected `cfg` condition name
+warning: unexpected `cfg` condition name: `featur`
   --> $DIR/diagnotics.rs:8:7
    |
 LL | #[cfg(featur = "foo")]
    |       ^^^^^^^^^^^^^^
    |
-   = help: expected values for `feature` are: `foo`
 help: there is a config with a similar name and value
    |
 LL | #[cfg(feature = "foo")]
    |       ~~~~~~~
 
-warning: unexpected `cfg` condition name
+warning: unexpected `cfg` condition name: `featur`
   --> $DIR/diagnotics.rs:12:7
    |
 LL | #[cfg(featur = "fo")]
@@ -31,13 +30,13 @@ help: there is a config with a similar name and different values
 LL | #[cfg(feature = "foo")]
    |       ~~~~~~~~~~~~~~~
 
-warning: unexpected `cfg` condition name
+warning: unexpected `cfg` condition name: `no_value`
   --> $DIR/diagnotics.rs:19:7
    |
 LL | #[cfg(no_value)]
    |       ^^^^^^^^ help: there is a config with a similar name: `no_values`
 
-warning: unexpected `cfg` condition name
+warning: unexpected `cfg` condition name: `no_value`
   --> $DIR/diagnotics.rs:23:7
    |
 LL | #[cfg(no_value = "foo")]
@@ -48,7 +47,7 @@ help: there is a config with a similar name and no value
 LL | #[cfg(no_values)]
    |       ~~~~~~~~~
 
-warning: unexpected `cfg` condition value
+warning: unexpected `cfg` condition value: `bar`
   --> $DIR/diagnotics.rs:27:7
    |
 LL | #[cfg(no_values = "bar")]

diff --git a/tests/ui/check-cfg/empty-names.stderr b/tests/ui/check-cfg/empty-names.stderr
@@ -1,9 +1,10 @@
-warning: unexpected `cfg` condition name
+warning: unexpected `cfg` condition name: `unknown_key`
   --> $DIR/empty-names.rs:6:7
    |
 LL | #[cfg(unknown_key = "value")]
    |       ^^^^^^^^^^^^^^^^^^^^^
    |
+   = help: expected names are: `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `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`
    = note: `#[warn(unexpected_cfgs)]` on by default
 
 warning: 1 warning emitted

diff --git a/tests/ui/check-cfg/empty-values.stderr b/tests/ui/check-cfg/empty-values.stderr
@@ -1,4 +1,4 @@
-warning: unexpected `cfg` condition value
+warning: unexpected `cfg` condition value: `value`
   --> $DIR/empty-values.rs:6:7
    |
 LL | #[cfg(test = "value")]

diff --git a/tests/ui/check-cfg/invalid-cfg-name.stderr b/tests/ui/check-cfg/invalid-cfg-name.stderr
@@ -1,4 +1,4 @@
-warning: unexpected `cfg` condition name
+warning: unexpected `cfg` condition name: `widnows`
   --> $DIR/invalid-cfg-name.rs:7:7
    |
 LL | #[cfg(widnows)]

diff --git a/tests/ui/check-cfg/invalid-cfg-value.stderr b/tests/ui/check-cfg/invalid-cfg-value.stderr
@@ -1,4 +1,4 @@
-warning: unexpected `cfg` condition value
+warning: unexpected `cfg` condition value: `sedre`
   --> $DIR/invalid-cfg-value.rs:7:7
    |
 LL | #[cfg(feature = "sedre")]
@@ -9,7 +9,7 @@ LL | #[cfg(feature = "sedre")]
    = note: expected values for `feature` are: `full`, `serde`
    = note: `#[warn(unexpected_cfgs)]` on by default
 
-warning: unexpected `cfg` condition value
+warning: unexpected `cfg` condition value: `rand`
   --> $DIR/invalid-cfg-value.rs:14:7
    |
 LL | #[cfg(feature = "rand")]