Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a Method to Seal DiagInner's Suggestions #130116

Merged
merged 1 commit into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
36 changes: 24 additions & 12 deletions compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -409,7 +405,7 @@ impl DiagInner {
&Option<ErrCode>,
&MultiSpan,
&[Subdiag],
&Result<Vec<CodeSuggestion>, SuggestionsDisabled>,
&Suggestions,
Vec<(&DiagArgName, &DiagArgValue)>,
&Option<IsLint>,
) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
12 changes: 9 additions & 3 deletions compiler/rustc_errors/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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 {
Expand All @@ -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

Expand Down
35 changes: 35 additions & 0 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_analysis/src/collect/type_of.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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();
}

Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
{
Expand All @@ -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();
}
}
}
Expand Down
28 changes: 17 additions & 11 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -4052,17 +4052,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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -2136,8 +2136,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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions tests/ui/parser/misspelled-keywords/impl-trait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

4 changes: 0 additions & 4 deletions tests/ui/parser/misspelled-keywords/ref.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading