Skip to content

Display correct unused field suggestion for nested struct patterns #50327

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Apr 30, 2018

Conversation

varkor
Copy link
Member

@varkor varkor commented Apr 30, 2018

Extends #47922 by checking more sophisticated patterns (e.g. references, slices, etc.).
Before:

warning: unused variable: `bar`
  --> src/main.rs:37:21
   |
37 |         &Foo::Bar { bar } => true,
   |                     ^^^ help: consider using `_bar` instead
   |
   = note: #[warn(unused_variables)] on by default

After:

warning: unused variable: `bar`
  --> src/main.rs:37:21
   |
37 |         &Foo::Bar { bar } => true,
   |                     ^^^ help: try ignoring the field: `bar: _`
   |
   = note: #[warn(unused_variables)] on by default

Fixes #50303.

r? @estebank

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

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 30, 2018

📌 Commit 2eb8343 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 Apr 30, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Apr 30, 2018
…=estebank

Display correct unused field suggestion for nested struct patterns

Extends rust-lang#47922 by checking more sophisticated patterns (e.g. references, slices, etc.).
Before:
```
warning: unused variable: `bar`
  --> src/main.rs:37:21
   |
37 |         &Foo::Bar { bar } => true,
   |                     ^^^ help: consider using `_bar` instead
   |
   = note: #[warn(unused_variables)] on by default
```
After:
```
warning: unused variable: `bar`
  --> src/main.rs:37:21
   |
37 |         &Foo::Bar { bar } => true,
   |                     ^^^ help: try ignoring the field: `bar: _`
   |
   = note: #[warn(unused_variables)] on by default
```

Fixes rust-lang#50303.

r? @estebank
bors added a commit that referenced this pull request Apr 30, 2018
Rollup of 7 pull requests

Successful merges:

 - #50233 (Make `Vec::new` a `const fn`)
 - #50312 (Add more links in panic docs)
 - #50316 (Fix some broken links in docs.)
 - #50325 (Add a few more tests for proc macro feature gating)
 - #50327 (Display correct unused field suggestion for nested struct patterns)
 - #50330 (check that #[used] is used only on statics)
 - #50344 (Update Cargo to 2018-04-28 122fd5be5201913d42e219e132d6569493583bca)

Failed merges:
@bors bors merged commit 2eb8343 into rust-lang:master Apr 30, 2018
@varkor varkor deleted the match-unused-struct-field branch April 30, 2018 20:16
@b-jonas0
Copy link

b-jonas0 commented May 9, 2018

The previous suggestion might have been incorrect, but I don't like the new suggestion either, because ignoring the field can change semantics. Consider the following snippet.

fn main() {
    struct K;
    impl Drop for K { fn drop(&mut self) { println!("K dropped"); } }
    struct M {k: K}
    let m = Some(M{k: K});
    if let Some(M{k}) = m { }
    println!("hello");
}

The output is:

K dropped
hello

The warning I get is:

warning: unused variable: `k`
 --> src/main.rs:7:19
  |
7 |     if let Some(M{k}) = m { }
  |                   ^ help: try ignoring the field: `k: _`
  |
  = note: #[warn(unused_variables)] on by default

If I follow that suggestion, then the output changes to:

hello
K dropped

A correct suggestion would be something like:

warning: unused variable: `k`
 --> src/main.rs:7:19
  |
7 |     if let Some(M{k}) = m { }
  |                   ^ help: consider using `k: _k` instead
  |
  = note: #[warn(unused_variables)] on by default

zackmdavis added a commit to zackmdavis/rust that referenced this pull request May 18, 2018
In e4b1a79 (rust-lang#47922), we corrected erroneous suggestions for unused
shorthand field pattern bindings, suggesting `field: _` where the
previous suggestion of `_field` wouldn't even have compiled
(rust-lang#47390). Soon, it was revealed that this was insufficient (rust-lang#50303), and
the fix was extended to references, slices, &c. (rust-lang#50327) But even this
proved inadequate, as the erroneous suggestions were still being issued
for patterns in local (`let`) bindings (rust-lang#50804). Here, we yank the
shorthand-detection and variable/node registration code into a new
common function that can be called while visiting both match arms and
`let` bindings.

Resolves rust-lang#50804.
kennytm added a commit to kennytm/rust that referenced this pull request May 19, 2018
…ed_field_pattern_3_straight_to_video, r=estebank

in which the unused shorthand field pattern debacle/saga continues

In e4b1a79 (rust-lang#47922), we corrected erroneous suggestions for unused
shorthand field pattern bindings, suggesting `field: _` where the
previous suggestion of `_field` wouldn't even have compiled
(rust-lang#47390). Soon, it was revealed that this was insufficient (rust-lang#50303), and
the fix was extended to references, slices, &c. (rust-lang#50327) But even this
proved inadequate, as the erroneous suggestions were still being issued
for patterns in local (`let`) bindings (rust-lang#50804). Here, we yank the
shorthand-detection and variable/node registration code into a new
common function that can be called while visiting both match arms and
`let` bindings.

Resolves rust-lang#50804.

r? @estebank
# for free to join this conversation on GitHub. Already have an account? # 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.

5 participants