Skip to content

Point arg num mismatch errors back to their definition #38121

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

Merged
merged 1 commit into from
Dec 6, 2016

Conversation

sophiajt
Copy link
Contributor

@sophiajt sophiajt commented Dec 2, 2016

This PR updates the arg num errors (like E0061) to point back at the function definition where they were defined.

Before:

error[E0061]: this function takes 2 parameters but 1 parameter was supplied
  --> E0061.rs:18:7
   |
18 |     f(0);
   |       ^
   |
   = note: the following parameter types were expected:
   = note: u16, &str

Now:

error[E0061]: this function takes 2 parameters but 1 parameter was supplied
  --> E0061.rs:18:7
   |
11 | fn f(a: u16, b: &str) {}
   | ------------------------ defined here
...
18 |     f(0);
   |       ^ expected 2 parameters

This is an incremental improvement. We probably want to underline only the function name and also have support for functions defined in crates outside of the current crate.

r? @nikomatsakis

@nikomatsakis
Copy link
Contributor

This is an incremental improvement. We probably want to underline only the function name and also have support for functions defined in crates outside of the current crate.

Hmm. Perhaps. I like seeing the full span here because it shows me the arguments inline.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 2, 2016

📌 Commit c735d7f has been approved by nikomatsakis

expected_count,
if expected_count == 1 {""} else {"s"}));
if let Some(def_s) = def_span {
err.span_label(def_s, &format!("defined here"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to keep lines 2543-2549 in the else for cases where the method was defined in a separate crate and we don't have access to the definition span?

@bors
Copy link
Collaborator

bors commented Dec 6, 2016

⌛ Testing commit c735d7f with merge 0999124...

bors added a commit that referenced this pull request Dec 6, 2016
Point arg num mismatch errors back to their definition

This PR updates the arg num errors (like E0061) to point back at the function definition where they were defined.

Before:

```
error[E0061]: this function takes 2 parameters but 1 parameter was supplied
  --> E0061.rs:18:7
   |
18 |     f(0);
   |       ^
   |
   = note: the following parameter types were expected:
   = note: u16, &str
```

Now:

```
error[E0061]: this function takes 2 parameters but 1 parameter was supplied
  --> E0061.rs:18:7
   |
11 | fn f(a: u16, b: &str) {}
   | ------------------------ defined here
...
18 |     f(0);
   |       ^ expected 2 parameters
```

This is an incremental improvement.  We probably want to underline only the function name and also have support for functions defined in crates outside of the current crate.

r? @nikomatsakis
@bors bors merged commit c735d7f into rust-lang:master Dec 6, 2016
@sophiajt
Copy link
Contributor Author

sophiajt commented Dec 6, 2016

@estebank - possibly. we could do an additional PR and take a look (since this landed before we could discuss)

@sophiajt sophiajt deleted the better_e0061 branch December 6, 2016 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants