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

Suggest correct syntax when writing type arg instead of assoc type #55808

Merged
merged 6 commits into from
Nov 23, 2018

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Nov 9, 2018

  • When confusing an associated type with a type argument, suggest the appropriate syntax. Given Iterator<isize>, suggest Iterator<Item = isize>.
  • When encountering multiple missing associated types, emit only one diagnostic.
  • Point at associated type def span for context.
  • Point at each extra type argument.

Follow up to #48288, fix #20977.

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 9, 2018
@estebank
Copy link
Contributor Author

estebank commented Nov 9, 2018

cc @nikomatsakis

@estebank
Copy link
Contributor Author

estebank commented Nov 9, 2018

The output can still be improved, IMO

error[E0107]: wrong number of type arguments: expected 0, found 1
 --> file2.rs:1:34
  |
1 | pub struct Foo { i: Box<Iterator<isize>> }
  |                                  ^^^^^ unexpected type argument

error[E0191]: the value of the associated type `Item` (from the trait `std::iter::Iterator`) must be specified
   --> file2.rs:1:25
    |
1   | pub struct Foo { i: Box<Iterator<isize>> }
    |                         ^^^^^^^^^^^^^^^ missing associated type `Item` value
    |
   ::: /Users/ekuber/personal/rust/src/libcore/iter/iterator.rs:104:5
    |
104 |     type Item;
    |     ---------- `Item` defined here
help: if you meant to assign the missing associated type, use the name
    |
1   | pub struct Foo { i: Box<Iterator<Item = isize>> }
    |                                  ^^^^^^^^^^^^

error: aborting due to 2 previous errors

The span pointing at the associated type definition span seems superfluous (at least without the corresponding trait's def span) and the wording could do with some copy editing.

@rust-highfive

This comment has been minimized.

@durka
Copy link
Contributor

durka commented Nov 9, 2018

"type value" is not good IMO. How about

associated type std::iter::Iterator::Item must be specified

and

to specify Item, write

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Nov 9, 2018

📌 Commit 5f569e2b43b455b7c4bdf6469ecb05829dba1461 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 9, 2018
@bors
Copy link
Contributor

bors commented Nov 11, 2018

☔ The latest upstream changes (presumably #55859) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 11, 2018
@estebank
Copy link
Contributor Author

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Nov 12, 2018

📌 Commit 5cbd6e3df7cbd2c613af6d8dc0faf0663bbe2e36 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 12, 2018
@bors
Copy link
Contributor

bors commented Nov 18, 2018

⌛ Testing commit 5cbd6e3df7cbd2c613af6d8dc0faf0663bbe2e36 with merge 42741ddb5182fac9cb0afb1f8425ae963b0b9db4...

@bors
Copy link
Contributor

bors commented Nov 18, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 18, 2018
@rust-highfive

This comment has been minimized.

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 18, 2018
When confusing an associated type with a type argument, suggest the
appropriate syntax.

Given `Iterator<isize>`, suggest `Iterator<Item = isize>`.
This is a somewhat arbitrary restriction in order to be consistent in the
output of the tests regardless of target platform.
@estebank
Copy link
Contributor Author

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Nov 22, 2018

📌 Commit 510f836 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 22, 2018
@bors
Copy link
Contributor

bors commented Nov 23, 2018

⌛ Testing commit 510f836 with merge aecbcd1...

bors added a commit that referenced this pull request Nov 23, 2018
Suggest correct syntax when writing type arg instead of assoc type

- When confusing an associated type with a type argument, suggest the appropriate syntax. Given `Iterator<isize>`, suggest `Iterator<Item = isize>`.
- When encountering multiple missing associated types, emit only one diagnostic.
- Point at associated type def span for context.
- Point at each extra type argument.

Follow up to #48288, fix #20977.
@bors
Copy link
Contributor

bors commented Nov 23, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: petrochenkov
Pushing aecbcd1 to master...

@bors bors merged commit 510f836 into rust-lang:master Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compiler error could suggest using an associated type for e.g. Iterator<T>
6 participants