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

E0107: "expected 1 type argument" can be read in two opposite ways: as "this needs to *have* 1 type argument" and "this needs to *be* 1 type argument" #66228

Closed
sietse opened this issue Nov 8, 2019 · 5 comments
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-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@sietse
Copy link

sietse commented Nov 8, 2019

Summary

When you have nested type arguments, underlining an inner type argument and saying 'expected 1 type argument' does not obviously mean "expected [this to have] 1 type arg (but it hasn't got any)": it can also be read as "expected [this to be] 1 type arg (but it isn't one)".

Minimal complete example

Code that gives a confusing error message:

use std::sync::Arc;
use std::thread::JoinHandle;

struct Test {
    // error on this line
    handle1: Arc<JoinHandle>,
}

fn main() {
}

This results in the following error message:

error[E0107]: wrong number of type arguments: expected 1, found 0
 --> error-wrong-number-of-type-args.rs:5:17
  |
6 |     handle1: Arc<JoinHandle>
  |                  ^^^^^^^^^^ expected 1 type argument

Expected and observed interpretation by the user

Expected interpretation: rustc wanted me to interpret underlining + "expected 1 type argument" as "This should have 1 type argument". In other words, this was the desired train of thought:

JoinHandle should have 1 type argument, rustc is pointing out to you which expression (JoinHandle plus its 0 or more type arguments) needs extra type arguments.

Observed interpretation: I interpreted underlining + "expected 1 type argument" + as "There should be 1 type argument here".** That led to the following train of thought:

Arc should have 1 type argument, and rustc is pointing out to me where the type argument should go. So JoinHandle is not a type argument? But why?! 😕 I thought JoinHandle was a type argument... [I then spent 30 minutes going through lessons and docs on type arguments, Arc, and JoinHandle, looking for clues. I only realised the answer when I opened the provided solution code to compare.]

Some notes on the user persona: the user (me, in this case) was somebody with about 10 years experience programming R and Python; they had read The Rust Programming Language; they were working on lesson 7 of Michael Snoyman's Rust Crash Course when they encountered this error message; they lurked sometimes on the /r/rust subreddit; and they had no experience of Rust beyond that.

Not as problematic when user has passed at least 1 type arg

This particular form of the E0107 error message is printed whenever provided type args < required type args. But it's less ambiguous when the user has passed at least one type arg, because then the entire expression (the type and its too-few args) gets underlined:

 --> error-wrong-number-of-type-args.rs:7:18
  |
6 |     handle2: Arc<Result<i32>>
  |                  ^^^^^^^^^^^ expected 2 type arguments

Possible fixes

Potential fix 1: change error phrasing to "expected this to have 1 type argument"

This fix is (a) an improvement on the current message, and (b) easy to implement -- a one-line change in the code. The message takes a (little!) bit more work to parse, when you break it down: knowing what 'this' refers to relies on reading the underlining and mentally discarding the type args.

  |
6 |     handle1: Arc<JoinHandle>
  |                  ^^^^^^^^^^ expected this to have 1 type argument
  |
6 |     handle2: Arc<Result<i32>>
  |                  ^^^^^^^^^^^ expected this to have 2 type arguments

Potential fix 2: mention the type's name in the error message

I like this fix a lot better, because it makes the error message self-contained and unambiguous while keeping it short. But getting the type name into the string requires more intimate knowledge of the error-reporting code, so you need a bit more codebase knowledge to fix it.

  |
6 |     handle1: Arc<JoinHandle>
  |                  ^^^^^^^^^^ expected JoinHandle to have 1 type argument
  |
6 |     handle2: Arc<Result<i32>>
  |                  ^^^^^^^^^^^ expected Result to have 2 type arguments

Who could do the work

The relevant code is in astconv.rs#L389-L397
If we choose fix 1, I could create a pull request for that -- it's a one-line fix.

For fix 2, I'd be willing to help out, but at the moment I'm not sure how to obtain the span of code that refers to only to the type that didn't get enough parameters. For something this unimportantant, the maintainer might get a better return on investment if they did it themselves, or opted for fix 1.

Meta

rustc --version --verbose:

rustc 1.40.0-nightly (1423bec54 2019-11-05)
binary: rustc
commit-hash: 1423bec54cf2db283b614e527cfd602b481485d1
commit-date: 2019-11-05
host: x86_64-unknown-linux-gnu
release: 1.40.0-nightly
LLVM version: 9.0

This issue has been assigned to @Patryk27 via this comment.

@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 8, 2019
@estebank
Copy link
Contributor

estebank commented Nov 8, 2019

I believe we should leverage structured suggestions, making the output something along the lines of:

  |
6 |     handle1: Arc<JoinHandle>
  |                  ^^^^^^^^^^- found no type arguments set for `JoinHandle`
  |                  |
  |                  expected this to have 1 type argument
  |                  help: set all the type arguments, with `T` replaced with the desired type: `JoinHandle<T>`
  |
6 |     handle2: Arc<Result<i32>>
  |                  ^^^^^^^---^
  |                  |      |
  |                  |      found 1 type argument
  |                  expected this to have 2 type arguments
  |                  help: set all the type arguments, with `E` replaced with the desired type: `Result<i32, E>`

Using structured suggestions has the benefit of 1) showing the corrected code with placeholders, showing the user how the code should look like and 2) allowing tools to provide one click actions to apply the suggestion.

I'm willing to mentor you in making this change if you're interested.

@jonas-schievink jonas-schievink added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Nov 9, 2019
@sietse
Copy link
Author

sietse commented Nov 10, 2019

Hi estebank,

Thank you for looking at this issue.

I'm willing to mentor you in making this change if you're interested.

I would enjoy that very much. I haven't been mentored on a rust issue before, so I'm a bit

I am in the latter stages of moving house, so my mode of collaboration would necessarily be quite asynchronous: reasonable periods of waiting until I can around to working/replying, punctuated by my coming back with questions / progress updates / 'I haven't abandoned this' updates. Chat or skype sessions, say, might be harder to organise (also because I'm in the CET/UTC+1 timezone); but we can certainly try that, it could be helpful when making an infodump at the start. Does this mesh with how you are able to work?

To get us started: my guess at a roadmap for solving this problem looks a bit like this. Does that seem reasonable to you?

  1. Identify the part of the code where I'll need to make the change.
  2. Make a list of the data the structured suggestion will need to refer to -- probably spans for the Type, its arguments, etc. Should be doable.
  3. There's multiple things to figure out next, in any order.
    a. Find out how to access or create the data (spans) the structured suggestion will need. This is the most important bit, and the bit I couldn't figure out from the source code so far.
    b. Find out how to use structured suggestsions: read the docs if any, or the proposal, and almost certainly read pull requests that introduce structured suggestions.
    c. Setup a feedback loop for experimenting with the code -- probably this will involve (a) compiling the code, and (b) running specific unit tests against it. This'll probably involve reading&following the rustc guide.
  4. Once the items under step 3 are in place, I'm probably ready to try to make the change, and then go into the pull request mill.

@varkor
Copy link
Member

varkor commented Jan 10, 2020

cc @estebank on providing mentoring instructions.

@Patryk27
Copy link
Contributor

Patryk27 commented Jun 6, 2020

@rustbot claim

I'm handling this as a part of #71924

@rustbot rustbot self-assigned this Jun 6, 2020
@sietse
Copy link
Author

sietse commented Jun 8, 2020

@Patryk27 I appreciate that a lot.

@bors bors closed this as completed in a62a760 Jan 13, 2021
flip1995 pushed a commit to flip1995/rust that referenced this issue Jan 15, 2021
Rework diagnostics for wrong number of generic args (fixes rust-lang#66228 and rust-lang#71924)

This PR reworks the `wrong number of {} arguments` message, so that it provides more details and contextual hints.
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-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants