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

Suggest adding a => after match guard #94932

Closed
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
31 changes: 30 additions & 1 deletion compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2480,7 +2480,7 @@ impl<'a> Parser<'a> {
)?;
let guard = if this.eat_keyword(kw::If) {
let if_span = this.prev_token.span;
let cond = this.parse_expr()?;
let cond = this.parse_match_guard_cond()?;
let (has_let_expr, does_not_have_bin_op) = check_let_expr(&cond);
if has_let_expr {
if does_not_have_bin_op {
Expand All @@ -2496,6 +2496,15 @@ impl<'a> Parser<'a> {
};
let arrow_span = this.token.span;
if let Err(mut err) = this.expect(&token::FatArrow) {
if guard.is_some() && this.token.kind == token::OpenDelim(token::Brace) {
err.span_suggestion_verbose(
this.token.span.shrink_to_lo(),
"try adding a fat arrow here",
"=> ".to_string(),
Applicability::MaybeIncorrect,
);
err.emit();
}
// We might have a `=>` -> `=` or `->` typo (issue #89396).
if TokenKind::FatArrow
.similar_tokens()
Expand Down Expand Up @@ -2617,6 +2626,26 @@ impl<'a> Parser<'a> {
}
}

fn parse_match_guard_cond(&mut self) -> PResult<'a, P<Expr>> {
let snapshot = self.clone();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only issue I'm seeing here is that this pessimises the parsing of every match arm. In this case I think it is reasonable to instead catch the problem in line 2499 and only in the error arm try to conditionally parse the match arm body. Probably also do that for the substitution cases handled right below.

let mut cond = self.parse_expr()?;
let (is_lhs_struct, is_rhs_struct) = match cond.kind {
ExprKind::Binary(_, ref lhs, ref rhs) => {
(matches!(lhs.kind, ExprKind::Struct(..)), matches!(rhs.kind, ExprKind::Struct(..)))
}
_ => (false, false),
};
Comment on lines +2632 to +2637
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you recognize here, we might want to also detect when a struct literal was parsed that instead was supposed to be an ident and the match arm body, but that is not as critical, I think.

if !is_lhs_struct
&& (!is_rhs_struct
|| is_rhs_struct && self.token.kind != token::OpenDelim(token::Brace))
&& self.token.kind != token::FatArrow
{
*self = snapshot;
cond = self.parse_cond_expr()?;
}
Ok(cond)
}

fn is_do_catch_block(&self) -> bool {
self.token.is_keyword(kw::Do)
&& self.is_keyword_ahead(1, &[kw::Catch])
Expand Down
5 changes: 2 additions & 3 deletions src/test/ui/parser/issues/issue-15980.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@ fn main(){
match x {
Err(ref e) if e.kind == io::EndOfFile {
//~^ NOTE while parsing this struct
//~| ERROR expected one of `!`, `.`, `::`, `=>`, `?`, or an operator, found `{`
//~| NOTE expected one of `!`, `.`, `::`, `=>`, `?`, or an operator
return
//~^ ERROR expected identifier, found keyword `return`
//~| NOTE expected identifier, found keyword
}
//~^ NOTE expected one of `.`, `=>`, `?`, or an operator
_ => {}
//~^ ERROR expected one of `.`, `=>`, `?`, or an operator, found reserved identifier `_`
//~| NOTE unexpected token
}
}
20 changes: 11 additions & 9 deletions src/test/ui/parser/issues/issue-15980.stderr
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
error: expected identifier, found keyword `return`
--> $DIR/issue-15980.rs:8:13
--> $DIR/issue-15980.rs:10:13
|
LL | Err(ref e) if e.kind == io::EndOfFile {
| ------------- while parsing this struct
LL |
...
LL | return
| ^^^^^^ expected identifier, found keyword
|
Expand All @@ -12,14 +12,16 @@ help: escape `return` to use it as an identifier
LL | r#return
| ++

error: expected one of `.`, `=>`, `?`, or an operator, found reserved identifier `_`
--> $DIR/issue-15980.rs:13:9
error: expected one of `!`, `.`, `::`, `=>`, `?`, or an operator, found `{`
--> $DIR/issue-15980.rs:6:47
|
LL | }
| - expected one of `.`, `=>`, `?`, or an operator
LL |
LL | _ => {}
| ^ unexpected token
LL | Err(ref e) if e.kind == io::EndOfFile {
| ^ expected one of `!`, `.`, `::`, `=>`, `?`, or an operator
|
help: try adding a fat arrow here
|
LL | Err(ref e) if e.kind == io::EndOfFile => {
| ++

error: aborting due to 2 previous errors

28 changes: 28 additions & 0 deletions src/test/ui/parser/missing-fat-arrow-after-match-guard.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#[derive(PartialEq)]
struct Foo {
x: isize,
}

fn foo() {
match () {
() if f == Foo { x: 42 } {}
//~^ ERROR expected one of `.`, `=>`, `?`, or an operator, found `{`
_ => {}
}
}

fn main() {
let x = 1;
let y = 2;
let value = 3;

match value {
Some(x) if x == y {
//~^ ERROR expected one of `!`, `.`, `::`, `=>`, `?`, or an operator, found `{`
x
},
_ => {
y
}
}
}
24 changes: 24 additions & 0 deletions src/test/ui/parser/missing-fat-arrow-after-match-guard.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
error: expected one of `.`, `=>`, `?`, or an operator, found `{`
--> $DIR/missing-fat-arrow-after-match-guard.rs:8:34
|
LL | () if f == Foo { x: 42 } {}
| ^ expected one of `.`, `=>`, `?`, or an operator
|
help: try adding a fat arrow here
|
LL | () if f == Foo { x: 42 } => {}
| ++

error: expected one of `!`, `.`, `::`, `=>`, `?`, or an operator, found `{`
--> $DIR/missing-fat-arrow-after-match-guard.rs:20:27
|
LL | Some(x) if x == y {
| ^ expected one of `!`, `.`, `::`, `=>`, `?`, or an operator
|
help: try adding a fat arrow here
|
LL | Some(x) if x == y => {
| ++

error: aborting due to 2 previous errors