-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Point at last valid token on failed expect_one_of
#40811
Conversation
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
ea6cde5
to
03eca71
Compare
```rust error: expected one of `.`, `;`, `?`, `}`, or an operator, found `)` --> $DIR/token-error-correct-3.rs:29:9 | 25 | foo() | - expected one of `.`, `;`, `?`, `}`, or an operator after this ... 29 | } else { | ^ unexpected token ```
Sounds wrong. It says it expects those symbols instead of/before the parenthesis, although I think it might just be the example mis-copied and not actual output from the compiler (as the tests look good)? I would also investigate if something like this is possible (note how label is shifted by 1):
|
Indeed, that was the case, @nagisa.
I was afraid the output when the unexpected token is right after the last correctly parsed token wouldn't be too great, but now that I mocked it out, it doesn't seem too bad: |
25 | foo()} else {
| ^
| |
| expected one of `.`, `;`, `?`, `}`, or an operator here
| unexpected token |
* Point at where the token was expected instead of the last token successfuly parsed. * Only show `unexpected token` if the next char and the unexpected token don't have the same span. * Change some cfail and pfail tests to ui test. * Don't show all possible tokens in span label if they are more than 6.
I've changed it so that when the next parsed token is in the same place as the next expected token, we only show the most detailed label 25 | foo()} else {
| ^ expected one of `.`, `;`, `?`, `}`, or an operator here vs. 25 | fs::create_dir_all(path.as_ref()).map(|()| true)
| - expected one of `.`, `;`, `?`, `}`, or an operator here
...
29 | } else {
| ^ unexpected token |
Ping. |
or maybe @nrc |
I'd probably detect this case:
And just give a single message instead, like:
|
@jonathandturner I added a check for it that does that. From one of the tests: error: expected one of `,`, `.`, `?`, or an operator, found `;`
--> $DIR/token-error-correct-3.rs:23:35
|
23 | callback(path.as_ref(); //~ NOTE: unclosed delimiter
| ^ expected one of `,`, `.`, `?`, or an operator here |
LGTM @bors r+ |
📌 Commit b477682 has been approved by |
@jonathandturner fixed merge conflict. @bors r=jonathandturner |
📌 Commit dedb7bb has been approved by |
Point at last valid token on failed `expect_one_of` ```rust error: expected one of `.`, `;`, `?`, `}`, or an operator, found `)` --> $DIR/token-error-correct-3.rs:29:9 | 25 | foo() | - expected one of `.`, `;`, `?`, `}`, or an operator after this ... 29 | } else { | ^ unexpected token ``` Fix #32540.
☀️ Test successful - status-appveyor, status-travis |
Fix #32540.