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

E0027 suggestion introduces refactoring-hazard #132008

Closed
aticu opened this issue Oct 21, 2024 · 3 comments · Fixed by #132025
Closed

E0027 suggestion introduces refactoring-hazard #132008

aticu opened this issue Oct 21, 2024 · 3 comments · Fixed by #132025
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@aticu
Copy link
Contributor

aticu commented Oct 21, 2024

Code

#[derive(Default)]
struct X {
    field1: (),
    field2: (),
}

fn main() {
    let x = Default::default();
    let X { field1 } = x;
}

Current output

error[E0027]: pattern does not mention field `field2`
 --> src/main.rs:9:9
  |
9 |     let X { field1 } = x;
  |         ^^^^^^^^^^^^ missing field `field2`
  |
help: include the missing field in the pattern
  |
9 |     let X { field1, field2 } = x;
  |                   ~~~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
  |
9 |     let X { field1, .. } = x;
  |                   ~~~~~~

Desired output

error[E0027]: pattern does not mention field `field2`
 --> src/main.rs:9:9
  |
9 |     let X { field1 } = x;
  |         ^^^^^^^^^^^^ missing field `field2`
  |
help: include the missing field in the pattern
  |
9 |     let X { field1, field2 } = x;
  |                   ~~~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
  |
9 |     let X { field1, field2: _ } = x;
  |                   ~~~~~~~~~~~~~
help: if you only care about the specified fields, you can explicitly ignore all other fields
  |
9 |     let X { field1, .. } = x;
  |                   ~~~~~~

Rationale and extra context

I often find myself trying to write code in such a way that future changes to a struct, require me to look at all uses, since they may not accurately reflect what I want after the refactoring. The current suggestion actively goes against that, ignoring future field-additions.
Ignoring fields is of course still a valid option in some cases, so the new suggestion would give both variants, allowing the user to choose which is more appropriate in this case (and making them aware of the trade-offs).

At the very least the message should reflect the trade-offs made when writing .. (also ignoring all fields added in the future).

Other cases

The error message for three fields:

error[E0027]: pattern does not mention fields `field2`, `field3`
  --> src/main.rs:10:9
   |
10 |     let X { field1 } = x;
   |         ^^^^^^^^^^^^ missing fields `field2`, `field3`
   |
help: include the missing fields in the pattern
   |
10 |     let X { field1, field2, field3 } = x;
   |                   ~~~~~~~~~~~~~~~~~~
help: if you don't care about these missing fields, you can explicitly ignore them
   |
10 |     let X { field1, .. } = x;
   |                   ~~~~~~

It is unclear to me whether for more than one field the error message should just scale to more fields (field2: _, field3: _) or just suggest the .. syntax and warn that it might result in unwanted ignored fields if more fields are added.

Rust Version

rustc 1.84.0-nightly (662180b34 2024-10-20)
binary: rustc
commit-hash: 662180b34d95f72d05b7c467b0baf4d23d36b1e1
commit-date: 2024-10-20
host: x86_64-unknown-linux-gnu
release: 1.84.0-nightly
LLVM version: 19.1.1

Anything else?

No response

@aticu aticu added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 21, 2024
@fmease fmease added A-resolve Area: Name resolution D-papercut Diagnostics: An error or lint that needs small tweaks. labels Oct 21, 2024
@Noratrieb Noratrieb added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. and removed A-resolve Area: Name resolution D-papercut Diagnostics: An error or lint that needs small tweaks. labels Oct 21, 2024
@duncpro
Copy link
Contributor

duncpro commented Oct 21, 2024

@rustbot claim

@duncpro
Copy link
Contributor

duncpro commented Oct 22, 2024

@aticu How does this sound to you?

error[E0027]: pattern does not mention field `name`
  --> /Users/dp/Desktop/rust/tests/ui/error-codes/E0027.rs:11:9
   |
LL |         Dog { age: x } => {}
   |         ^^^^^^^^^^^^^^ missing field `name`
   |
help: include the missing field in the pattern
   |
LL |         Dog { age: x, name } => {} 
   |                     ~~~~~~~~
help: if the value is not relevant, discard it
   |
LL |         Dog { age: x, name: _ } => {}
   |                     ~~~~~~~~~~~
help: or allow missing fields here
   |
LL |         Dog { age: x, .. } => {}
   |                     ~~~~~~

@aticu
Copy link
Contributor Author

aticu commented Oct 22, 2024

Looks good to me, thanks!

tgross35 added a commit to tgross35/rust that referenced this issue Nov 5, 2024
@bors bors closed this as completed in c17cf1d Nov 5, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 5, 2024
Rollup merge of rust-lang#132025 - duncpro:E0027, r=compiler-errors

fix suggestion for diagnostic error E0027

Closes rust-lang#132008
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 E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants