Emit proper errors when on missing closure braces #88546

scrabsha · 2021-08-31T21:27:14Z

This commit focuses on emitting clean errors for the following syntax
error:

Some(42).map(|a|
    dbg!(a);
    a
);

Previous implementation tried to recover after parsing the closure body
(the dbg expression) by replacing the next ; with a ,, which made
the next expression belong to the next function argument. As such, the
following errors were emitted (among others):

the semicolon token was not expected,
a is not in scope,
Option::map is supposed to take one argument, not two.

This commit allows us to gracefully handle this situation by adding
giving the parser the ability to remember when it has just parsed a
closure body inside a function call. When this happens, we can treat the
unexpected ; specifically and try to parse as much statements as
possible in order to eat the whole block. When we can't parse statements
anymore, we generate a clean error indicating that the braces are
missing, and return an ExprKind::Err.

Closes #88065.

r? @estebank

compiler/rustc_parse/src/parser/mod.rs

compiler/rustc_parse/src/parser/expr.rs

src/test/ui/expr/malformed_closure/ruby_style_closure.rs

estebank · 2021-09-02T13:42:41Z

src/test/ui/expr/malformed_closure/ruby_style_closure.stderr

+error[E0277]: expected a `FnOnce<({integer},)>` closure, found `Option<_>`
+  --> $DIR/ruby_style_closure.rs:2:22
+   |
+LL |     let p = Some(45).and_then({
+   |                      ^^^^^^^^ expected an `FnOnce<({integer},)>` closure, found `Option<_>`


This should be pointing at the Some(x * 2) as well. (Not something to address in this PR)

Done in #88719

estebank

r=me after addressing the comments.

compiler/rustc_parse/src/parser/mod.rs

compiler/rustc_parse/src/parser/expr.rs

compiler/rustc_parse/src/parser/mod.rs

src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr

estebank · 2021-09-06T10:14:51Z

compiler/rustc_parse/src/parser/expr.rs

+        if self.token.kind == TokenKind::Semi && self.token_cursor.frame.delim == DelimToken::Paren
+        {
+            // It is likely that the closure body is a block but where the
+            // braces have been removed. We will recover and eat the next
+            // statements later in the parsing process.
+            body = self.mk_expr_err(body.span);
+        }


I'm somewhat uneasy with this check, but having both Semi and Paren be true should only happen on malformed code.

estebank · 2021-09-06T10:15:02Z

@bors r+

bors · 2021-09-06T10:15:04Z

📌 Commit 99c46c0 has been approved by estebank

…races, r=estebank Emit proper errors when on missing closure braces This commit focuses on emitting clean errors for the following syntax error: ``` Some(42).map(|a| dbg!(a); a ); ``` Previous implementation tried to recover after parsing the closure body (the `dbg` expression) by replacing the next `;` with a `,`, which made the next expression belong to the next function argument. As such, the following errors were emitted (among others): - the semicolon token was not expected, - a is not in scope, - Option::map is supposed to take one argument, not two. This commit allows us to gracefully handle this situation by adding giving the parser the ability to remember when it has just parsed a closure body inside a function call. When this happens, we can treat the unexpected `;` specifically and try to parse as much statements as possible in order to eat the whole block. When we can't parse statements anymore, we generate a clean error indicating that the braces are missing, and return an ExprKind::Err. Closes rust-lang#88065. r? `@estebank`

This commit focuses on emitting clean errors for the following syntax error: ``` Some(42).map(|a| dbg!(a); a ); ``` Previous implementation tried to recover after parsing the closure body (the `dbg` expression) by replacing the next `;` with a `,`, which made the next expression belong to the next function argument. As such, the following errors were emitted (among others): - the semicolon token was not expected, - a is not in scope, - Option::map is supposed to take one argument, not two. This commit allows us to gracefully handle this situation by adding giving the parser the ability to remember when it has just parsed a closure body inside a function call. When this happens, we can treat the unexpected `;` specifically and try to parse as much statements as possible in order to eat the whole block. When we can't parse statements anymore, we generate a clean error indicating that the braces are missing, and return an ExprKind::Err.

estebank · 2021-09-09T15:51:03Z

@bors r+

bors · 2021-09-09T15:51:05Z

📌 Commit b21425d has been approved by estebank

scrabsha · 2021-09-09T15:53:28Z

Thanks for the approval and sorry for the noise I created. I'll do better next time :)

…arth Rollup of 15 pull requests Successful merges: - rust-lang#85200 (Ignore derived Clone and Debug implementations during dead code analysis) - rust-lang#86165 (Add proc_macro::Span::{before, after}.) - rust-lang#87088 (Fix stray notes when the source code is not available) - rust-lang#87441 (Emit suggestion when passing byte literal to format macro) - rust-lang#88546 (Emit proper errors when on missing closure braces) - rust-lang#88578 (fix(rustc): suggest `items` be borrowed in `for i in items[x..]`) - rust-lang#88632 (Fix issues with Markdown summary options) - rust-lang#88639 (rustdoc: Fix ICE with `doc(hidden)` on tuple variant fields) - rust-lang#88667 (Tweak `write_fmt` doc.) - rust-lang#88720 (Rustdoc coverage fields count) - rust-lang#88732 (RustWrapper: avoid deleted unclear attribute methods) - rust-lang#88742 (Fix table in docblocks) - rust-lang#88776 (Workaround blink/chromium grid layout limitation of 1000 rows) - rust-lang#88807 (Fix typo in docs for iterators) - rust-lang#88812 (Fix typo `option` -> `options`.) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

… r=nagisa Point at argument instead of call for their obligations When an obligation is introduced by a specific `fn` argument, point at the argument instead of the `fn` call if the obligation fails to be fulfilled. Move the information about pointing at the call argument expression in an unmet obligation span from the `FulfillmentError` to a new `ObligationCauseCode`. When giving an error about an obligation introduced by a function call that an argument doesn't fulfill, and that argument is a block, add a span_label pointing at the innermost tail expression. Current output: ``` error[E0425]: cannot find value `x` in this scope --> f10.rs:4:14 | 4 | Some(x * 2) | ^ not found in this scope error[E0277]: expected a `FnOnce<({integer},)>` closure, found `Option<_>` --> f10.rs:2:31 | 2 | let p = Some(45).and_then({ | ______________________--------_^ | | | | | required by a bound introduced by this call 3 | | |x| println!("doubling {}", x); 4 | | Some(x * 2) | | ----------- 5 | | }); | |_____^ expected an `FnOnce<({integer},)>` closure, found `Option<_>` | = help: the trait `FnOnce<({integer},)>` is not implemented for `Option<_>` ``` Previous output: ``` error[E0425]: cannot find value `x` in this scope --> f10.rs:4:14 | 4 | Some(x * 2) | ^ not found in this scope error[E0277]: expected a `FnOnce<({integer},)>` closure, found `Option<_>` --> f10.rs:2:22 | 2 | let p = Some(45).and_then({ | ^^^^^^^^ expected an `FnOnce<({integer},)>` closure, found `Option<_>` | = help: the trait `FnOnce<({integer},)>` is not implemented for `Option<_>` ``` Partially address rust-lang#27300. Will require rebasing on top of rust-lang#88546.

…-in-macro, r=fee1-dead Allow semicolon after closure within parentheses in macros rust-lang#88546 added some parsing logic that if we're parsing a closure, and we're within parentheses, and a semicolon follows, then we must be parsing something erroneous like: `f(|| a; b)`, so it replaces the closure body with an error expression. However, it's valid to parse those tokens if we're within a macro, as in rust-lang#103222. This is a bit unsatisfying fix. Is there a more robust way of checking that we're within a macro? I would also be open to removing this "_It is likely that the closure body is a block but where the braces have been removed_" check altogether at the expense of more verbose errors, since it seems very suspicious in the first place... Fixes rust-lang#103222.

rust-highfive assigned estebank Aug 31, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 31, 2021

inquisitivecrystal reviewed Sep 1, 2021

View reviewed changes

compiler/rustc_parse/src/parser/mod.rs Outdated Show resolved Hide resolved

compiler/rustc_parse/src/parser/mod.rs Outdated Show resolved Hide resolved

scrabsha force-pushed the scrabsha/closure-missing-braces branch from 921a11b to 3bdda1c Compare September 1, 2021 07:52

estebank reviewed Sep 1, 2021

View reviewed changes

compiler/rustc_parse/src/parser/mod.rs Outdated Show resolved Hide resolved

estebank reviewed Sep 1, 2021

View reviewed changes

compiler/rustc_parse/src/parser/expr.rs Outdated Show resolved Hide resolved

estebank reviewed Sep 2, 2021

View reviewed changes

src/test/ui/expr/malformed_closure/ruby_style_closure.rs Show resolved Hide resolved

estebank reviewed Sep 2, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

estebank approved these changes Sep 3, 2021

View reviewed changes

compiler/rustc_parse/src/parser/mod.rs Outdated Show resolved Hide resolved

compiler/rustc_parse/src/parser/expr.rs Outdated Show resolved Hide resolved

compiler/rustc_parse/src/parser/mod.rs Outdated Show resolved Hide resolved

estebank reviewed Sep 3, 2021

View reviewed changes

src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr Outdated Show resolved Hide resolved

estebank reviewed Sep 6, 2021

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 6, 2021

estebank mentioned this pull request Sep 7, 2021

Point at argument instead of call for their obligations #88719

Merged

GuillaumeGomez mentioned this pull request Sep 9, 2021

Rollup of 7 pull requests #88774

Closed

scrabsha force-pushed the scrabsha/closure-missing-braces branch from 99c46c0 to b21425d Compare September 9, 2021 15:47

Manishearth mentioned this pull request Sep 10, 2021

Rollup of 15 pull requests #88824

Merged

bors merged commit dc003dd into rust-lang:master Sep 11, 2021

rustbot added this to the 1.57.0 milestone Sep 11, 2021

scrabsha deleted the scrabsha/closure-missing-braces branch September 14, 2021 07:46

compiler-errors mentioned this pull request Oct 19, 2022

Allow semicolon after closure within parentheses in macros #103224

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Emit proper errors when on missing closure braces #88546

Emit proper errors when on missing closure braces #88546

scrabsha commented Aug 31, 2021 •

edited

Loading

estebank Sep 2, 2021 •

edited

Loading

estebank Sep 7, 2021

This comment has been minimized.

estebank left a comment

estebank Sep 6, 2021

estebank commented Sep 6, 2021

bors commented Sep 6, 2021

estebank commented Sep 9, 2021

bors commented Sep 9, 2021

scrabsha commented Sep 9, 2021

Emit proper errors when on missing closure braces #88546

Emit proper errors when on missing closure braces #88546

Conversation

scrabsha commented Aug 31, 2021 • edited Loading

estebank Sep 2, 2021 • edited Loading

Choose a reason for hiding this comment

estebank Sep 7, 2021

Choose a reason for hiding this comment

This comment has been minimized.

estebank left a comment

Choose a reason for hiding this comment

estebank Sep 6, 2021

Choose a reason for hiding this comment

estebank commented Sep 6, 2021

bors commented Sep 6, 2021

estebank commented Sep 9, 2021

bors commented Sep 9, 2021

scrabsha commented Sep 9, 2021

scrabsha commented Aug 31, 2021 •

edited

Loading

estebank Sep 2, 2021 •

edited

Loading