Skip to content

Commit

Permalink
Auto merge of rust-lang#87915 - estebank:fancy-spans, r=oli-obk
Browse files Browse the repository at this point in the history
Use smaller spans for some structured suggestions

Use more accurate suggestion spans for

* argument parse error
* fully qualified path
* missing code block type
* numeric casts
  • Loading branch information
bors committed Sep 13, 2021
2 parents b0ee495 + 34d1963 commit 9bb77da
Show file tree
Hide file tree
Showing 60 changed files with 1,200 additions and 958 deletions.
15 changes: 15 additions & 0 deletions compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,21 @@ impl Diagnostic {
)
}

/// Show a suggestion that has multiple parts to it, always as it's own subdiagnostic.
/// In other words, multiple changes need to be applied as part of this suggestion.
pub fn multipart_suggestion_verbose(
&mut self,
msg: &str,
suggestion: Vec<(Span, String)>,
applicability: Applicability,
) -> &mut Self {
self.multipart_suggestion_with_style(
msg,
suggestion,
applicability,
SuggestionStyle::ShowAlways,
)
}
/// [`Diagnostic::multipart_suggestion()`] but you can set the [`SuggestionStyle`].
pub fn multipart_suggestion_with_style(
&mut self,
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_errors/src/diagnostic_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,20 @@ impl<'a> DiagnosticBuilder<'a> {
self
}

/// See [`Diagnostic::multipart_suggestion()`].
pub fn multipart_suggestion_verbose(
&mut self,
msg: &str,
suggestion: Vec<(Span, String)>,
applicability: Applicability,
) -> &mut Self {
if !self.0.allow_suggestions {
return self;
}
self.0.diagnostic.multipart_suggestion_verbose(msg, suggestion, applicability);
self
}

/// See [`Diagnostic::tool_only_multipart_suggestion()`].
pub fn tool_only_multipart_suggestion(
&mut self,
Expand Down
85 changes: 46 additions & 39 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1633,50 +1633,57 @@ impl<'a> Parser<'a> {
{
let rfc_note = "anonymous parameters are removed in the 2018 edition (see RFC 1685)";

let (ident, self_sugg, param_sugg, type_sugg) = match pat.kind {
PatKind::Ident(_, ident, _) => (
ident,
format!("self: {}", ident),
format!("{}: TypeName", ident),
format!("_: {}", ident),
),
// Also catches `fn foo(&a)`.
PatKind::Ref(ref pat, mutab)
if matches!(pat.clone().into_inner().kind, PatKind::Ident(..)) =>
{
match pat.clone().into_inner().kind {
PatKind::Ident(_, ident, _) => {
let mutab = mutab.prefix_str();
(
ident,
format!("self: &{}{}", mutab, ident),
format!("{}: &{}TypeName", ident, mutab),
format!("_: &{}{}", mutab, ident),
)
let (ident, self_sugg, param_sugg, type_sugg, self_span, param_span, type_span) =
match pat.kind {
PatKind::Ident(_, ident, _) => (
ident,
"self: ".to_string(),
": TypeName".to_string(),
"_: ".to_string(),
pat.span.shrink_to_lo(),
pat.span.shrink_to_hi(),
pat.span.shrink_to_lo(),
),
// Also catches `fn foo(&a)`.
PatKind::Ref(ref inner_pat, mutab)
if matches!(inner_pat.clone().into_inner().kind, PatKind::Ident(..)) =>
{
match inner_pat.clone().into_inner().kind {
PatKind::Ident(_, ident, _) => {
let mutab = mutab.prefix_str();
(
ident,
"self: ".to_string(),
format!("{}: &{}TypeName", ident, mutab),
"_: ".to_string(),
pat.span.shrink_to_lo(),
pat.span,
pat.span.shrink_to_lo(),
)
}
_ => unreachable!(),
}
_ => unreachable!(),
}
}
_ => {
// Otherwise, try to get a type and emit a suggestion.
if let Some(ty) = pat.to_ty() {
err.span_suggestion_verbose(
pat.span,
"explicitly ignore the parameter name",
format!("_: {}", pprust::ty_to_string(&ty)),
Applicability::MachineApplicable,
);
err.note(rfc_note);
}
_ => {
// Otherwise, try to get a type and emit a suggestion.
if let Some(ty) = pat.to_ty() {
err.span_suggestion_verbose(
pat.span,
"explicitly ignore the parameter name",
format!("_: {}", pprust::ty_to_string(&ty)),
Applicability::MachineApplicable,
);
err.note(rfc_note);
}

return None;
}
};
return None;
}
};

// `fn foo(a, b) {}`, `fn foo(a<x>, b<y>) {}` or `fn foo(usize, usize) {}`
if first_param {
err.span_suggestion(
pat.span,
self_span,
"if this is a `self` type, give it a parameter name",
self_sugg,
Applicability::MaybeIncorrect,
Expand All @@ -1686,14 +1693,14 @@ impl<'a> Parser<'a> {
// `fn foo(HashMap: TypeName<u32>)`.
if self.token != token::Lt {
err.span_suggestion(
pat.span,
param_span,
"if this is a parameter name, give it a type",
param_sugg,
Applicability::HasPlaceholders,
);
}
err.span_suggestion(
pat.span,
type_span,
"if this is a type, explicitly ignore the parameter name",
type_sugg,
Applicability::MachineApplicable,
Expand Down
17 changes: 6 additions & 11 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_ast::{
};
use rustc_ast_pretty::pprust::path_segment_to_string;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder, SuggestionStyle};
use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def::Namespace::{self, *};
use rustc_hir::def::{self, CtorKind, CtorOf, DefKind};
Expand Down Expand Up @@ -1960,11 +1960,10 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
introduce_suggestion.push((*span, formatter(&lt_name)));
}
}
err.multipart_suggestion_with_style(
err.multipart_suggestion_verbose(
&msg,
introduce_suggestion,
Applicability::MaybeIncorrect,
SuggestionStyle::ShowAlways,
);
}

Expand All @@ -1976,14 +1975,13 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
})
.map(|(formatter, span)| (*span, formatter(name)))
.collect();
err.multipart_suggestion_with_style(
err.multipart_suggestion_verbose(
&format!(
"consider using the `{}` lifetime",
lifetime_names.iter().next().unwrap()
),
spans_suggs,
Applicability::MaybeIncorrect,
SuggestionStyle::ShowAlways,
);
};
let suggest_new = |err: &mut DiagnosticBuilder<'_>, suggs: Vec<Option<String>>| {
Expand Down Expand Up @@ -2074,11 +2072,10 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
};
spans_suggs.push((span, sugg.to_string()));
}
err.multipart_suggestion_with_style(
err.multipart_suggestion_verbose(
"consider using the `'static` lifetime",
spans_suggs,
Applicability::MaybeIncorrect,
SuggestionStyle::ShowAlways,
);
continue;
}
Expand Down Expand Up @@ -2163,11 +2160,10 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
.unwrap_or((span, sugg));
introduce_suggestion.push((span, sugg.to_string()));
}
err.multipart_suggestion_with_style(
err.multipart_suggestion_verbose(
&msg,
introduce_suggestion,
Applicability::MaybeIncorrect,
SuggestionStyle::ShowAlways,
);
if should_break {
break;
Expand Down Expand Up @@ -2243,11 +2239,10 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
if spans_suggs.len() > 0 {
// This happens when we have `Foo<T>` where we point at the space before `T`,
// but this can be confusing so we give a suggestion with placeholders.
err.multipart_suggestion_with_style(
err.multipart_suggestion_verbose(
"consider using one of the available lifetimes here",
spans_suggs,
Applicability::HasPlaceholders,
SuggestionStyle::ShowAlways,
);
}
}
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_typeck/src/astconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1687,14 +1687,13 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
constraint=constraint,
));
} else {
err.span_suggestion(
span,
err.span_suggestion_verbose(
span.with_hi(assoc_name.span.lo()),
"use fully qualified syntax to disambiguate",
format!(
"<{} as {}>::{}",
"<{} as {}>::",
ty_param_name(),
bound.print_only_trait_path(),
assoc_name,
),
Applicability::MaybeIncorrect,
);
Expand Down
Loading

0 comments on commit 9bb77da

Please sign in to comment.