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

Use heuristics to suggest assignment #65566

Merged
merged 5 commits into from Oct 27, 2019

Conversation

@estebank
Copy link
Contributor

estebank commented Oct 18, 2019

When detecting a possible = -> : typo in a let binding, suggest
assigning instead of setting the type.

Partially address #57828.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Oct 18, 2019

r? @zackmdavis

(rust_highfive has picked a reviewer for you, use r? to override)

@estebank

This comment has been minimized.

Copy link
Contributor Author

estebank commented Oct 18, 2019

Follow up work would be to silence likely knock-down errors. To do, we should keep track of having suggested := (the span is enough) and not emit the "invalid parenthesized type parameters" and "ambiguous associated type" errors if their spans are contained by it.

src/librustc_resolve/late.rs Outdated Show resolved Hide resolved
src/librustc_resolve/late.rs Outdated Show resolved Hide resolved
src/librustc_resolve/late.rs Outdated Show resolved Hide resolved
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,

This comment has been minimized.

Copy link
@Centril

Centril Oct 19, 2019

Member

We have let _: <type> here -- what's special about that?

This comment has been minimized.

Copy link
@estebank

estebank Oct 19, 2019

Author Contributor

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.

src/librustc_resolve/late/diagnostics.rs Outdated Show resolved Hide resolved
@Centril

This comment has been minimized.

Copy link
Member

Centril commented Oct 19, 2019

Is this a commonly occurring problem? cc @hsivonen didn't really elaborate and they are not a newcomer.

@estebank

This comment has been minimized.

Copy link
Contributor Author

estebank commented Oct 19, 2019

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.

@estebank estebank force-pushed the estebank:let-expr-as-ty branch from 3b245fb to cdc5400 Oct 19, 2019
src/librustc_resolve/late.rs Outdated Show resolved Hide resolved
@hsivonen

This comment has been minimized.

Copy link
Contributor

hsivonen commented Oct 21, 2019

Is this a commonly occurring problem?

I don't know how commonly this occurs, but when it did occur, it was confusing.

@estebank

This comment has been minimized.

Copy link
Contributor Author

estebank commented Oct 21, 2019

@hsivonen would the current output have been enough for you to quickly identify the issue?

error[E0573]: expected type, found local variable `num`
  --> $DIR/let-binding-init-expr-as-ty.rs:2:27
   |
LL |     let foo: i32::from_be(num);
   |            --             ^^^ not a type
   |            |
   |            help: use `=` if you meant to assign

error: parenthesized type parameters may only be used with a `Fn` trait
  --> $DIR/let-binding-init-expr-as-ty.rs:2:19
   |
LL |     let foo: i32::from_be(num);
   |                   ^^^^^^^^^^^^
   |
   = note: `#[deny(parenthesized_params_in_types_and_modules)]` on by default
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #42238 <https://github.com/rust-lang/rust/issues/42238>

error[E0223]: ambiguous associated type
  --> $DIR/let-binding-init-expr-as-ty.rs:2:14
   |
LL |     let foo: i32::from_be(num);
   |              ^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `<i32 as Trait>::from_be`

Further improvements would get pretty hairy pretty quickly.

@hsivonen

This comment has been minimized.

Copy link
Contributor

hsivonen commented Oct 22, 2019

would the current output have been enough for you to quickly identify the issue?

Likely yes. Thank you!

@estebank

This comment has been minimized.

Copy link
Contributor Author

estebank commented Oct 25, 2019

@rust-highfive rust-highfive assigned Centril and unassigned zackmdavis Oct 25, 2019
@Centril

This comment has been minimized.

Copy link
Member

Centril commented Oct 26, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 26, 2019

📌 Commit 10ce36c has been approved by Centril

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 26, 2019

⌛️ Testing commit 10ce36c with merge cf96b4b...

bors added a commit that referenced this pull request Oct 26, 2019
Use heuristics to suggest assignment

When detecting a possible `=` -> `:` typo in a `let` binding, suggest
assigning instead of setting the type.

Partially address #57828.
@Centril

This comment has been minimized.

Copy link
Member

Centril commented Oct 26, 2019

@bors treeclosed=1000

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Oct 26, 2019

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.
2019-10-26T07:18:53.1399271Z do so (now or later) by using -b with the checkout command again. Example:
2019-10-26T07:18:53.1400079Z 
2019-10-26T07:18:53.1403308Z   git checkout -b <new-branch-name>
2019-10-26T07:18:53.1404331Z 
2019-10-26T07:18:53.1404938Z HEAD is now at cf96b4bad Auto merge of #65566 - estebank:let-expr-as-ty, r=Centril
2019-10-26T07:18:53.2013734Z ##[section]Starting: Collect CPU-usage statistics in the background
2019-10-26T07:18:53.2276470Z ==============================================================================
2019-10-26T07:18:53.2276592Z Task         : Bash
2019-10-26T07:18:53.2276677Z Description  : Run a Bash script on macOS, Linux, or Windows
---
2019-10-26T07:20:32.8004531Z Chocolatey installed 0/1 packages. 1 packages failed.
2019-10-26T07:20:32.8004965Z  See the log for details (C:\ProgramData\chocolatey\logs\chocolatey.log).
2019-10-26T07:20:32.8011060Z 
2019-10-26T07:20:32.8017389Z Failures
2019-10-26T07:20:32.8024735Z  - msys2 (exited 1) - msys2 not installed. An error occurred during installation:
2019-10-26T07:20:32.8025305Z  The remote server returned an error: (503) Server Unavailable. Service Unavailable
2019-10-26T07:20:33.3100079Z 
2019-10-26T07:20:33.3202189Z ##[error]Bash exited with code '1'.
2019-10-26T07:20:33.3437294Z ##[section]Starting: Upload CPU usage statistics
2019-10-26T07:20:33.3549928Z ==============================================================================
2019-10-26T07:20:33.3550044Z Task         : Bash
2019-10-26T07:20:33.3550116Z Description  : Run a Bash script on macOS, Linux, or Windows
---
2019-10-26T07:20:33.6908889Z ========================== Starting Command Output ===========================
2019-10-26T07:20:33.6914878Z [command]"C:\Program Files\Git\bin\bash.exe" --noprofile --norc /d/a/_temp/a38c3b31-4a84-465b-89b3-e2fc1ba236e7.sh
2019-10-26T07:20:33.7363987Z /d/a/_temp/a38c3b31-4a84-465b-89b3-e2fc1ba236e7.sh: line 1: aws: command not found
2019-10-26T07:20:33.7391556Z 
2019-10-26T07:20:33.7410693Z ##[error]Bash exited with code '127'.
2019-10-26T07:20:33.7487889Z ##[section]Starting: Checkout
2019-10-26T07:20:33.7639758Z ==============================================================================
2019-10-26T07:20:33.7639868Z Task         : Get sources
2019-10-26T07:20:33.7639973Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

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 @TimNN. (Feature Requests)

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 26, 2019

💔 Test failed - checks-azure

@JohnTitor

This comment has been minimized.

Copy link
Member

JohnTitor commented Oct 26, 2019

It seems chocolatey is now available, let's check if it works.
@bors treeclosed-

@JohnTitor

This comment has been minimized.

Copy link
Member

JohnTitor commented Oct 26, 2019

@bors retry

Centril added a commit to Centril/rust that referenced this pull request Oct 26, 2019
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.
@estebank

This comment has been minimized.

Copy link
Contributor Author

estebank commented Oct 26, 2019

Fallout from the suggestions change. I'll rebase and fix.

estebank added 5 commits Oct 18, 2019
When detecting a possible `=` -> `:` typo in a `let` binding, suggest
assigning instead of setting the type.
@estebank estebank force-pushed the estebank:let-expr-as-ty branch from 10ce36c to b579c5a Oct 26, 2019
@estebank

This comment has been minimized.

Copy link
Contributor Author

estebank commented Oct 26, 2019

@bors r=Centril

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 26, 2019

📌 Commit b579c5a has been approved by Centril

Centril added a commit to Centril/rust that referenced this pull request Oct 27, 2019
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.
bors added a commit that referenced this pull request Oct 27, 2019
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
@bors bors merged commit b579c5a into rust-lang:master Oct 27, 2019
4 checks passed
4 checks passed
pr Build #20191026.68 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-6.0) Linux x86_64-gnu-llvm-6.0 succeeded
Details
pr (LinuxTools) LinuxTools succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.