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

Detect ruby-style closure in parser #116645

Merged
merged 1 commit into from
Oct 13, 2023
Merged
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
59 changes: 59 additions & 0 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,65 @@ impl<'a> Parser<'a> {
None
}

pub(super) fn recover_closure_body(
&mut self,
mut err: DiagnosticBuilder<'a, ErrorGuaranteed>,
before: token::Token,
prev: token::Token,
token: token::Token,
lo: Span,
decl_hi: Span,
) -> PResult<'a, P<Expr>> {
err.span_label(lo.to(decl_hi), "while parsing the body of this closure");
match before.kind {
token::OpenDelim(Delimiter::Brace)
if !matches!(token.kind, token::OpenDelim(Delimiter::Brace)) =>
{
// `{ || () }` should have been `|| { () }`
err.multipart_suggestion(
"you might have meant to open the body of the closure, instead of enclosing \
the closure in a block",
vec![
(before.span, String::new()),
(prev.span.shrink_to_hi(), " {".to_string()),
],
Applicability::MaybeIncorrect,
);
err.emit();
self.eat_to_tokens(&[&token::CloseDelim(Delimiter::Brace)]);
}
token::OpenDelim(Delimiter::Parenthesis)
if !matches!(token.kind, token::OpenDelim(Delimiter::Brace)) =>
{
// We are within a function call or tuple, we can emit the error
// and recover.
self.eat_to_tokens(&[&token::CloseDelim(Delimiter::Parenthesis), &token::Comma]);

err.multipart_suggestion_verbose(
"you might have meant to open the body of the closure",
vec![
(prev.span.shrink_to_hi(), " {".to_string()),
(self.token.span.shrink_to_lo(), "}".to_string()),
],
Applicability::MaybeIncorrect,
);
err.emit();
}
_ if !matches!(token.kind, token::OpenDelim(Delimiter::Brace)) => {
// We don't have a heuristic to correctly identify where the block
// should be closed.
err.multipart_suggestion_verbose(
"you might have meant to open the body of the closure",
vec![(prev.span.shrink_to_hi(), " {".to_string())],
Applicability::HasPlaceholders,
);
return Err(err);
}
_ => return Err(err),
}
Ok(self.mk_expr_err(lo.to(self.token.span)))
}

/// Eats and discards tokens until one of `kets` is encountered. Respects token trees,
/// passes through any errors encountered. Used for error recovery.
pub(super) fn eat_to_tokens(&mut self, kets: &[&TokenKind]) {
Expand Down
18 changes: 15 additions & 3 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2209,6 +2209,7 @@ impl<'a> Parser<'a> {
fn parse_expr_closure(&mut self) -> PResult<'a, P<Expr>> {
let lo = self.token.span;

let before = self.prev_token.clone();
let binder = if self.check_keyword(kw::For) {
let lo = self.token.span;
let lifetime_defs = self.parse_late_bound_lifetime_defs()?;
Expand Down Expand Up @@ -2239,7 +2240,12 @@ impl<'a> Parser<'a> {
FnRetTy::Default(_) => {
let restrictions =
self.restrictions - Restrictions::STMT_EXPR - Restrictions::ALLOW_LET;
self.parse_expr_res(restrictions, None)?
let prev = self.prev_token.clone();
let token = self.token.clone();
match self.parse_expr_res(restrictions, None) {
Ok(expr) => expr,
Err(err) => self.recover_closure_body(err, before, prev, token, lo, decl_hi)?,
}
}
_ => {
// If an explicit return type is given, require a block to appear (RFC 968).
Expand Down Expand Up @@ -2459,10 +2465,16 @@ impl<'a> Parser<'a> {
/// Parses a `let $pat = $expr` pseudo-expression.
fn parse_expr_let(&mut self, restrictions: Restrictions) -> PResult<'a, P<Expr>> {
let is_recovered = if !restrictions.contains(Restrictions::ALLOW_LET) {
Some(self.sess.emit_err(errors::ExpectedExpressionFoundLet {
let err = errors::ExpectedExpressionFoundLet {
span: self.token.span,
reason: ForbiddenLetReason::OtherForbidden,
}))
};
if self.prev_token.kind == token::BinOp(token::Or) {
// This was part of a closure, the that part of the parser recover.
return Err(err.into_diagnostic(&self.sess.span_diagnostic));
} else {
Some(self.sess.emit_err(err))
}
} else {
None
};
Expand Down
10 changes: 10 additions & 0 deletions tests/ui/expr/malformed_closure/missing_block_in_fn_call.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// run-rustfix
fn main() {
let _ = vec![1, 2, 3].into_iter().map(|x| {
let y = x; //~ ERROR expected expression, found `let` statement
y
});
let _: () = foo(); //~ ERROR mismatched types
}

fn foo() {}
10 changes: 10 additions & 0 deletions tests/ui/expr/malformed_closure/missing_block_in_fn_call.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// run-rustfix
fn main() {
let _ = vec![1, 2, 3].into_iter().map(|x|
let y = x; //~ ERROR expected expression, found `let` statement
y
);
let _: () = foo; //~ ERROR mismatched types
}

fn foo() {}
38 changes: 38 additions & 0 deletions tests/ui/expr/malformed_closure/missing_block_in_fn_call.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
error: expected expression, found `let` statement
--> $DIR/missing_block_in_fn_call.rs:4:9
|
LL | let _ = vec![1, 2, 3].into_iter().map(|x|
| --- while parsing the body of this closure
LL | let y = x;
| ^^^
|
= note: only supported directly in conditions of `if` and `while` expressions
help: you might have meant to open the body of the closure
|
LL ~ let _ = vec![1, 2, 3].into_iter().map(|x| {
LL | let y = x;
LL | y
LL ~ });
|

error[E0308]: mismatched types
--> $DIR/missing_block_in_fn_call.rs:7:17
|
LL | let _: () = foo;
| -- ^^^ expected `()`, found fn item
| |
| expected due to this
...
LL | fn foo() {}
| -------- function `foo` defined here
|
= note: expected unit type `()`
found fn item `fn() {foo}`
help: use parentheses to call this function
|
LL | let _: () = foo();
| ++

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0308`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
fn main() {
let x = |x|
let y = x; //~ ERROR expected expression, found `let` statement
let _ = () + ();
y
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: expected expression, found `let` statement
--> $DIR/missing_block_in_let_binding.rs:3:9
|
LL | let x = |x|
| --- while parsing the body of this closure
LL | let y = x;
| ^^^
|
= note: only supported directly in conditions of `if` and `while` expressions
help: you might have meant to open the body of the closure
|
LL | let x = |x| {
| +

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// run-rustfix
fn main() {
let _ = vec![1, 2, 3].into_iter().map(|x| {
let y = x; //~ ERROR expected expression, found `let` statement
y
});
let _: () = foo(); //~ ERROR mismatched types
}
fn foo() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// run-rustfix
fn main() {
let _ = vec![1, 2, 3].into_iter().map({|x|
let y = x; //~ ERROR expected expression, found `let` statement
y
});
let _: () = foo; //~ ERROR mismatched types
}
fn foo() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
error: expected expression, found `let` statement
--> $DIR/ruby_style_closure_parse_error.rs:4:9
|
LL | let _ = vec![1, 2, 3].into_iter().map({|x|
| --- while parsing the body of this closure
LL | let y = x;
| ^^^
|
= note: only supported directly in conditions of `if` and `while` expressions
help: you might have meant to open the body of the closure, instead of enclosing the closure in a block
|
LL - let _ = vec![1, 2, 3].into_iter().map({|x|
LL + let _ = vec![1, 2, 3].into_iter().map(|x| {
|

error[E0308]: mismatched types
--> $DIR/ruby_style_closure_parse_error.rs:7:17
|
LL | let _: () = foo;
| -- ^^^ expected `()`, found fn item
| |
| expected due to this
LL | }
LL | fn foo() {}
| -------- function `foo` defined here
|
= note: expected unit type `()`
found fn item `fn() {foo}`
help: use parentheses to call this function
|
LL | let _: () = foo();
| ++

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0308`.
8 changes: 7 additions & 1 deletion tests/ui/or-patterns/or-patterns-syntactic-fail.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,15 @@ error: expected identifier, found `:`
--> $DIR/or-patterns-syntactic-fail.rs:11:19
|
LL | let _ = |A | B: E| ();
| ^ expected identifier
| ---- ^ expected identifier
| |
| while parsing the body of this closure
|
= note: type ascription syntax has been removed, see issue #101728 <https://github.com/rust-lang/rust/issues/101728>
help: you might have meant to open the body of the closure
|
LL | let _ = |A | { B: E| ();
| +
Comment on lines +10 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a false positive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I saw this, but I have no clue how we're meant to even start to try to parse such code in practice 😅

I feel it's fine, it still gives people some sense of what's happening.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea... idk. I just feel like the suggestion doesn't help. But maybe it's better than without it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can always special case it, but the previous error wasn't much better, tbqh


error: top-level or-patterns are not allowed in function parameters
--> $DIR/or-patterns-syntactic-fail.rs:18:13
Expand Down
1 change: 1 addition & 0 deletions tests/ui/parser/issues/issue-32505.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub fn test() {
foo(|_|) //~ ERROR expected expression, found `)`
//~^ ERROR cannot find function `foo` in this scope
}

fn main() { }
18 changes: 16 additions & 2 deletions tests/ui/parser/issues/issue-32505.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,21 @@ error: expected expression, found `)`
--> $DIR/issue-32505.rs:2:12
|
LL | foo(|_|)
| ^ expected expression
| ---^ expected expression
| |
| while parsing the body of this closure
|
help: you might have meant to open the body of the closure
|
LL | foo(|_| {})
| ++

error[E0425]: cannot find function `foo` in this scope
--> $DIR/issue-32505.rs:2:5
|
LL | foo(|_|)
| ^^^ not found in this scope

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

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