diff options
| author | Matthias Krüger <matthias.krueger@famsik.de> | 2024-09-18 04:42:31 +0200 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-09-18 04:42:31 +0200 |
| commit | 09b255d3d4fdfce05e5878f1e919d5ca84bc9962 (patch) | |
| tree | 5653b057c14db34c33937e31180428c561ec9141 | |
| parent | f7b4c72c8ffb320616bda27f70bb8e0a341673fb (diff) | |
| parent | 741005792e4209145fce6b354602a00812d52186 (diff) | |
| download | rust-09b255d3d4fdfce05e5878f1e919d5ca84bc9962.tar.gz rust-09b255d3d4fdfce05e5878f1e919d5ca84bc9962.zip | |
Rollup merge of #130116 - veera-sivarajan:freeze-suggestions, r=chenyukang
Implement a Method to Seal `DiagInner`'s Suggestions This PR adds a method on `DiagInner` called `.seal_suggestions()` to prevent new suggestions from being added while preserving existing suggestions. This is useful because currently there is no way to prevent new suggestions from being added to a diagnostic. `.disable_suggestions()` is the closest but it gets rid of all suggestions before and after the call. Therefore, `.seal_suggestions()` can be used when, for example, misspelled keyword is detected and reported. In such cases, we may want to prevent other suggestions from being added to the diagnostic, as they would likely be meaningless once the misspelled keyword is identified. For context: https://github.com/rust-lang/rust/pull/129899#discussion_r1741307132 To store an additional state, the type of the `suggestions` field in `DiagInner` was changed into a three variant enum. While this change affects files across different crates, care was taken to preserve the existing code's semantics. This is validated by the fact that all UI tests pass without any modifications. r? chenyukang
| -rw-r--r-- | compiler/rustc_codegen_ssa/src/back/write.rs | 4 | ||||
| -rw-r--r-- | compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs | 2 | ||||
| -rw-r--r-- | compiler/rustc_errors/src/diagnostic.rs | 36 | ||||
| -rw-r--r-- | compiler/rustc_errors/src/emitter.rs | 2 | ||||
| -rw-r--r-- | compiler/rustc_errors/src/json.rs | 12 | ||||
| -rw-r--r-- | compiler/rustc_errors/src/lib.rs | 35 | ||||
| -rw-r--r-- | compiler/rustc_hir_analysis/src/collect/type_of.rs | 4 | ||||
| -rw-r--r-- | compiler/rustc_parse/src/parser/diagnostics.rs | 10 | ||||
| -rw-r--r-- | compiler/rustc_resolve/src/late.rs | 28 | ||||
| -rw-r--r-- | compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs | 6 | ||||
| -rw-r--r-- | tests/ui/parser/issues/issue-70549-resolve-after-recovered-self-ctor.stderr | 8 | ||||
| -rw-r--r-- | tests/ui/parser/misspelled-keywords/impl-trait.stderr | 4 | ||||
| -rw-r--r-- | tests/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 d83cb799b57..c170cd41ec4 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::emitter::Emitter; 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 @@ impl Emitter for SharedEmitter { // 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::snippet::Style; 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<ErrCode>, pub span: MultiSpan, pub children: Vec<Subdiag>, - pub suggestions: Result<Vec<CodeSuggestion>, 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 @@ impl DiagInner { 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 @@ impl DiagInner { &Option<ErrCode>, &MultiSpan, &[Subdiag], - &Result<Vec<CodeSuggestion>, SuggestionsDisabled>, + &Suggestions, Vec<(&DiagArgName, &DiagArgValue)>, &Option<IsLint>, ) { @@ -823,16 +819,32 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { 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 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { } } - 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 @@ impl Emitter for HumanEmitter { 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::emitter::{ 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 @@ impl Diagnostic { 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 @@ impl SuggestionStyle { } } +/// 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<CodeSuggestion>), + /// 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<CodeSuggestion> { + 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_ast_pretty::pprust; 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 @@ impl<'a> Parser<'a> { } // 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 @@ impl<'a> Parser<'a> { && 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 @@ impl<'a> Parser<'a> { // 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 4bf2cc287da..ac03a3ac42c 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -17,7 +17,7 @@ use rustc_ast::visit::{visit_opt, walk_list, AssocCtxt, BoundKind, FnCtxt, FnKin 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}; @@ -4085,17 +4085,23 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { 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<T, E>(res1: &mut Result<Vec<T>, E>, res2: Result<Vec<T>, 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 85a6ef5caab..2de6ee9cf91 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_data_structures::unord::UnordSet; 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}; @@ -2137,8 +2137,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { 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(<identifier>: <type>) {} - | ~~~~~~~~~~~~~~~~~~~~ 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(<identifier>: <type>) {} - | ~~~~~~~~~~~~~~~~~~~~ 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<T: impl Debug>() -> u8 {} | ~~~~ -help: you might have meant to end the type parameters here - | -LL | fn code<T: impll> 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 |
