Convert unexpected_cfg_name to struct diagnostics

Xiretza · Xiretza · commit b1ae16767f46 · 2024-04-23T16:18:56.000Z
diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl
@@ -699,6 +699,22 @@ lint_undropped_manually_drops = calls to `std::mem::drop` with `std::mem::Manual
     .label = argument has type `{$arg_ty}`
     .suggestion = use `std::mem::ManuallyDrop::into_inner` to get the inner value
 
+lint_unexpected_cfg_name_add_cargo_feature = consider using a Cargo feature instead or adding `{$build_rs_println}` to the top of a `build.rs`
+lint_unexpected_cfg_name_add_cmdline_arg = to expect this configuration use `{$cmdline_arg}`
+lint_unexpected_cfg_name_define_features = consider defining some features in `Cargo.toml`
+lint_unexpected_cfg_name_doc_cargo = see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration
+lint_unexpected_cfg_name_doc_rustc = see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration
+lint_unexpected_cfg_name_expected_names = expected names are: `{$possibilities}`{$and_more ->
+        [0] {""}
+        *[others]  and {$and_more} more
+    }
+lint_unexpected_cfg_name_expected_values = expected values for `{$best_match}` are: `{$possibilities}`
+lint_unexpected_cfg_name_similar_name = there is a config with a similar name
+lint_unexpected_cfg_name_similar_name_different_values = there is a config with a similar name and different values
+lint_unexpected_cfg_name_similar_name_no_value = there is a config with a similar name and no value
+lint_unexpected_cfg_name_similar_name_value = there is a config with a similar name and value
+lint_unexpected_cfg_name_with_similar_value = found config with similar value
+
 lint_ungated_async_fn_track_caller = `#[track_caller]` on async functions is a no-op
      .label = this function will not propagate the caller location
 
diff --git a/compiler/rustc_lint/src/context/diagnostics/check_cfg.rs b/compiler/rustc_lint/src/context/diagnostics/check_cfg.rs
@@ -3,28 +3,38 @@ use rustc_session::{config::ExpectedValues, Session};
 use rustc_span::edit_distance::find_best_match_for_name;
 use rustc_span::{sym, Span, Symbol};
 
+use crate::lints;
+
 const MAX_CHECK_CFG_NAMES_OR_VALUES: usize = 35;
 
-fn check_cfg_expected_note(
+fn sort_and_truncate_possibilities(
     sess: &Session,
-    possibilities: &[Symbol],
-    type_: &str,
-    name: Option<Symbol>,
-    suffix: &str,
-) -> String {
-    use std::fmt::Write;
-
+    mut possibilities: Vec<Symbol>,
+) -> (Vec<Symbol>, usize) {
     let n_possibilities = if sess.opts.unstable_opts.check_cfg_all_expected {
         possibilities.len()
     } else {
         std::cmp::min(possibilities.len(), MAX_CHECK_CFG_NAMES_OR_VALUES)
     };
 
-    let mut possibilities = possibilities.iter().map(Symbol::as_str).collect::<Vec<_>>();
-    possibilities.sort();
+    possibilities.sort_by(|s1, s2| s1.as_str().cmp(s2.as_str()));
 
     let and_more = possibilities.len().saturating_sub(n_possibilities);
-    let possibilities = possibilities[..n_possibilities].join("`, `");
+    possibilities.truncate(n_possibilities);
+    (possibilities, and_more)
+}
+
+fn check_cfg_expected_note(
+    sess: &Session,
+    possibilities: Vec<Symbol>,
+    type_: &str,
+    name: Option<Symbol>,
+    suffix: &str,
+) -> String {
+    use std::fmt::Write;
+
+    let (possibilities, and_more) = sort_and_truncate_possibilities(sess, possibilities);
+    let possibilities = possibilities.iter().map(Symbol::as_str).collect::<Vec<_>>().join("`, `");
 
     let mut note = String::with_capacity(50 + possibilities.len());
 
@@ -68,91 +78,95 @@ pub(super) fn unexpected_cfg_name(
     let is_from_cargo = rustc_session::utils::was_invoked_from_cargo();
     let mut is_feature_cfg = name == sym::feature;
 
-    if is_feature_cfg && is_from_cargo {
-        diag.help("consider defining some features in `Cargo.toml`");
+    let code_sugg = if is_feature_cfg && is_from_cargo {
+        lints::UnexpectedCfgNameCodeSugg::ConsiderDefiningFeatures
     // Suggest the most probable if we found one
     } else if let Some(best_match) = find_best_match_for_name(&possibilities, name, None) {
+        is_feature_cfg |= best_match == sym::feature;
+
         if let Some(ExpectedValues::Some(best_match_values)) =
             sess.psess.check_config.expecteds.get(&best_match)
         {
             // We will soon sort, so the initial order does not matter.
             #[allow(rustc::potential_query_instability)]
-            let mut possibilities =
-                best_match_values.iter().flatten().map(Symbol::as_str).collect::<Vec<_>>();
-            possibilities.sort();
+            let mut possibilities = best_match_values.iter().flatten().collect::<Vec<_>>();
+            possibilities.sort_by_key(|s| s.as_str());
+
+            let get_possibilities_sub = || {
+                if !possibilities.is_empty() {
+                    let possibilities =
+                        possibilities.iter().copied().cloned().collect::<Vec<_>>().into();
+                    Some(lints::UnexpectedCfgNameExpectedValues { best_match, possibilities })
+                } else {
+                    None
+                }
+            };
 
-            let mut should_print_possibilities = true;
             if let Some((value, value_span)) = value {
                 if best_match_values.contains(&Some(value)) {
-                    diag.span_suggestion(
-                        name_span,
-                        "there is a config with a similar name and value",
-                        best_match,
-                        Applicability::MaybeIncorrect,
-                    );
-                    should_print_possibilities = false;
+                    lints::UnexpectedCfgNameCodeSugg::SimilarNameAndValue {
+                        span: name_span,
+                        code: best_match.to_string(),
+                    }
                 } else if best_match_values.contains(&None) {
-                    diag.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;
+                    lints::UnexpectedCfgNameCodeSugg::SimilarNameNoValue {
+                        span: name_span.to(value_span),
+                        code: best_match.to_string(),
+                    }
                 } else if let Some(first_value) = possibilities.first() {
-                    diag.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,
-                    );
+                    lints::UnexpectedCfgNameCodeSugg::SimilarNameDifferentValues {
+                        span: name_span.to(value_span),
+                        code: format!("{best_match} = \"{first_value}\""),
+                        expected: get_possibilities_sub(),
+                    }
                 } else {
-                    diag.span_suggestion(
-                        name_span.to(value_span),
-                        "there is a config with a similar name and different values",
-                        best_match,
-                        Applicability::MaybeIncorrect,
-                    );
-                };
+                    lints::UnexpectedCfgNameCodeSugg::SimilarNameDifferentValues {
+                        span: name_span.to(value_span),
+                        code: best_match.to_string(),
+                        expected: get_possibilities_sub(),
+                    }
+                }
             } else {
-                diag.span_suggestion(
-                    name_span,
-                    "there is a config with a similar name",
-                    best_match,
-                    Applicability::MaybeIncorrect,
-                );
-            }
-
-            if !possibilities.is_empty() && should_print_possibilities {
-                let possibilities = possibilities.join("`, `");
-                diag.help(format!("expected values for `{best_match}` are: `{possibilities}`"));
+                lints::UnexpectedCfgNameCodeSugg::SimilarName {
+                    span: name_span,
+                    code: best_match.to_string(),
+                    expected: get_possibilities_sub(),
+                }
             }
         } else {
-            diag.span_suggestion(
-                name_span,
-                "there is a config with a similar name",
-                best_match,
-                Applicability::MaybeIncorrect,
-            );
+            lints::UnexpectedCfgNameCodeSugg::SimilarName {
+                span: name_span,
+                code: best_match.to_string(),
+                expected: None,
+            }
         }
-
-        is_feature_cfg |= best_match == sym::feature;
     } else {
-        if !names_possibilities.is_empty() && names_possibilities.len() <= 3 {
+        let similar_values = if !names_possibilities.is_empty() && names_possibilities.len() <= 3 {
             names_possibilities.sort();
-            for cfg_name in names_possibilities.iter() {
-                diag.span_suggestion(
-                    name_span,
-                    "found config with similar value",
-                    format!("{cfg_name} = \"{name}\""),
-                    Applicability::MaybeIncorrect,
-                );
-            }
-        }
-        if !possibilities.is_empty() {
-            diag.help_once(check_cfg_expected_note(sess, &possibilities, "names", None, ""));
+            names_possibilities
+                .iter()
+                .map(|cfg_name| lints::UnexpectedCfgNameWithSimilarValue {
+                    span: name_span,
+                    code: format!("{cfg_name} = \"{name}\""),
+                })
+                .collect()
+        } else {
+            vec![]
+        };
+        let expected_names = if !possibilities.is_empty() {
+            let (possibilities, and_more) = sort_and_truncate_possibilities(sess, possibilities);
+            Some(lints::UnexpectedCfgNameExpectedNames {
+                possibilities: possibilities.into(),
+                and_more,
+            })
+        } else {
+            None
+        };
+        lints::UnexpectedCfgNameCodeSugg::SimilarValues {
+            with_similar_values: similar_values,
+            expected_names,
         }
-    }
+    };
 
     let inst = if let Some((value, _value_span)) = value {
         let pre = if is_from_cargo { "\\" } else { "" };
@@ -161,15 +175,20 @@ pub(super) fn unexpected_cfg_name(
         format!("cfg({name})")
     };
 
-    if is_from_cargo {
-        if !is_feature_cfg {
-            diag.help(format!("consider using a Cargo feature instead or adding `println!(\"cargo:rustc-check-cfg={inst}\");` to the top of a `build.rs`"));
-        }
-        diag.note("see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration");
+    let meta_sugg = if is_from_cargo {
+        let sub = if !is_feature_cfg {
+            Some(lints::UnexpectedCfgNameCargoSugg {
+                build_rs_println: format!("println!(\"cargo:rustc-check-cfg={inst}\");"),
+            })
+        } else {
+            None
+        };
+        lints::UnexpectedCfgNameMetaSugg::FromCargo { sub }
     } else {
-        diag.help(format!("to expect this configuration use `--check-cfg={inst}`"));
-        diag.note("see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration");
-    }
+        lints::UnexpectedCfgNameMetaSugg::Standalone { cmdline_arg: format!("--check-cfg={inst}") }
+    };
+
+    diag.subdiagnostic(diag.dcx, lints::UnexpectedCfgNameSub { code_sugg, meta_sugg });
 }
 
 pub(super) fn unexpected_cfg_value(
@@ -200,7 +219,7 @@ pub(super) fn unexpected_cfg_value(
     if !possibilities.is_empty() {
         diag.note(check_cfg_expected_note(
             sess,
-            &possibilities,
+            possibilities.clone(),
             "values",
             Some(name),
             if have_none_possibility { "(none), " } else { "" },
diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs
@@ -5,7 +5,7 @@ use std::num::NonZero;
 use crate::errors::RequestedLevel;
 use crate::fluent_generated as fluent;
 use rustc_errors::{
-    codes::*, Applicability, Diag, DiagArgValue, DiagMessage, DiagStyledString,
+    codes::*, Applicability, Diag, DiagArgValue, DiagMessage, DiagStyledString, DiagSymbolList,
     ElidedLifetimeInPathSubdiag, EmissionGuarantee, LintDiagnostic, SubdiagMessageOp,
     Subdiagnostic, SuggestionStyle,
 };
@@ -2340,3 +2340,112 @@ pub struct BreakWithLabelAndLoopSub {
     #[suggestion_part(code = ")")]
     pub right: Span,
 }
+
+#[derive(Subdiagnostic)]
+pub struct UnexpectedCfgNameSub {
+    #[subdiagnostic]
+    pub code_sugg: UnexpectedCfgNameCodeSugg,
+    #[subdiagnostic]
+    pub meta_sugg: UnexpectedCfgNameMetaSugg,
+}
+
+#[derive(Subdiagnostic)]
+pub enum UnexpectedCfgNameMetaSugg {
+    #[note(lint_unexpected_cfg_name_doc_cargo)]
+    FromCargo {
+        #[subdiagnostic]
+        sub: Option<UnexpectedCfgNameCargoSugg>,
+    },
+    #[help(lint_unexpected_cfg_name_add_cmdline_arg)]
+    #[note(lint_unexpected_cfg_name_doc_rustc)]
+    Standalone { cmdline_arg: String },
+}
+
+#[derive(Subdiagnostic)]
+#[help(lint_unexpected_cfg_name_add_cargo_feature)]
+pub struct UnexpectedCfgNameCargoSugg {
+    pub build_rs_println: String,
+}
+
+#[derive(Subdiagnostic)]
+pub enum UnexpectedCfgNameCodeSugg {
+    #[help(lint_unexpected_cfg_name_define_features)]
+    ConsiderDefiningFeatures,
+    #[suggestion(
+        lint_unexpected_cfg_name_similar_name_value,
+        applicability = "maybe-incorrect",
+        code = "{code}"
+    )]
+    SimilarNameAndValue {
+        #[primary_span]
+        span: Span,
+        code: String,
+    },
+    #[suggestion(
+        lint_unexpected_cfg_name_similar_name_no_value,
+        applicability = "maybe-incorrect",
+        code = "{code}"
+    )]
+    SimilarNameNoValue {
+        #[primary_span]
+        span: Span,
+        code: String,
+    },
+    #[suggestion(
+        lint_unexpected_cfg_name_similar_name_different_values,
+        applicability = "maybe-incorrect",
+        code = "{code}"
+    )]
+    SimilarNameDifferentValues {
+        #[primary_span]
+        span: Span,
+        code: String,
+        #[subdiagnostic]
+        expected: Option<UnexpectedCfgNameExpectedValues>,
+    },
+    #[suggestion(
+        lint_unexpected_cfg_name_similar_name,
+        applicability = "maybe-incorrect",
+        code = "{code}"
+    )]
+    SimilarName {
+        #[primary_span]
+        span: Span,
+        code: String,
+        #[subdiagnostic]
+        expected: Option<UnexpectedCfgNameExpectedValues>,
+    },
+    SimilarValues {
+        #[subdiagnostic]
+        with_similar_values: Vec<UnexpectedCfgNameWithSimilarValue>,
+        #[subdiagnostic]
+        expected_names: Option<UnexpectedCfgNameExpectedNames>,
+    },
+}
+
+#[derive(Subdiagnostic)]
+#[help(lint_unexpected_cfg_name_expected_values)]
+pub struct UnexpectedCfgNameExpectedValues {
+    pub best_match: Symbol,
+    pub possibilities: DiagSymbolList,
+}
+
+#[derive(Subdiagnostic)]
+#[suggestion(
+    lint_unexpected_cfg_name_with_similar_value,
+    applicability = "maybe-incorrect",
+    code = "{code}"
+)]
+pub struct UnexpectedCfgNameWithSimilarValue {
+    #[primary_span]
+    pub span: Span,
+    pub code: String,
+}
+
+#[derive(Subdiagnostic)]
+// FIXME: help_once
+#[help(lint_unexpected_cfg_name_expected_names)]
+pub struct UnexpectedCfgNameExpectedNames {
+    pub possibilities: DiagSymbolList,
+    pub and_more: usize,
+}
