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

Improve error message for missing parens in tuple destructure in for loop #48492

Closed
comex opened this issue Feb 24, 2018 · 3 comments
Closed
Labels
A-diagnostics Area: Messages for errors, warnings, and lints

Comments

@comex
Copy link
Contributor

comex commented Feb 24, 2018

Given:

    for foo, bar in [(1, 2), (3, 4)] {}

rustc produces:

error: missing `in` in `for` loop
  --> src/main.rs:44:12
   |
44 |     for foo, bar in [(1, 2)] {}
   |            ^ help: try adding `in` here

error: expected expression, found `,`
  --> src/main.rs:44:12
   |
44 |     for foo, bar in [(1, 2)] {}
   |            ^

…but it would be better if it figured out that the user presumably intended for (foo, bar) in ..., and gave a more specific error message.

This is probably a somewhat common error coming from Python, which has similar syntax for for loops but allows omitting the parentheses.

@stokhos stokhos added the A-diagnostics Area: Messages for errors, warnings, and lints label Feb 24, 2018
@petrochenkov
Copy link
Contributor

If this is something Python-inspired, then these diagnostics/recovery should ideally apply to let as well.

let a, b = (10, 11);

=>

let (a, b) = (10, 11);

I suspect we can even implement this recovery as "auto-tupling" in all expression and pattern contexts.

illegal a, b, c

=>

(a, b, c)

@dtolnay
Copy link
Member

dtolnay commented Feb 25, 2018

👍 Not just Python, this will be helpful for Go users as well.

for k, v := range myMap {
    x, err = /* ... */
}

@zackmdavis
Copy link
Member

(I've made some progress on this; PR forthcoming)

zackmdavis added a commit to zackmdavis/rust that referenced this issue Feb 28, 2018
Programmers used to working in some other languages (such as Python or
Go) might expect to be able to destructure values with comma-separated
identifiers but no parentheses on the left side of an assignment.

Previously, the first name in such code would get parsed as a
single-indentifier pattern—recognizing, for example, the
`let a` in `let a, b = (1, 2);`—whereupon we would have a fatal syntax
error on seeing an unexpected comma rather than the expected semicolon
(all the way nearer to the end of `parse_full_stmt`).

Instead, let's look for that comma when parsing the pattern, and if we
see it, make-believe that we're parsing the remaining elements in a
tuple pattern, so that we can suggest wrapping it all in parentheses. We
need to do this in a separate wrapper method called on the top-level
pattern (or `|`-patterns) in a `let` statement, `for` loop, `if`- or
`while let` expression, or match arm rather than within `parse_pat`
itself, because `parse_pat` gets called recursively to parse the
sub-patterns within a tuple pattern.

Resolves rust-lang#48492.
zackmdavis added a commit to zackmdavis/rust that referenced this issue Mar 8, 2018
Programmers used to working in some other languages (such as Python or
Go) might expect to be able to destructure values with comma-separated
identifiers but no parentheses on the left side of an assignment.

Previously, the first name in such code would get parsed as a
single-indentifier pattern—recognizing, for example, the
`let a` in `let a, b = (1, 2);`—whereupon we would have a fatal syntax
error on seeing an unexpected comma rather than the expected semicolon
(all the way nearer to the end of `parse_full_stmt`).

Instead, let's look for that comma when parsing the pattern, and if we
see it, momentarily make-believe that we're parsing the remaining
elements in a tuple pattern, so that we can suggest wrapping it all in
parentheses. We need to do this in a separate wrapper method called on
the top-level pattern (or `|`-patterns) in a `let` statement, `for`
loop, `if`- or `while let` expression, or match arm rather than within
`parse_pat` itself, because `parse_pat` gets called recursively to parse
the sub-patterns within a tuple pattern.

Resolves rust-lang#48492.
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 9, 2018
…ion_of_tuples, r=estebank

in which parentheses are suggested for should-have-been-tuple-patterns

![destructure_suggest_parens](https://user-images.githubusercontent.com/1076988/36638335-48b082d4-19a7-11e8-9726-0d043544df2f.png)

Programmers used to working in some other languages (such as Python or
Go) might expect to be able to destructure values with comma-separated
identifiers but no parentheses on the left side of an assignment.

Previously, the first name in such code would get parsed as a
single-indentifier pattern—recognizing, for example, the
`let a` in `let a, b = (1, 2);`—whereupon we would have a fatal syntax
error on seeing an unexpected comma rather than the expected semicolon
(all the way nearer to the end of `parse_full_stmt`).

Instead, let's look for that comma when parsing the pattern, and if we
see it, make-believe that we're parsing the remaining elements in a
tuple pattern, so that we can suggest wrapping it all in parentheses. We
need to do this in a separate wrapper method called on a "top-level"
pattern, rather than within
`parse_pat` itself, because `parse_pat` gets called recursively to parse
the sub-patterns within a tuple pattern.

~~We could also do this for `match` arms, `if let`, and `while let`, but
we elect not to in this patch, as it seems less likely for users to make
the mistake in those contexts.~~

Resolves rust-lang#48492.

r? @petrochenkov
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints
Projects
None yet
Development

No branches or pull requests

5 participants