Skip to content

Commit

Permalink
Account for (pat if expr) => {}
Browse files Browse the repository at this point in the history
When encountering match arm (pat if expr) => {}, recover and suggest removing parentheses. Fix rust-lang#100825.
  • Loading branch information
estebank committed Nov 29, 2023
1 parent 5598266 commit a0cfc53
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 47 deletions.
3 changes: 3 additions & 0 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,9 @@ parse_unexpected_lifetime_in_pattern = unexpected lifetime `{$symbol}` in patter
parse_unexpected_parentheses_in_for_head = unexpected parentheses surrounding `for` loop head
.suggestion = remove parentheses in `for` loop
parse_unexpected_parentheses_in_match_arm_pattern = unexpected parentheses surrounding `match` arm pattern
.suggestion = remove parentheses surrounding the pattern
parse_unexpected_self_in_generic_parameters = unexpected keyword `Self` in generic parameters
.note = you cannot use `Self` as a generic parameter because it is reserved for associated items
Expand Down
18 changes: 18 additions & 0 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1281,6 +1281,24 @@ pub(crate) struct ParenthesesInForHeadSugg {
pub right: Span,
}

#[derive(Diagnostic)]
#[diag(parse_unexpected_parentheses_in_match_arm_pattern)]
pub(crate) struct ParenthesesInMatchPat {
#[primary_span]
pub span: Vec<Span>,
#[subdiagnostic]
pub sugg: ParenthesesInMatchPatSugg,
}

#[derive(Subdiagnostic)]
#[multipart_suggestion(parse_suggestion, applicability = "machine-applicable")]
pub(crate) struct ParenthesesInMatchPatSugg {
#[suggestion_part(code = "")]
pub left: Span,
#[suggestion_part(code = "")]
pub right: Span,
}

#[derive(Diagnostic)]
#[diag(parse_doc_comment_on_param_type)]
pub(crate) struct DocCommentOnParamType {
Expand Down
125 changes: 86 additions & 39 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use super::{
use crate::errors;
use crate::maybe_recover_from_interpolated_ty_qpath;
use ast::mut_visit::{noop_visit_expr, MutVisitor};
use ast::{GenBlockKind, Path, PathSegment};
use ast::{GenBlockKind, Pat, Path, PathSegment};
use core::mem;
use rustc_ast::ptr::P;
use rustc_ast::token::{self, Delimiter, Token, TokenKind};
Expand Down Expand Up @@ -2856,47 +2856,10 @@ impl<'a> Parser<'a> {
}

pub(super) fn parse_arm(&mut self) -> PResult<'a, Arm> {
// Used to check the `let_chains` and `if_let_guard` features mostly by scanning
// `&&` tokens.
fn check_let_expr(expr: &Expr) -> (bool, bool) {
match &expr.kind {
ExprKind::Binary(BinOp { node: BinOpKind::And, .. }, lhs, rhs) => {
let lhs_rslt = check_let_expr(lhs);
let rhs_rslt = check_let_expr(rhs);
(lhs_rslt.0 || rhs_rslt.0, false)
}
ExprKind::Let(..) => (true, true),
_ => (false, true),
}
}
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,
CommaRecoveryMode::EitherTupleOrPipe,
)?;
let guard = if this.eat_keyword(kw::If) {
let if_span = this.prev_token.span;
let mut cond = this.parse_match_guard_condition()?;

CondChecker::new(this).visit_expr(&mut cond);

let (has_let_expr, does_not_have_bin_op) = check_let_expr(&cond);
if has_let_expr {
if does_not_have_bin_op {
// Remove the last feature gating of a `let` expression since it's stable.
this.sess.gated_spans.ungate_last(sym::let_chains, cond.span);
}
let span = if_span.to(cond.span);
this.sess.gated_spans.gate(sym::if_let_guard, span);
}
Some(cond)
} else {
None
};
let (pat, guard) = this.parse_match_arm_pat_and_guard()?;
let arrow_span = this.token.span;
if let Err(mut err) = this.expect(&token::FatArrow) {
// We might have a `=>` -> `=` or `->` typo (issue #89396).
Expand Down Expand Up @@ -3026,6 +2989,90 @@ impl<'a> Parser<'a> {
})
}

fn parse_match_arm_guard(&mut self) -> PResult<'a, Option<P<Expr>>> {
// Used to check the `let_chains` and `if_let_guard` features mostly by scanning
// `&&` tokens.
fn check_let_expr(expr: &Expr) -> (bool, bool) {
match &expr.kind {
ExprKind::Binary(BinOp { node: BinOpKind::And, .. }, lhs, rhs) => {
let lhs_rslt = check_let_expr(lhs);
let rhs_rslt = check_let_expr(rhs);
(lhs_rslt.0 || rhs_rslt.0, false)
}
ExprKind::Let(..) => (true, true),
_ => (false, true),
}
}
if !self.eat_keyword(kw::If) {
// No match arm guard present.
return Ok(None);
}

let if_span = self.prev_token.span;
let mut cond = self.parse_match_guard_condition()?;

CondChecker::new(self).visit_expr(&mut cond);

let (has_let_expr, does_not_have_bin_op) = check_let_expr(&cond);
if has_let_expr {
if does_not_have_bin_op {
// Remove the last feature gating of a `let` expression since it's stable.
self.sess.gated_spans.ungate_last(sym::let_chains, cond.span);
}
let span = if_span.to(cond.span);
self.sess.gated_spans.gate(sym::if_let_guard, span);
}
Ok(Some(cond))
}

fn parse_match_arm_pat_and_guard(&mut self) -> PResult<'a, (P<Pat>, Option<P<Expr>>)> {
if self.token.kind == token::OpenDelim(Delimiter::Parenthesis) {
// Detect and recover from `($pat if $cond) => $arm`.
let left = self.token.span;
match self.parse_pat_allow_top_alt(
None,
RecoverComma::Yes,
RecoverColon::Yes,
CommaRecoveryMode::EitherTupleOrPipe,
) {
Ok(pat) => Ok((pat, self.parse_match_arm_guard()?)),
Err(err)
if let prev_sp = self.prev_token.span
&& let true = self.eat_keyword(kw::If) =>
{
// We know for certain we've found `($pat if` so far.
let mut cond = match self.parse_match_guard_condition() {
Ok(cond) => cond,
Err(cond_err) => {
cond_err.cancel();
return Err(err);
}
};
err.cancel();
CondChecker::new(self).visit_expr(&mut cond);
self.eat_to_tokens(&[&token::CloseDelim(Delimiter::Parenthesis)]);
self.expect(&token::CloseDelim(Delimiter::Parenthesis))?;
let right = self.prev_token.span;
self.sess.emit_err(errors::ParenthesesInMatchPat {
span: vec![left, right],
sugg: errors::ParenthesesInMatchPatSugg { left, right },
});
Ok((self.mk_pat(left.to(prev_sp), ast::PatKind::Wild), Some(cond)))
}
Err(err) => Err(err),
}
} else {
// Regular parser flow:
let pat = self.parse_pat_allow_top_alt(
None,
RecoverComma::Yes,
RecoverColon::Yes,
CommaRecoveryMode::EitherTupleOrPipe,
)?;
Ok((pat, self.parse_match_arm_guard()?))
}
}

fn parse_match_guard_condition(&mut self) -> PResult<'a, P<Expr>> {
self.parse_expr_res(Restrictions::ALLOW_LET | Restrictions::IN_IF_GUARD, None).map_err(
|mut err| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ fn main() {
let val = 42;
let x = match val {
(0 if true) => {
//~^ ERROR expected identifier, found keyword `if`
//~| ERROR expected one of `)`, `,`, `...`, `..=`, `..`, or `|`, found keyword `if`
//~| ERROR expected one of `)`, `,`, `@`, or `|`, found keyword `true`
//~| ERROR mismatched types
//~^ ERROR unexpected parentheses surrounding `match` arm pattern
42u8
}
_ => 0u8,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,28 @@
error: expected one of `)`, `,`, `...`, `..=`, `..`, or `|`, found keyword `if`
--> $DIR/recover-parens-around-match-arm-head.rs:4:12
error: unexpected parentheses surrounding `match` arm pattern
--> $DIR/recover-parens-around-match-arm-head.rs:4:9
|
LL | (0 if true) => {
| ^^ expected one of `)`, `,`, `...`, `..=`, `..`, or `|`
| ^ ^
|
help: remove parentheses surrounding the pattern
|
LL - (0 if true) => {
LL + 0 if true => {
|

error[E0308]: mismatched types
--> $DIR/recover-parens-around-match-arm-head.rs:10:19
|
LL | let _y: u32 = x;
| --- ^ expected `u32`, found `u8`
| |
| expected due to this
|
help: you can convert a `u8` to a `u32`
|
LL | let _y: u32 = x.into();
| +++++++

error: aborting due to previous error
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0308`.

0 comments on commit a0cfc53

Please sign in to comment.