Skip to content

Commit

Permalink
Rollup merge of rust-lang#87160 - estebank:colon-recovery, r=nagisa
Browse files Browse the repository at this point in the history
When recovering from a `:` in a pattern, use adequate AST pattern

If the suggestion to use `::` instead of `:` in the pattern isn't correct, a second resolution error will be emitted.
  • Loading branch information
matthiaskrgr authored Dec 1, 2021
2 parents 2446a21 + c027105 commit e68e5d2
Show file tree
Hide file tree
Showing 4 changed files with 252 additions and 134 deletions.
182 changes: 180 additions & 2 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use super::pat::Expected;
use super::ty::AllowPlus;
use super::TokenType;
use super::{BlockMode, Parser, PathStyle, Restrictions, SemiColonMode, SeqSep, TokenExpectType};
use super::{
BlockMode, Parser, PathStyle, RecoverColon, RecoverComma, Restrictions, SemiColonMode, SeqSep,
TokenExpectType, TokenType,
};

use rustc_ast as ast;
use rustc_ast::ptr::P;
Expand All @@ -19,6 +22,8 @@ use rustc_span::source_map::Spanned;
use rustc_span::symbol::{kw, Ident};
use rustc_span::{MultiSpan, Span, SpanSnippetError, DUMMY_SP};

use std::mem::take;

use tracing::{debug, trace};

const TURBOFISH_SUGGESTION_STR: &str =
Expand Down Expand Up @@ -2075,4 +2080,177 @@ impl<'a> Parser<'a> {
);
err
}

/// 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_colon_colon_in_pat_typo(
&mut self,
mut first_pat: P<Pat>,
ra: RecoverColon,
expected: Expected,
) -> P<Pat> {
if RecoverColon::Yes != ra || token::Colon != self.token.kind {
return first_pat;
}
if !matches!(first_pat.kind, PatKind::Ident(_, _, None) | PatKind::Path(..))
|| !self.look_ahead(1, |token| token.is_ident() && !token.is_reserved_ident())
{
return first_pat;
}
// The pattern looks like it might be a path with a `::` -> `:` typo:
// `match foo { bar:baz => {} }`
let span = self.token.span;
// We only emit "unexpected `:`" error here if we can successfully parse the
// whole pattern correctly in that case.
let snapshot = self.clone();

// Create error for "unexpected `:`".
match self.expected_one_of_not_found(&[], &[]) {
Err(mut err) => {
self.bump(); // Skip the `:`.
match self.parse_pat_no_top_alt(expected) {
Err(mut inner_err) => {
// Carry on as if we had not done anything, callers will emit a
// reasonable error.
inner_err.cancel();
err.cancel();
*self = snapshot;
}
Ok(mut pat) => {
// We've parsed the rest of the pattern.
let new_span = first_pat.span.to(pat.span);
let mut show_sugg = false;
// Try to construct a recovered pattern.
match &mut pat.kind {
PatKind::Struct(qself @ None, path, ..)
| PatKind::TupleStruct(qself @ None, path, _)
| PatKind::Path(qself @ None, path) => match &first_pat.kind {
PatKind::Ident(_, ident, _) => {
path.segments.insert(0, PathSegment::from_ident(ident.clone()));
path.span = new_span;
show_sugg = true;
first_pat = pat;
}
PatKind::Path(old_qself, old_path) => {
path.segments = old_path
.segments
.iter()
.cloned()
.chain(take(&mut path.segments))
.collect();
path.span = new_span;
*qself = old_qself.clone();
first_pat = pat;
show_sugg = true;
}
_ => {}
},
PatKind::Ident(BindingMode::ByValue(Mutability::Not), ident, None) => {
match &first_pat.kind {
PatKind::Ident(_, old_ident, _) => {
let path = PatKind::Path(
None,
Path {
span: new_span,
segments: vec![
PathSegment::from_ident(old_ident.clone()),
PathSegment::from_ident(ident.clone()),
],
tokens: None,
},
);
first_pat = self.mk_pat(new_span, path);
show_sugg = true;
}
PatKind::Path(old_qself, old_path) => {
let mut segments = old_path.segments.clone();
segments.push(PathSegment::from_ident(ident.clone()));
let path = PatKind::Path(
old_qself.clone(),
Path { span: new_span, segments, tokens: None },
);
first_pat = self.mk_pat(new_span, path);
show_sugg = true;
}
_ => {}
}
}
_ => {}
}
if show_sugg {
err.span_suggestion(
span,
"maybe write a path separator here",
"::".to_string(),
Applicability::MaybeIncorrect,
);
} else {
first_pat = self.mk_pat(new_span, PatKind::Wild);
}
err.emit();
}
}
}
_ => {
// Carry on as if we had not done anything. This should be unreachable.
*self = snapshot;
}
};
first_pat
}

/// 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,
) -> PResult<'a, ()> {
if rc == RecoverComma::No || self.token != token::Comma {
return Ok(());
}

// An unexpected comma after a top-level pattern is a clue that the
// user (perhaps more accustomed to some other language) forgot the
// parentheses in what should have been a tuple pattern; return a
// suggestion-enhanced error here rather than choking on the comma later.
let comma_span = self.token.span;
self.bump();
if let Err(mut err) = self.skip_pat_list() {
// We didn't expect this to work anyway; we just wanted to advance to the
// end of the comma-sequence so we know the span to suggest parenthesizing.
err.cancel();
}
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(",", " |"),
Applicability::MachineApplicable,
);
}
Err(err)
}

/// Parse and throw away a parenthesized comma separated
/// sequence of patterns until `)` is reached.
fn skip_pat_list(&mut self) -> PResult<'a, ()> {
while !self.check(&token::CloseDelim(token::Paren)) {
self.parse_pat_no_top_alt(None)?;
if !self.eat(&token::Comma) {
return Ok(());
}
}
Ok(())
}
}
113 changes: 9 additions & 104 deletions compiler/rustc_parse/src/parser/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@ use crate::{maybe_recover_from_interpolated_ty_qpath, maybe_whole};
use rustc_ast::mut_visit::{noop_visit_pat, MutVisitor};
use rustc_ast::ptr::P;
use rustc_ast::token;
use rustc_ast::{self as ast, AttrVec, Attribute, MacCall, Pat, PatField, PatKind, RangeEnd};
use rustc_ast::{BindingMode, Expr, ExprKind, Mutability, Path, QSelf, RangeSyntax};
use rustc_ast::{
self as ast, AttrVec, Attribute, BindingMode, Expr, ExprKind, MacCall, Mutability, Pat,
PatField, PatKind, Path, QSelf, RangeEnd, RangeSyntax,
};
use rustc_ast_pretty::pprust;
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder, PResult};
use rustc_span::source_map::{respan, Span, Spanned};
use rustc_span::symbol::{kw, sym, Ident};

type Expected = Option<&'static str>;
pub(super) type Expected = Option<&'static str>;

/// `Expected` for function and lambda parameter patterns.
pub(super) const PARAM_EXPECTED: Expected = Some("parameter name");
Expand Down Expand Up @@ -98,55 +100,9 @@ impl<'a> Parser<'a> {
// If we parsed a leading `|` which should be gated,
// then we should really gate the leading `|`.
// This complicated procedure is done purely for diagnostics UX.
let mut first_pat = first_pat;

if let (RecoverColon::Yes, token::Colon) = (ra, &self.token.kind) {
if matches!(
first_pat.kind,
PatKind::Ident(BindingMode::ByValue(Mutability::Not), _, None)
| PatKind::Path(..)
) && self.look_ahead(1, |token| token.is_ident() && !token.is_reserved_ident())
{
// The pattern looks like it might be a path with a `::` -> `:` typo:
// `match foo { bar:baz => {} }`
let span = self.token.span;
// We only emit "unexpected `:`" error here if we can successfully parse the
// whole pattern correctly in that case.
let snapshot = self.clone();

// Create error for "unexpected `:`".
match self.expected_one_of_not_found(&[], &[]) {
Err(mut err) => {
self.bump(); // Skip the `:`.
match self.parse_pat_no_top_alt(expected) {
Err(mut inner_err) => {
// Carry on as if we had not done anything, callers will emit a
// reasonable error.
inner_err.cancel();
err.cancel();
*self = snapshot;
}
Ok(pat) => {
// We've parsed the rest of the pattern.
err.span_suggestion(
span,
"maybe write a path separator here",
"::".to_string(),
Applicability::MachineApplicable,
);
err.emit();
first_pat =
self.mk_pat(first_pat.span.to(pat.span), PatKind::Wild);
}
}
}
_ => {
// Carry on as if we had not done anything. This should be unreachable.
*self = snapshot;
}
};
}
}

// Check if the user wrote `foo:bar` instead of `foo::bar`.
let first_pat = self.maybe_recover_colon_colon_in_pat_typo(first_pat, ra, expected);

if let Some(leading_vert_span) = leading_vert_span {
// If there was a leading vert, treat this as an or-pattern. This improves
Expand Down Expand Up @@ -321,57 +277,6 @@ impl<'a> Parser<'a> {
err.emit();
}

/// Some special error handling for the "top-level" patterns in a match arm,
/// `for` loop, `let`, &c. (in contrast to subpatterns within such).
fn maybe_recover_unexpected_comma(&mut self, lo: Span, rc: RecoverComma) -> PResult<'a, ()> {
if rc == RecoverComma::No || self.token != token::Comma {
return Ok(());
}

// An unexpected comma after a top-level pattern is a clue that the
// user (perhaps more accustomed to some other language) forgot the
// parentheses in what should have been a tuple pattern; return a
// suggestion-enhanced error here rather than choking on the comma later.
let comma_span = self.token.span;
self.bump();
if let Err(mut err) = self.skip_pat_list() {
// We didn't expect this to work anyway; we just wanted to advance to the
// end of the comma-sequence so we know the span to suggest parenthesizing.
err.cancel();
}
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(",", " |"),
Applicability::MachineApplicable,
);
}
Err(err)
}

/// Parse and throw away a parenthesized comma separated
/// sequence of patterns until `)` is reached.
fn skip_pat_list(&mut self) -> PResult<'a, ()> {
while !self.check(&token::CloseDelim(token::Paren)) {
self.parse_pat_no_top_alt(None)?;
if !self.eat(&token::Comma) {
return Ok(());
}
}
Ok(())
}

/// A `|` or possibly `||` token shouldn't be here. Ban it.
fn ban_illegal_vert(&mut self, lo: Option<Span>, pos: &str, ctx: &str) {
let span = self.token.span;
Expand Down Expand Up @@ -1168,7 +1073,7 @@ impl<'a> Parser<'a> {
self.mk_pat(span, PatKind::Ident(bm, ident, None))
}

fn mk_pat(&self, span: Span, kind: PatKind) -> P<Pat> {
pub(super) fn mk_pat(&self, span: Span, kind: PatKind) -> P<Pat> {
P(Pat { kind, span, id: ast::DUMMY_NODE_ID, tokens: None })
}
}
Loading

0 comments on commit e68e5d2

Please sign in to comment.