-
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
Use heuristics to suggest assignment #65566
Conversation
r? @zackmdavis (rust_highfive has picked a reviewer for you, use r? to override) |
Follow up work would be to silence likely knock-down errors. To do, we should keep track of having suggested |
src/librustc_resolve/late.rs
Outdated
let val = match local { | ||
Local { pat, ty: Some(ty), init: None, .. } => match pat.kind { | ||
// We check for this to avoid tuple struct fields. | ||
PatKind::Wild => None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have let _: <type>
here -- what's special about that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've seen no way to differentiate between let _: <type>;
and struct T(usize);
. Without this filter the suggestion triggers incorrectly for the latter in one of the tests.
Is this a commonly occurring problem? cc @hsivonen didn't really elaborate and they are not a newcomer. |
We already handle this kind of error when there are parse errors, but this case is insidious because it parses correctly and then causes multiple knock down errors. The big win would be to turn the three errors into just one. Edit: I think this is a common thing to happen when editing code, akin to trailing commas or missing semicolons. I know I've hit it a couple of times. |
3b245fb
to
cdc5400
Compare
I don't know how commonly this occurs, but when it did occur, it was confusing. |
@hsivonen would the current output have been enough for you to quickly identify the issue?
Further improvements would get pretty hairy pretty quickly. |
Likely yes. Thank you! |
r? @Centril |
@bors r+ |
📌 Commit 10ce36c has been approved by |
⌛ Testing commit 10ce36c with merge cf96b4bad10c951e1e6e0053fe091868fafa5ac9... |
@bors treeclosed=1000 |
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-azure |
It seems chocolatey is now available, let's check if it works. |
@bors retry |
Use heuristics to suggest assignment When detecting a possible `=` -> `:` typo in a `let` binding, suggest assigning instead of setting the type. Partially address rust-lang#57828.
Fallout from the suggestions change. I'll rebase and fix. |
When detecting a possible `=` -> `:` typo in a `let` binding, suggest assigning instead of setting the type.
10ce36c
to
b579c5a
Compare
@bors r=Centril |
📌 Commit b579c5a has been approved by |
Use heuristics to suggest assignment When detecting a possible `=` -> `:` typo in a `let` binding, suggest assigning instead of setting the type. Partially address rust-lang#57828.
Rollup of 6 pull requests Successful merges: - #65566 (Use heuristics to suggest assignment) - #65738 (Coherence should allow fundamental types to impl traits when they are local) - #65777 (Don't ICE for completely unexpandable `impl Trait` types) - #65834 (Remove lint callback from driver) - #65839 (Clean up `check_consts` now that new promotion pass is implemented) - #65855 (Add long error explaination for E0666) Failed merges: r? @ghost
When detecting a possible
=
->:
typo in alet
binding, suggestassigning instead of setting the type.
Partially address #57828.