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.

Sign up for GitHub

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

Jump to bottom

Error suggestion for E0027 (pattern missing field) contains erroneous comma #78511

Closed
mk12 opened this issue Oct 29, 2020 · 1 comment · Fixed by #81103
Closed

Error suggestion for E0027 (pattern missing field) contains erroneous comma #78511

mk12 opened this issue Oct 29, 2020 · 1 comment · Fixed by #81103
Assignees
@zackmdavis
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. 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

@mk12
Copy link
Contributor

mk12 commented Oct 29, 2020

I tried this code:

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=4f2737c4c44adb53103b875e0b7044cd

struct Foo {
    x: i32,
    y: i32,
}

fn test(Foo {
    x,
}: Foo) {}

I expected to see this happen:

A suggestion to change x, to x, ..

Instead, this happened:

A suggestion to change x, to x, .., (note the extra comma!)

With the comma at the end, I get a parser error and a suggestion to remove the comma.

Meta

Nightly on playground: 1.49.0-nightly (2020-10-27 07e968b)
@mk12 mk12 added the C-bug Category: This is a bug. label Oct 29, 2020
@njasm
Copy link
Contributor

njasm commented Oct 29, 2020

@rustbot modify labels: +A-diagnostics

@rustbot rustbot added the A-diagnostics Area: Messages for errors, warnings, and lints label Oct 29, 2020
@jyn514 jyn514 added A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 29, 2020
@JohnTitor JohnTitor added the D-papercut Diagnostics: An error or lint that needs small tweaks. label Oct 29, 2020
@zackmdavis zackmdavis self-assigned this Jan 16, 2021
@zackmdavis zackmdavis mentioned this issue Jan 17, 2021
Merged
m-ou-se added a commit to m-ou-se/rust that referenced this issue Jan 18, 2021
@m-ou-se
Rollup merge of rust-lang#81103 - zackmdavis:comma_trail, r=davidtwco
a246cdb 
don't suggest erroneous trailing comma after `..`

In rust-lang#76612, suggestions were added for missing fields in patterns. However, the suggestions are being inserted just at the end
of the last field in the pattern—before any trailing comma after the last field. This resulted in the "if you don't care about missing fields" suggestion to recommend code with a trailing comma after the field ellipsis (`..,`), which is actually not legal ("`..` must be at the end and cannot have a trailing comma")!

Incidentally, the doc-comment on `error_unmentioned_fields` was using `you_cant_use_this_field` as an example field name (presumably copy-paste inherited from the description of Issue rust-lang#76077), but the present author found this confusing, because unmentioned fields aren't necessarily unusable.

The suggested code in the diff this commit introduces to `destructuring-assignment/struct_destructure_fail.stderr` doesn't work, but it didn't work beforehand, either (because of the "found reserved identifier `_`" thing), so you can't really call it a regression; it could be fixed in a separate PR.

Resolves rust-lang#78511.

r? `@davidtwco` or `@estebank`
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 19, 2021
@bors
Auto merge of rust-lang#81103 - zackmdavis:comma_trail, r=davidtwco
94e6ea9 
don't suggest erroneous trailing comma after `..`

In rust-lang#76612, suggestions were added for missing fields in patterns. However, the suggestions are being inserted just at the end
of the last field in the pattern—before any trailing comma after the last field. This resulted in the "if you don't care about missing fields" suggestion to recommend code with a trailing comma after the field ellipsis (`..,`), which is actually not legal ("`..` must be at the end and cannot have a trailing comma")!

Incidentally, the doc-comment on `error_unmentioned_fields` was using `you_cant_use_this_field` as an example field name (presumably copy-paste inherited from the description of Issue rust-lang#76077), but the present author found this confusing, because unmentioned fields aren't necessarily unusable.

The suggested code in the diff this commit introduces to `destructuring-assignment/struct_destructure_fail.stderr` doesn't work, but it didn't work beforehand, either (because of the "found reserved identifier `_`" thing), so you can't really call it a regression; it could be fixed in a separate PR.

Resolves rust-lang#78511.

r? `@davidtwco` or `@estebank`
@bors bors closed this as completed in 14eb94f Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zackmdavis zackmdavis

Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. 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
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

don't suggest erroneous trailing comma after .. zackmdavis/rust
6 participants
@mk12 @zackmdavis @njasm @jyn514 @JohnTitor @rustbot