From 741005792e4209145fce6b354602a00812d52186 Mon Sep 17 00:00:00 2001 From: Veera Date: Sun, 8 Sep 2024 15:59:05 -0400 Subject: [PATCH] Implement a Method to Seal `DiagInner`'s Suggestions --- compiler/rustc_codegen_ssa/src/back/write.rs | 4 +-- .../src/annotate_snippet_emitter_writer.rs | 2 +- compiler/rustc_errors/src/diagnostic.rs | 36 ++++++++++++------- compiler/rustc_errors/src/emitter.rs | 2 +- compiler/rustc_errors/src/json.rs | 12 +++++-- compiler/rustc_errors/src/lib.rs | 35 ++++++++++++++++++ .../rustc_hir_analysis/src/collect/type_of.rs | 4 +-- .../rustc_parse/src/parser/diagnostics.rs | 10 ++++-- compiler/rustc_resolve/src/late.rs | 28 +++++++++------ .../traits/fulfillment_errors.rs | 6 ++-- ...9-resolve-after-recovered-self-ctor.stderr | 8 ----- .../misspelled-keywords/impl-trait.stderr | 4 --- .../ui/parser/misspelled-keywords/ref.stderr | 4 --- 13 files changed, 102 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index feb27c148a1..ceaaa01b047 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -16,7 +16,7 @@ use rustc_errors::translation::Translate; use rustc_errors::{ Diag, DiagArgMap, DiagCtxt, DiagMessage, ErrCode, FatalError, FluentBundle, Level, MultiSpan, - Style, + Style, Suggestions, }; use rustc_fs_util::link_or_copy; use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; @@ -1903,7 +1903,7 @@ fn emit_diagnostic(&mut self, mut diag: rustc_errors::DiagInner) { // Check that we aren't missing anything interesting when converting to // the cut-down local `DiagInner`. assert_eq!(diag.span, MultiSpan::new()); - assert_eq!(diag.suggestions, Ok(vec![])); + assert_eq!(diag.suggestions, Suggestions::Enabled(vec![])); assert_eq!(diag.sort_span, rustc_span::DUMMY_SP); assert_eq!(diag.is_lint, None); // No sensible check for `diag.emitted_at`. diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index d71ae9d210d..556e6d46f89 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -48,7 +48,7 @@ impl Emitter for AnnotateSnippetEmitter { fn emit_diagnostic(&mut self, mut diag: DiagInner) { let fluent_args = to_fluent_args(diag.args.iter()); - let mut suggestions = diag.suggestions.unwrap_or(vec![]); + let mut suggestions = diag.suggestions.unwrap_tag(); self.primary_span_formatted(&mut diag.span, &mut suggestions, &fluent_args); self.fix_multispans_in_extern_macros_and_render_macro_backtrace( diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 7a4d8dba179..6b756cff225 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -19,13 +19,9 @@ use crate::{ CodeSuggestion, DiagCtxtHandle, DiagMessage, ErrCode, ErrorGuaranteed, ExplicitBug, Level, MultiSpan, StashKey, SubdiagMessage, Substitution, SubstitutionPart, SuggestionStyle, + Suggestions, }; -/// Error type for `DiagInner`'s `suggestions` field, indicating that -/// `.disable_suggestions()` was called on the `DiagInner`. -#[derive(Clone, Debug, PartialEq, Eq, Hash, Encodable, Decodable)] -pub struct SuggestionsDisabled; - /// Simplified version of `FluentArg` that can implement `Encodable` and `Decodable`. Collection of /// `DiagArg` are converted to `FluentArgs` (consuming the collection) at the start of diagnostic /// emission. @@ -296,7 +292,7 @@ pub struct DiagInner { pub code: Option, pub span: MultiSpan, pub children: Vec, - pub suggestions: Result, SuggestionsDisabled>, + pub suggestions: Suggestions, pub args: DiagArgMap, /// This is not used for highlighting or rendering any error message. Rather, it can be used @@ -325,7 +321,7 @@ pub fn new_with_messages(level: Level, messages: Vec<(DiagMessage, Style)>) -> S code: None, span: MultiSpan::new(), children: vec![], - suggestions: Ok(vec![]), + suggestions: Suggestions::Enabled(vec![]), args: Default::default(), sort_span: DUMMY_SP, is_lint: None, @@ -409,7 +405,7 @@ fn keys( &Option, &MultiSpan, &[Subdiag], - &Result, SuggestionsDisabled>, + &Suggestions, Vec<(&DiagArgName, &DiagArgValue)>, &Option, ) { @@ -823,16 +819,32 @@ pub fn span_help>( self } - /// Disallow attaching suggestions this diagnostic. + /// Disallow attaching suggestions to this diagnostic. /// Any suggestions attached e.g. with the `span_suggestion_*` methods /// (before and after the call to `disable_suggestions`) will be ignored. #[rustc_lint_diagnostics] pub fn disable_suggestions(&mut self) -> &mut Self { - self.suggestions = Err(SuggestionsDisabled); + self.suggestions = Suggestions::Disabled; self } - /// Helper for pushing to `self.suggestions`, if available (not disable). + /// Prevent new suggestions from being added to this diagnostic. + /// + /// Suggestions added before the call to `.seal_suggestions()` will be preserved + /// and new suggestions will be ignored. + #[rustc_lint_diagnostics] + pub fn seal_suggestions(&mut self) -> &mut Self { + if let Suggestions::Enabled(suggestions) = &mut self.suggestions { + let suggestions_slice = std::mem::take(suggestions).into_boxed_slice(); + self.suggestions = Suggestions::Sealed(suggestions_slice); + } + self + } + + /// Helper for pushing to `self.suggestions`. + /// + /// A new suggestion is added if suggestions are enabled for this diagnostic. + /// Otherwise, they are ignored. #[rustc_lint_diagnostics] fn push_suggestion(&mut self, suggestion: CodeSuggestion) { for subst in &suggestion.substitutions { @@ -846,7 +858,7 @@ fn push_suggestion(&mut self, suggestion: CodeSuggestion) { } } - if let Ok(suggestions) = &mut self.suggestions { + if let Suggestions::Enabled(suggestions) = &mut self.suggestions { suggestions.push(suggestion); } } diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 2b135df91a4..3ab371b8057 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -498,7 +498,7 @@ fn source_map(&self) -> Option<&Lrc> { fn emit_diagnostic(&mut self, mut diag: DiagInner) { let fluent_args = to_fluent_args(diag.args.iter()); - let mut suggestions = diag.suggestions.unwrap_or(vec![]); + let mut suggestions = diag.suggestions.unwrap_tag(); self.primary_span_formatted(&mut diag.span, &mut suggestions, &fluent_args); self.fix_multispans_in_extern_macros_and_render_macro_backtrace( diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 32e59f9ab03..6a2ecf13f7b 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -33,7 +33,8 @@ use crate::registry::Registry; use crate::translation::{to_fluent_args, Translate}; use crate::{ - CodeSuggestion, FluentBundle, LazyFallbackBundle, MultiSpan, SpanLabel, Subdiag, TerminalUrl, + CodeSuggestion, FluentBundle, LazyFallbackBundle, MultiSpan, SpanLabel, Subdiag, Suggestions, + TerminalUrl, }; #[cfg(test)] @@ -292,7 +293,7 @@ impl Diagnostic { /// Converts from `rustc_errors::DiagInner` to `Diagnostic`. fn from_errors_diagnostic(diag: crate::DiagInner, je: &JsonEmitter) -> Diagnostic { let args = to_fluent_args(diag.args.iter()); - let sugg = diag.suggestions.iter().flatten().map(|sugg| { + let sugg_to_diag = |sugg: &CodeSuggestion| { let translated_message = je.translate_message(&sugg.msg, &args).map_err(Report::new).unwrap(); Diagnostic { @@ -303,7 +304,12 @@ fn from_errors_diagnostic(diag: crate::DiagInner, je: &JsonEmitter) -> Diagnosti children: vec![], rendered: None, } - }); + }; + let sugg = match &diag.suggestions { + Suggestions::Enabled(suggestions) => suggestions.iter().map(sugg_to_diag), + Suggestions::Sealed(suggestions) => suggestions.iter().map(sugg_to_diag), + Suggestions::Disabled => [].iter().map(sugg_to_diag), + }; // generate regular command line output and store it in the json diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 13da1721a4a..094e3010471 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -126,6 +126,41 @@ fn hide_inline(&self) -> bool { } } +/// Represents the help messages seen on a diagnostic. +#[derive(Clone, Debug, PartialEq, Hash, Encodable, Decodable)] +pub enum Suggestions { + /// Indicates that new suggestions can be added or removed from this diagnostic. + /// + /// `DiagInner`'s new_* methods initialize the `suggestions` field with + /// this variant. Also, this is the default variant for `Suggestions`. + Enabled(Vec), + /// Indicates that suggestions cannot be added or removed from this diagnostic. + /// + /// Gets toggled when `.seal_suggestions()` is called on the `DiagInner`. + Sealed(Box<[CodeSuggestion]>), + /// Indicates that no suggestion is available for this diagnostic. + /// + /// Gets toggled when `.disable_suggestions()` is called on the `DiagInner`. + Disabled, +} + +impl Suggestions { + /// Returns the underlying list of suggestions. + pub fn unwrap_tag(self) -> Vec { + match self { + Suggestions::Enabled(suggestions) => suggestions, + Suggestions::Sealed(suggestions) => suggestions.into_vec(), + Suggestions::Disabled => Vec::new(), + } + } +} + +impl Default for Suggestions { + fn default() -> Self { + Self::Enabled(vec![]) + } +} + #[derive(Clone, Debug, PartialEq, Hash, Encodable, Decodable)] pub struct CodeSuggestion { /// Each substitute can have multiple variants due to multiple diff --git a/compiler/rustc_hir_analysis/src/collect/type_of.rs b/compiler/rustc_hir_analysis/src/collect/type_of.rs index 5cb90e97eef..b877cacd998 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of.rs @@ -1,6 +1,6 @@ use core::ops::ControlFlow; -use rustc_errors::{Applicability, StashKey}; +use rustc_errors::{Applicability, StashKey, Suggestions}; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::HirId; @@ -670,7 +670,7 @@ fn infer_placeholder_type<'tcx>( // The parser provided a sub-optimal `HasPlaceholders` suggestion for the type. // We are typeck and have the real type, so remove that and suggest the actual type. - if let Ok(suggestions) = &mut err.suggestions { + if let Suggestions::Enabled(suggestions) = &mut err.suggestions { suggestions.clear(); } diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index bee73c58cb7..fd488cf1d31 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -16,7 +16,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_errors::{ pluralize, Applicability, Diag, DiagCtxtHandle, ErrorGuaranteed, FatalError, PErr, PResult, - Subdiagnostic, + Subdiagnostic, Suggestions, }; use rustc_session::errors::ExprParenthesesNeeded; use rustc_span::edit_distance::find_best_match_for_name; @@ -775,7 +775,7 @@ fn is_ident_eq_keyword(found: &TokenKind, expected: &TokenType) -> bool { } // Check for misspelled keywords if there are no suggestions added to the diagnostic. - if err.suggestions.as_ref().is_ok_and(|code_suggestions| code_suggestions.is_empty()) { + if matches!(&err.suggestions, Suggestions::Enabled(list) if list.is_empty()) { self.check_for_misspelled_kw(&mut err, &expected); } Err(err) @@ -803,6 +803,9 @@ fn check_for_misspelled_kw(&self, err: &mut Diag<'_>, expected: &[TokenType]) { && let Some(misspelled_kw) = find_similar_kw(curr_ident, &expected_keywords) { err.subdiagnostic(misspelled_kw); + // We don't want other suggestions to be added as they are most likely meaningless + // when there is a misspelled keyword. + err.seal_suggestions(); } else if let Some((prev_ident, _)) = self.prev_token.ident() && !prev_ident.is_used_keyword() { @@ -818,6 +821,9 @@ fn check_for_misspelled_kw(&self, err: &mut Diag<'_>, expected: &[TokenType]) { // positives like suggesting keyword `for` for `extern crate foo {}`. if let Some(misspelled_kw) = find_similar_kw(prev_ident, &all_keywords) { err.subdiagnostic(misspelled_kw); + // We don't want other suggestions to be added as they are most likely meaningless + // when there is a misspelled keyword. + err.seal_suggestions(); } } } diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 0aa351cad40..80898dc4bd1 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -17,7 +17,7 @@ use rustc_ast::*; use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap}; use rustc_errors::codes::*; -use rustc_errors::{Applicability, DiagArgValue, IntoDiagArg, StashKey}; +use rustc_errors::{Applicability, DiagArgValue, IntoDiagArg, StashKey, Suggestions}; use rustc_hir::def::Namespace::{self, *}; use rustc_hir::def::{self, CtorKind, DefKind, LifetimeRes, NonMacroAttrKind, PartialRes, PerNS}; use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID, LOCAL_CRATE}; @@ -4052,17 +4052,23 @@ fn smart_resolve_path_fragment( err.sort_span = parent_err.sort_span; err.is_lint = parent_err.is_lint.clone(); - // merge the parent's suggestions with the typo suggestions - fn append_result(res1: &mut Result, E>, res2: Result, E>) { - match res1 { - Ok(vec1) => match res2 { - Ok(mut vec2) => vec1.append(&mut vec2), - Err(e) => *res1 = Err(e), - }, - Err(_) => (), - }; + // merge the parent_err's suggestions with the typo (err's) suggestions + match &mut err.suggestions { + Suggestions::Enabled(typo_suggestions) => match &mut parent_err.suggestions { + Suggestions::Enabled(parent_suggestions) => { + // If both suggestions are enabled, append parent_err's suggestions to err's suggestions. + typo_suggestions.append(parent_suggestions) + } + Suggestions::Sealed(_) | Suggestions::Disabled => { + // If the parent's suggestions are either sealed or disabled, it signifies that + // new suggestions cannot be added or removed from the diagnostic. Therefore, + // we assign both types of suggestions to err's suggestions and discard the + // existing suggestions in err. + err.suggestions = std::mem::take(&mut parent_err.suggestions); + } + }, + Suggestions::Sealed(_) | Suggestions::Disabled => (), } - append_result(&mut err.suggestions, parent_err.suggestions.clone()); parent_err.cancel(); diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index 5918686213a..93c0870406d 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -6,7 +6,7 @@ use rustc_errors::codes::*; use rustc_errors::{ pluralize, struct_span_code_err, Applicability, Diag, ErrorGuaranteed, MultiSpan, StashKey, - StringPart, + StringPart, Suggestions, }; use rustc_hir::def::Namespace; use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE}; @@ -2136,8 +2136,8 @@ pub fn note_obligation_cause( if let Some(span) = err.span.primary_span() && let Some(mut diag) = self.dcx().steal_non_err(span, StashKey::AssociatedTypeSuggestion) - && let Ok(ref mut s1) = err.suggestions - && let Ok(ref mut s2) = diag.suggestions + && let Suggestions::Enabled(ref mut s1) = err.suggestions + && let Suggestions::Enabled(ref mut s2) = diag.suggestions { s1.append(s2); diag.cancel() diff --git a/tests/ui/parser/issues/issue-70549-resolve-after-recovered-self-ctor.stderr b/tests/ui/parser/issues/issue-70549-resolve-after-recovered-self-ctor.stderr index 00f372bc008..c2c0faa21d1 100644 --- a/tests/ui/parser/issues/issue-70549-resolve-after-recovered-self-ctor.stderr +++ b/tests/ui/parser/issues/issue-70549-resolve-after-recovered-self-ctor.stderr @@ -14,10 +14,6 @@ help: there is a keyword `mut` with a similar name | LL | fn foo(&mut Self) {} | ~~~ -help: declare the type after the parameter binding - | -LL | fn foo(: ) {} - | ~~~~~~~~~~~~~~~~~~~~ error: unexpected lifetime `'static` in pattern --> $DIR/issue-70549-resolve-after-recovered-self-ctor.rs:8:13 @@ -47,10 +43,6 @@ help: there is a keyword `mut` with a similar name | LL | fn bar(&'static mut Self) {} | ~~~ -help: declare the type after the parameter binding - | -LL | fn bar(: ) {} - | ~~~~~~~~~~~~~~~~~~~~ error: expected one of `:`, `@`, or `|`, found keyword `Self` --> $DIR/issue-70549-resolve-after-recovered-self-ctor.rs:14:17 diff --git a/tests/ui/parser/misspelled-keywords/impl-trait.stderr b/tests/ui/parser/misspelled-keywords/impl-trait.stderr index 15a8f99b8b1..02a0c808311 100644 --- a/tests/ui/parser/misspelled-keywords/impl-trait.stderr +++ b/tests/ui/parser/misspelled-keywords/impl-trait.stderr @@ -8,10 +8,6 @@ help: there is a keyword `impl` with a similar name | LL | fn code() -> u8 {} | ~~~~ -help: you might have meant to end the type parameters here - | -LL | fn code Debug>() -> u8 {} - | + error: aborting due to 1 previous error diff --git a/tests/ui/parser/misspelled-keywords/ref.stderr b/tests/ui/parser/misspelled-keywords/ref.stderr index 3a79b7bdb00..b8b52702314 100644 --- a/tests/ui/parser/misspelled-keywords/ref.stderr +++ b/tests/ui/parser/misspelled-keywords/ref.stderr @@ -8,10 +8,6 @@ help: there is a keyword `ref` with a similar name | LL | Some(ref list) => println!("{list:?}"), | ~~~ -help: missing `,` - | -LL | Some(refe, list) => println!("{list:?}"), - | + error[E0023]: this pattern has 2 fields, but the corresponding tuple variant has 1 field --> $DIR/ref.rs:4:14