Skip to content

Conversation

ada4a
Copy link
Contributor

@ada4a ada4a commented Sep 3, 2025

Fixes #13287

changelog: [collapsible_match]: exclude binding modes from struct field pattern suggestions

@rustbot
Copy link
Collaborator

rustbot commented Sep 3, 2025

r? @dswij

rustbot has assigned @dswij.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 3, 2025
@ada4a ada4a force-pushed the collapsible_match branch from babec70 to fba062b Compare September 3, 2025 23:53
@ada4a
Copy link
Contributor Author

ada4a commented Sep 4, 2025

I could additionally mention that the user might need to push ref( mut) to somewhere inside the replaced binding, e.g., in a case like:

  fn struct_field_pat_with_binding_mode(err: Option<Error>) {
      if let Some(Error { ref token, .. }) = err {
          if let Some(t) = token {
              println!("token used as a ref");
          }
      }
  }

the required change will be

fn struct_field_pat_with_binding_mode(err: Option<Error>) {
    if let Some(Error { token: Some(ref t), .. }) = err {
        println!("token used as a ref");
    }
}

But firstly, this is actually not required in the example provided in the original issue -- when in this situation:

fn struct_field_pat_with_binding_mode(err: Option<Error>) {
    if let Some(Error { ref token, .. }) = err {
        if let Some(Token::Name) = token {
            //~^ collapsible_match
            println!("token used as a ref");
        }
    }
}

we inline token:

fn struct_field_pat_with_binding_mode(err: Option<Error>) {
    if let Some(Error { token: Some(Token::Name), .. }) = err {
        println!("token used as a ref");
    }
}

we not only don't need to put a ref anywhere, but AFAICT wherever we put we get a compile error (and that would also be the case if err was Option<&Error>).

I guess we could employ smarter heuristics to only add the "you'll need to push the ref( mut)" note if we see that the pattern to be replaced with doesn't contain any bindings.

But I ultimately decided not to bother, given that we already don't do any of that in an existing test case -- this case:

match Some(&[1]) {
Some(ref s) => match s {
//~^ collapsible_match
[n] => foo(n),
_ => (),
},
_ => (),
}
}

gets a message that doesn't acknowledge the ref at all:
help: the outer pattern can be modified to include the inner pattern
--> tests/ui/collapsible_match2.rs:64:14
|
LL | Some(ref s) => match s {
| ^^^^^ replace this binding
LL |
LL | [n] => foo(n),
| ^^^ with this pattern

Copy link
Member

@dswij dswij left a comment

Choose a reason for hiding this comment

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

@dswij dswij added this pull request to the merge queue Oct 7, 2025
Merged via the queue into rust-lang:master with commit ea54123 Oct 7, 2025
11 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 7, 2025
@ada4a ada4a deleted the collapsible_match branch October 7, 2025 16:03
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.

collapsible_match might produce an incorrect suggestion for nested fields, when ref / mut / box modifiers are used

3 participants