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

Better diagnostics for : instead of -> in return type #77115

Closed
LeSeulArtichaut opened this issue Sep 23, 2020 · 7 comments · Fixed by #78379
Closed

Better diagnostics for : instead of -> in return type #77115

LeSeulArtichaut opened this issue Sep 23, 2020 · 7 comments · Fixed by #78379
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints 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. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users.

Comments

@LeSeulArtichaut
Copy link
Contributor

Follow-up to #77019

Originally posted by @bugaevc in #77035 (comment):

Would also be cool to emit this upon encountering

fn foo(_arg: i32): i32 {
    ...
}

which is how you'd define the return type in Scala and Kotlin.

@LeSeulArtichaut LeSeulArtichaut added A-diagnostics Area: Messages for errors, warnings, and lints 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. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. labels Sep 23, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 23, 2020

For reference, the current error is

error: expected `;` or `{`, found `:`
 --> src/lib.rs:1:18
  |
1 | fn foo(_arg: i32): i32 {
  |                  ^ expected `;` or `{`

error: aborting due to previous error

Playground

@LeSeulArtichaut LeSeulArtichaut self-assigned this Sep 23, 2020
@camelid camelid added the D-confusing Diagnostics: Confusing error or lint that should be reworked. label Sep 24, 2020
@estebank
Copy link
Contributor

Pushed #78379 to use the regular "expected"/"found" error machinery, which will correctly say ->, ;, where or { can go there. That should be enough to direct people in the right direction (we would have to add a lot of checks to see if they meant ; or ->, which might be worth it 🤷‍♀️).

@pickfire
Copy link
Contributor

I used tools with expected/found error machinery like in go and nix, but I still find rust explicit method better, after trying out those tools I get confused most of the time. I think it is only good as a fallback but specific error messages should still be available if possible.

@bugaevc
Copy link

bugaevc commented Oct 26, 2020

For the reference, #77035 provides this nice specialized error for =>:

error: return types are denoted using `->`
  --> $DIR/fn-recover-return-sign.rs:3:8
   |
LL | fn a() => usize { 0 }
   |        ^^ help: use `->` instead

error: return types are denoted using `->`

@estebank
Copy link
Contributor

@bugaevc for a while now I've been thinking that we should have a general typo checker for arbitrary positions. Such a check would allow us to give suggestions for for example match foo { _ -> {} } without us explicitly coding it. Is this something you would also be interesting to work on?

@bugaevc
Copy link

bugaevc commented Oct 26, 2020

It wasn't me who implemented #77035 — that was @mibac138 🙂

That being said, I would be interested on working on something like that, yes! Perhaps you could describe how it could work and give me a few pointers?

@estebank
Copy link
Contributor

In my mind, we could introduce a check in expect and expect_one_of to check for confusable tokens. We'd have a mapping of similar looking or close enough in the keyboard and compare them against what was expected. If there's a match, emit a specific error and return a non-error back up.

In expected_one_of_not_found we already perform something similar for closing delimiters, so the machinery to signal recovery is already in place. That can be used higher in the call stack to avoid producing further errors caused by misparses. Ideally we could pass in a list of expected tokens after the next one to do some sanity checks for the recovery not to kick in if it is too far from what we'd expect. (Basically, just bail if the code is too far gone from normal code.)

@bors bors closed this as completed in 892ebe9 Oct 28, 2020
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 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. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants