Skip to content

Commit

Permalink
Tweak diagnostics
Browse files Browse the repository at this point in the history
* Recover from invalid `'label: ` before block.
* Make suggestion to enclose statements in a block multipart.
* Point at `match`, `while`, `loop` and `unsafe` keywords when failing
  to parse their expression.
* Do not suggest `{ ; }`.
* Do not suggest `|` when very unlikely to be what was wanted (in `let`
  statements).
  • Loading branch information
estebank committed Feb 28, 2022
1 parent 97cde9f commit f42b4f5
Show file tree
Hide file tree
Showing 26 changed files with 316 additions and 161 deletions.
3 changes: 2 additions & 1 deletion compiler/rustc_expand/src/expand.rs
Expand Up @@ -20,7 +20,7 @@ use rustc_data_structures::sync::Lrc;
use rustc_errors::{Applicability, PResult};
use rustc_feature::Features;
use rustc_parse::parser::{
AttemptLocalParseRecovery, ForceCollect, Parser, RecoverColon, RecoverComma,
AttemptLocalParseRecovery, CommaRecoveryMode, ForceCollect, Parser, RecoverColon, RecoverComma,
};
use rustc_parse::validate_attr;
use rustc_session::lint::builtin::{UNUSED_ATTRIBUTES, UNUSED_DOC_COMMENTS};
Expand Down Expand Up @@ -911,6 +911,7 @@ pub fn parse_ast_fragment<'a>(
None,
RecoverComma::No,
RecoverColon::Yes,
CommaRecoveryMode::LikelyTuple,
)?),
AstFragmentKind::Crate => AstFragment::Crate(this.parse_crate_mod()?),
AstFragmentKind::Arms
Expand Down
53 changes: 39 additions & 14 deletions compiler/rustc_parse/src/parser/diagnostics.rs
@@ -1,8 +1,8 @@
use super::pat::Expected;
use super::ty::{AllowPlus, IsAsCast};
use super::{
BlockMode, Parser, PathStyle, RecoverColon, RecoverComma, Restrictions, SemiColonMode, SeqSep,
TokenExpectType, TokenType,
BlockMode, CommaRecoveryMode, Parser, PathStyle, RecoverColon, RecoverComma, Restrictions,
SemiColonMode, SeqSep, TokenExpectType, TokenType,
};

use rustc_ast as ast;
Expand Down Expand Up @@ -2245,12 +2245,32 @@ impl<'a> Parser<'a> {
first_pat
}

crate fn maybe_recover_unexpected_block_label(&mut self) -> bool {
let Some(label) = self.eat_label().filter(|_| {
self.eat(&token::Colon) && self.token.kind == token::OpenDelim(token::Brace)
}) else {
return false;
};
let span = label.ident.span.to(self.prev_token.span);
let mut err = self.struct_span_err(span, "block label not supported here");
err.span_label(span, "not supported here");
err.tool_only_span_suggestion(
label.ident.span.until(self.token.span),
"remove this block label",
String::new(),
Applicability::MachineApplicable,
);
err.emit();
true
}

/// Some special error handling for the "top-level" patterns in a match arm,
/// `for` loop, `let`, &c. (in contrast to subpatterns within such).
crate fn maybe_recover_unexpected_comma(
&mut self,
lo: Span,
rc: RecoverComma,
rt: CommaRecoveryMode,
) -> PResult<'a, ()> {
if rc == RecoverComma::No || self.token != token::Comma {
return Ok(());
Expand All @@ -2270,20 +2290,25 @@ impl<'a> Parser<'a> {
let seq_span = lo.to(self.prev_token.span);
let mut err = self.struct_span_err(comma_span, "unexpected `,` in pattern");
if let Ok(seq_snippet) = self.span_to_snippet(seq_span) {
const MSG: &str = "try adding parentheses to match on a tuple...";

err.span_suggestion(
seq_span,
MSG,
format!("({})", seq_snippet),
Applicability::MachineApplicable,
);
err.span_suggestion(
seq_span,
"...or a vertical bar to match on multiple alternatives",
seq_snippet.replace(',', " |"),
err.multipart_suggestion(
&format!(
"try adding parentheses to match on a tuple{}",
if let CommaRecoveryMode::LikelyTuple = rt { "" } else { "..." },
),
vec![
(seq_span.shrink_to_lo(), "(".to_string()),
(seq_span.shrink_to_hi(), ")".to_string()),
],
Applicability::MachineApplicable,
);
if let CommaRecoveryMode::EitherTupleOrPipe = rt {
err.span_suggestion(
seq_span,
"...or a vertical bar to match on multiple alternatives",
seq_snippet.replace(',', " |"),
Applicability::MachineApplicable,
);
}
}
Err(err)
}
Expand Down
56 changes: 46 additions & 10 deletions compiler/rustc_parse/src/parser/expr.rs
@@ -1,4 +1,4 @@
use super::pat::{RecoverColon, RecoverComma, PARAM_EXPECTED};
use super::pat::{CommaRecoveryMode, RecoverColon, RecoverComma, PARAM_EXPECTED};
use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign};
use super::{
AttrWrapper, BlockMode, ClosureSpans, ForceCollect, Parser, PathStyle, Restrictions, TokenType,
Expand Down Expand Up @@ -1286,18 +1286,27 @@ impl<'a> Parser<'a> {
} else if let Some(label) = self.eat_label() {
self.parse_labeled_expr(label, attrs, true)
} else if self.eat_keyword(kw::Loop) {
self.parse_loop_expr(None, self.prev_token.span, attrs)
let sp = self.prev_token.span;
self.parse_loop_expr(None, self.prev_token.span, attrs).map_err(|mut err| {
err.span_label(sp, "while parsing this `loop` expression");
err
})
} else if self.eat_keyword(kw::Continue) {
let kind = ExprKind::Continue(self.eat_label());
Ok(self.mk_expr(lo.to(self.prev_token.span), kind, attrs))
} else if self.eat_keyword(kw::Match) {
let match_sp = self.prev_token.span;
self.parse_match_expr(attrs).map_err(|mut err| {
err.span_label(match_sp, "while parsing this match expression");
err.span_label(match_sp, "while parsing this `match` expression");
err
})
} else if self.eat_keyword(kw::Unsafe) {
let sp = self.prev_token.span;
self.parse_block_expr(None, lo, BlockCheckMode::Unsafe(ast::UserProvided), attrs)
.map_err(|mut err| {
err.span_label(sp, "while parsing this `unsafe` expression");
err
})
} else if self.check_inline_const(0) {
self.parse_const_block(lo.to(self.token.span), false)
} else if self.is_do_catch_block() {
Expand Down Expand Up @@ -2160,7 +2169,12 @@ impl<'a> Parser<'a> {
/// The `let` token has already been eaten.
fn parse_let_expr(&mut self, attrs: AttrVec) -> PResult<'a, P<Expr>> {
let lo = self.prev_token.span;
let pat = self.parse_pat_allow_top_alt(None, RecoverComma::Yes, RecoverColon::Yes)?;
let pat = self.parse_pat_allow_top_alt(
None,
RecoverComma::Yes,
RecoverColon::Yes,
CommaRecoveryMode::LikelyTuple,
)?;
self.expect(&token::Eq)?;
let expr = self.with_res(self.restrictions | Restrictions::NO_STRUCT_LITERAL, |this| {
this.parse_assoc_expr_with(1 + prec_let_scrutinee_needs_par(), None.into())
Expand Down Expand Up @@ -2223,7 +2237,12 @@ impl<'a> Parser<'a> {
_ => None,
};

let pat = self.parse_pat_allow_top_alt(None, RecoverComma::Yes, RecoverColon::Yes)?;
let pat = self.parse_pat_allow_top_alt(
None,
RecoverComma::Yes,
RecoverColon::Yes,
CommaRecoveryMode::LikelyTuple,
)?;
if !self.eat_keyword(kw::In) {
self.error_missing_in_for_loop();
}
Expand Down Expand Up @@ -2266,8 +2285,15 @@ impl<'a> Parser<'a> {
lo: Span,
mut attrs: AttrVec,
) -> PResult<'a, P<Expr>> {
let cond = self.parse_cond_expr()?;
let (iattrs, body) = self.parse_inner_attrs_and_block()?;
let cond = self.parse_cond_expr().map_err(|mut err| {
err.span_label(lo, "while parsing the condition of this `while` expression");
err
})?;
let (iattrs, body) = self.parse_inner_attrs_and_block().map_err(|mut err| {
err.span_label(lo, "while parsing the body of this `while` expression");
err.span_label(cond.span, "this `while` condition successfully parsed");
err
})?;
attrs.extend(iattrs);
Ok(self.mk_expr(lo.to(self.prev_token.span), ExprKind::While(cond, body, opt_label), attrs))
}
Expand All @@ -2284,7 +2310,7 @@ impl<'a> Parser<'a> {
Ok(self.mk_expr(lo.to(self.prev_token.span), ExprKind::Loop(body, opt_label), attrs))
}

fn eat_label(&mut self) -> Option<Label> {
crate fn eat_label(&mut self) -> Option<Label> {
self.token.lifetime().map(|ident| {
self.bump();
Label { ident }
Expand All @@ -2305,7 +2331,12 @@ impl<'a> Parser<'a> {
Applicability::MaybeIncorrect, // speculative
);
}
return Err(e);
if self.maybe_recover_unexpected_block_label() {
e.cancel();
self.bump();
} else {
return Err(e);
}
}
attrs.extend(self.parse_inner_attributes()?);

Expand Down Expand Up @@ -2441,7 +2472,12 @@ impl<'a> Parser<'a> {
let attrs = self.parse_outer_attributes()?;
self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| {
let lo = this.token.span;
let pat = this.parse_pat_allow_top_alt(None, RecoverComma::Yes, RecoverColon::Yes)?;
let pat = this.parse_pat_allow_top_alt(
None,
RecoverComma::Yes,
RecoverColon::Yes,
CommaRecoveryMode::EitherTupleOrPipe,
)?;
let guard = if this.eat_keyword(kw::If) {
let if_span = this.prev_token.span;
let cond = this.parse_expr()?;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/mod.rs
Expand Up @@ -15,7 +15,7 @@ pub use attr_wrapper::AttrWrapper;
pub use diagnostics::AttemptLocalParseRecovery;
use diagnostics::Error;
pub(crate) use item::FnParseMode;
pub use pat::{RecoverColon, RecoverComma};
pub use pat::{CommaRecoveryMode, RecoverColon, RecoverComma};
pub use path::PathStyle;

use rustc_ast::ptr::P;
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_parse/src/parser/nonterminal.rs
Expand Up @@ -5,7 +5,7 @@ use rustc_ast_pretty::pprust;
use rustc_errors::PResult;
use rustc_span::symbol::{kw, Ident};

use crate::parser::pat::{RecoverColon, RecoverComma};
use crate::parser::pat::{CommaRecoveryMode, RecoverColon, RecoverComma};
use crate::parser::{FollowedByType, ForceCollect, Parser, PathStyle};

impl<'a> Parser<'a> {
Expand Down Expand Up @@ -125,7 +125,7 @@ impl<'a> Parser<'a> {
token::NtPat(self.collect_tokens_no_attrs(|this| match kind {
NonterminalKind::PatParam { .. } => this.parse_pat_no_top_alt(None),
NonterminalKind::PatWithOr { .. } => {
this.parse_pat_allow_top_alt(None, RecoverComma::No, RecoverColon::No)
this.parse_pat_allow_top_alt(None, RecoverComma::No, RecoverColon::No, CommaRecoveryMode::EitherTupleOrPipe)
}
_ => unreachable!(),
})?)
Expand Down
51 changes: 42 additions & 9 deletions compiler/rustc_parse/src/parser/pat.rs
Expand Up @@ -33,6 +33,13 @@ pub enum RecoverColon {
No,
}

/// Whether or not to recover a `a, b` when parsing patterns as `(a, b)` or that *and* `a | b`.
#[derive(PartialEq, Copy, Clone)]
pub enum CommaRecoveryMode {
LikelyTuple,
EitherTupleOrPipe,
}

/// The result of `eat_or_separator`. We want to distinguish which case we are in to avoid
/// emitting duplicate diagnostics.
#[derive(Debug, Clone, Copy)]
Expand Down Expand Up @@ -68,8 +75,9 @@ impl<'a> Parser<'a> {
expected: Expected,
rc: RecoverComma,
ra: RecoverColon,
rt: CommaRecoveryMode,
) -> PResult<'a, P<Pat>> {
self.parse_pat_allow_top_alt_inner(expected, rc, ra).map(|(pat, _)| pat)
self.parse_pat_allow_top_alt_inner(expected, rc, ra, rt).map(|(pat, _)| pat)
}

/// Returns the pattern and a bool indicating whether we recovered from a trailing vert (true =
Expand All @@ -79,6 +87,7 @@ impl<'a> Parser<'a> {
expected: Expected,
rc: RecoverComma,
ra: RecoverColon,
rt: CommaRecoveryMode,
) -> PResult<'a, (P<Pat>, bool)> {
// Keep track of whether we recovered from a trailing vert so that we can avoid duplicated
// suggestions (which bothers rustfix).
Expand All @@ -92,7 +101,7 @@ impl<'a> Parser<'a> {

// Parse the first pattern (`p_0`).
let first_pat = self.parse_pat_no_top_alt(expected)?;
self.maybe_recover_unexpected_comma(first_pat.span, rc)?;
self.maybe_recover_unexpected_comma(first_pat.span, rc, rt)?;

// If the next token is not a `|`,
// this is not an or-pattern and we should exit here.
Expand Down Expand Up @@ -130,7 +139,7 @@ impl<'a> Parser<'a> {
err.span_label(lo, WHILE_PARSING_OR_MSG);
err
})?;
self.maybe_recover_unexpected_comma(pat.span, rc)?;
self.maybe_recover_unexpected_comma(pat.span, rc, rt)?;
pats.push(pat);
}
let or_pattern_span = lo.to(self.prev_token.span);
Expand All @@ -155,8 +164,12 @@ impl<'a> Parser<'a> {
// We use `parse_pat_allow_top_alt` regardless of whether we actually want top-level
// or-patterns so that we can detect when a user tries to use it. This allows us to print a
// better error message.
let (pat, trailing_vert) =
self.parse_pat_allow_top_alt_inner(expected, rc, RecoverColon::No)?;
let (pat, trailing_vert) = self.parse_pat_allow_top_alt_inner(
expected,
rc,
RecoverColon::No,
CommaRecoveryMode::LikelyTuple,
)?;
let colon = self.eat(&token::Colon);

if let PatKind::Or(pats) = &pat.kind {
Expand Down Expand Up @@ -315,7 +328,12 @@ impl<'a> Parser<'a> {
} else if self.check(&token::OpenDelim(token::Bracket)) {
// Parse `[pat, pat,...]` as a slice pattern.
let (pats, _) = self.parse_delim_comma_seq(token::Bracket, |p| {
p.parse_pat_allow_top_alt(None, RecoverComma::No, RecoverColon::No)
p.parse_pat_allow_top_alt(
None,
RecoverComma::No,
RecoverColon::No,
CommaRecoveryMode::EitherTupleOrPipe,
)
})?;
PatKind::Slice(pats)
} else if self.check(&token::DotDot) && !self.is_pat_range_end_start(1) {
Expand Down Expand Up @@ -529,7 +547,12 @@ impl<'a> Parser<'a> {
/// Parse a tuple or parenthesis pattern.
fn parse_pat_tuple_or_parens(&mut self) -> PResult<'a, PatKind> {
let (fields, trailing_comma) = self.parse_paren_comma_seq(|p| {
p.parse_pat_allow_top_alt(None, RecoverComma::No, RecoverColon::No)
p.parse_pat_allow_top_alt(
None,
RecoverComma::No,
RecoverColon::No,
CommaRecoveryMode::LikelyTuple,
)
})?;

// Here, `(pat,)` is a tuple pattern.
Expand Down Expand Up @@ -873,7 +896,12 @@ impl<'a> Parser<'a> {
/// Parse tuple struct or tuple variant pattern (e.g. `Foo(...)` or `Foo::Bar(...)`).
fn parse_pat_tuple_struct(&mut self, qself: Option<QSelf>, path: Path) -> PResult<'a, PatKind> {
let (fields, _) = self.parse_paren_comma_seq(|p| {
p.parse_pat_allow_top_alt(None, RecoverComma::No, RecoverColon::No)
p.parse_pat_allow_top_alt(
None,
RecoverComma::No,
RecoverColon::No,
CommaRecoveryMode::EitherTupleOrPipe,
)
})?;
if qself.is_some() {
self.sess.gated_spans.gate(sym::more_qualified_paths, path.span);
Expand Down Expand Up @@ -1033,7 +1061,12 @@ impl<'a> Parser<'a> {
// Parsing a pattern of the form `fieldname: pat`.
let fieldname = self.parse_field_name()?;
self.bump();
let pat = self.parse_pat_allow_top_alt(None, RecoverComma::No, RecoverColon::No)?;
let pat = self.parse_pat_allow_top_alt(
None,
RecoverComma::No,
RecoverColon::No,
CommaRecoveryMode::EitherTupleOrPipe,
)?;
hi = pat.span;
(pat, fieldname, false)
} else {
Expand Down

0 comments on commit f42b4f5

Please sign in to comment.