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.

Sign up for GitHub

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

Jump to bottom

Improving error for unnecessary parentheses in for loop head #62724

Closed
badboy opened this issue Jul 16, 2019 · 2 comments · Fixed by #62928
Closed

Improving error for unnecessary parentheses in for loop head #62724

badboy opened this issue Jul 16, 2019 · 2 comments · Fixed by #62928
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-enhancement Category: An issue proposing an enhancement or a PR with one. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@badboy
Copy link
Member

badboy commented Jul 16, 2019

The following code:

fn main() {
    let v = vec![1,2,3];
    
    for (x in v) { }
}

leads to this error:

error: expected one of `)`, `,`, or `@`, found `in`
 --> src/main.rs:4:12
  |
4 |     for (x in v) { }
  |            ^^ expected one of `)`, `,`, or `@` here

(playground)

A colleague of mine typed this code, coming with a C++/JavaScript/Kotlin background where parentheses are required in the for loop head.

I wonder if it’s possible/worth it to make that error message nicer and easier to understand. (@estebank asked me to file this issue, so it doesn't get lost)
@jonas-schievink jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 16, 2019
@estebank estebank added A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` P-low Low priority labels Jul 16, 2019
@hellow554
Copy link
Contributor

I wonder why this isn't accepted. You can write if (x == 3) { }, so shouldn't this code be accepted as well? 🤔

@badboy
Copy link
Member Author

badboy commented Jul 17, 2019

I wonder why this isn't accepted. You can write if (x == 3) { }, so shouldn't this code be accepted as well? 🤔

The syntax for that loop is explicitly "for" $pattern "in" $expression {}.
For if the syntax would be if $expression {} and only the expression can contain the parentheses.
Of course it could be just accepted with optional parentheses for the loop, but we don't really want that (and parentheses in if are a lint-warning)

@Centril Centril mentioned this issue Jul 24, 2019
Merged
Centril added a commit to Centril/rust that referenced this issue Jul 29, 2019
@Centril
Rollup merge of rust-lang#62928 - Centril:recover-parens-around-for-h…
04ec35b 
…ead, r=estebank

Syntax: Recover on `for ( $pat in $expr ) $block`

Fixes rust-lang#62724 by adding some recovery:

```
error: unexpected closing `)`
  --> $DIR/recover-for-loop-parens-around-head.rs:10:23
   |
LL |     for ( elem in vec ) {
   |         --------------^
   |         |
   |         opening `(`
   |         help: remove parenthesis in `for` loop: `elem in vec`
```

The last 2 commits are drive-by cleanups.

r? @estebank
Centril added a commit to Centril/rust that referenced this issue Jul 29, 2019
@Centril
Rollup merge of rust-lang#62928 - Centril:recover-parens-around-for-h…
b867bb0 
…ead, r=estebank

Syntax: Recover on `for ( $pat in $expr ) $block`

Fixes rust-lang#62724 by adding some recovery:

```
error: unexpected closing `)`
  --> $DIR/recover-for-loop-parens-around-head.rs:10:23
   |
LL |     for ( elem in vec ) {
   |         --------------^
   |         |
   |         opening `(`
   |         help: remove parenthesis in `for` loop: `elem in vec`
```

The last 2 commits are drive-by cleanups.

r? @estebank
Centril added a commit to Centril/rust that referenced this issue Jul 30, 2019
@Centril
Rollup merge of rust-lang#62928 - Centril:recover-parens-around-for-h…
022f7a7 
…ead, r=estebank

Syntax: Recover on `for ( $pat in $expr ) $block`

Fixes rust-lang#62724 by adding some recovery:

```
error: unexpected closing `)`
  --> $DIR/recover-for-loop-parens-around-head.rs:10:23
   |
LL |     for ( elem in vec ) {
   |         --------------^
   |         |
   |         opening `(`
   |         help: remove parenthesis in `for` loop: `elem in vec`
```

The last 2 commits are drive-by cleanups.

r? @estebank
Centril added a commit to Centril/rust that referenced this issue Jul 30, 2019
@Centril
Rollup merge of rust-lang#62928 - Centril:recover-parens-around-for-h…
3602987 
…ead, r=estebank

Syntax: Recover on `for ( $pat in $expr ) $block`

Fixes rust-lang#62724 by adding some recovery:

```
error: unexpected closing `)`
  --> $DIR/recover-for-loop-parens-around-head.rs:10:23
   |
LL |     for ( elem in vec ) {
   |         --------------^
   |         |
   |         opening `(`
   |         help: remove parenthesis in `for` loop: `elem in vec`
```

The last 2 commits are drive-by cleanups.

r? @estebank
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-enhancement Category: An issue proposing an enhancement or a PR with one. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Syntax: Recover on for ( $pat in $expr ) $block Centril/rust
4 participants
@badboy @hellow554 @estebank @jonas-schievink