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

Fixed: Multiple errors on single typo in match pattern #55156

Merged
merged 1 commit into from
Oct 20, 2018

Conversation

PramodBisht
Copy link
Contributor

Here we have fixed the case where we were throwing two diagnostic messages E0026 and E0027 for same case.

Example

error[E0026]: variant `A::A` does not have a field named `fob`
  --> src/test/ui/issue-52717.rs:20:12
   |
20 |     A::A { fob } => { println!("{}", fob); }
   |            ^^^ variant `A::A` does not have this field

error[E0027]: pattern does not mention field `foo`
  --> src/test/ui/issue-52717.rs:20:5
   |
20 |     A::A { fob } => { println!("{}", fob); }
   |     ^^^^^^^^^^^^ missing field `foo`

error: aborting due to 2 previous errors

Here above we can see that both E0026 and E0027 are depicting
same thing.

So, to fix this issue, we are simply checking if for last element of inexistent_fields is there any value lies in unmentioned_fields using levenshtein algorithm, if it does then for that case we are simply deleting element from unmentioned_fields. More or less, now instead of showing separate message in E0027 we are giving extra hint on E0026

r? @estebank

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

@estebank estebank left a comment

Choose a reason for hiding this comment

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

Couple of nitpicks, but lgtm

| ^^^^^
| |
| variant `MyOption::MySome` does not have this field
| help: did you mean: `0`
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unfortunate, but not a regression as we were already providing this suggestion/error. Ideally it should actually suggest MyOption::MySome(42)... Not a blocker for this PR, but lets open a follow up ticket.

src/librustc_typeck/check/_match.rs Outdated Show resolved Hide resolved
src/librustc_typeck/check/_match.rs Outdated Show resolved Hide resolved
src/librustc_typeck/check/_match.rs Outdated Show resolved Hide resolved
src/librustc_typeck/check/_match.rs Outdated Show resolved Hide resolved
Symbol::intern(field.to_string().as_str()))
.collect::<Vec<Symbol>>();
let suggested_name = find_best_match_for_name(input.iter(),
&ident.name.as_str(), None);
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix indentation above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

which IDE or tools I should use to detect this issues?

Copy link
Contributor

Choose a reason for hiding this comment

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

Generally, you can make your changes, run rustfmt on the file and only take the changes for the code you changed :-/

That workflow is annoying enough that I just memorized the most relevant rules. Basically, if you have multiple lines

like(this,
     right,
     here);

you want them to be aligned to the (, which is why there's a preference to

this_other(
    indentation,
    of,
    multiple,
    args,
);

as it is independent of function name length.

@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

Here we have fixed the case where we were throwing two diagnostic
messages `E0026` and `E0027` for same case like this

Example
error[E0026]: variant `A::A` does not have a field named `fob`
  --> src/test/ui/issue-52717.rs:20:12
   |
20 |     A::A { fob } => { println!("{}", fob); }
   |            ^^^ variant `A::A` does not have this field

error[E0027]: pattern does not mention field `foo`
  --> src/test/ui/issue-52717.rs:20:5
   |
20 |     A::A { fob } => { println!("{}", fob); }
   |     ^^^^^^^^^^^^ missing field `foo`

error: aborting due to 2 previous errors

Here above we can see that both `E0026` and `E0027` are depicting
same thing.

So, to fix this issue, we are simply checking element of
`inexistent_fields` is there any value lies in
`unmentioned_fields` using Levenshtein algorithm, if does
then for that case we are simply deleting element from
`unmentioned_fields`. More or less now instead of showing
separate message in `E0027` we are giving extra hint on `E0026`

Address: rust-lang#52717
@PramodBisht
Copy link
Contributor Author

r? @estebank again

fn main() {
let x = A::A { foo: 3 };
match x {
A::A { fob } => { println!("{}", fob); }
Copy link
Contributor

@estebank estebank Oct 19, 2018

Choose a reason for hiding this comment

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

There's another case that we're not handling (but it's fine):

    A::A { fob } => { println!("{}", foo); }

In the present case after applying the suggestion, the code will complain about the printed fob, while in the quoted case we'll have two complains, one about fob and another about the printed foo.

It'll be interesting coming up with a solution for this as a follow up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Working toward that :)

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 19, 2018

📌 Commit 978dc3d has been approved by estebank

@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 Oct 19, 2018
bors added a commit that referenced this pull request Oct 20, 2018
Fixed: Multiple errors on single typo in match pattern

Here we have fixed the case where we were throwing two diagnostic messages `E0026` and `E0027` for same case.

Example
```
error[E0026]: variant `A::A` does not have a field named `fob`
  --> src/test/ui/issue-52717.rs:20:12
   |
20 |     A::A { fob } => { println!("{}", fob); }
   |            ^^^ variant `A::A` does not have this field

error[E0027]: pattern does not mention field `foo`
  --> src/test/ui/issue-52717.rs:20:5
   |
20 |     A::A { fob } => { println!("{}", fob); }
   |     ^^^^^^^^^^^^ missing field `foo`

error: aborting due to 2 previous errors
```

Here above we can see that both `E0026` and `E0027` are depicting
same thing.

So, to fix this issue, we are simply checking if for last element of `inexistent_fields` is there any value lies in `unmentioned_fields` using levenshtein algorithm, if it does then for that case we are simply deleting element from `unmentioned_fields`. More or less, now instead of showing separate message in `E0027` we are giving extra hint on `E0026`

r? @estebank
@bors
Copy link
Contributor

bors commented Oct 20, 2018

⌛ Testing commit 978dc3d with merge 155510e...

@bors
Copy link
Contributor

bors commented Oct 20, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: estebank
Pushing 155510e to master...

bors added a commit that referenced this pull request Oct 20, 2018
Rollup of 5 pull requests

Successful merges:

 - #55156 (Fixed: Multiple errors on single typo in match pattern)
 - #55189 (update books for the next release)
 - #55193 (make asm diagnostic instruction optional)
 - #55203 (Write an initial version of the `program_clauses` callback)
 - #55213 (ignore target folders)

Failed merges:

r? @ghost
@bors bors merged commit 978dc3d into rust-lang:master Oct 20, 2018
@PramodBisht PramodBisht deleted the issue/52717 branch October 21, 2018 12:13
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.

None yet

4 participants