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

Lint for generalizeable captures #10530

Closed
est31 opened this issue Mar 22, 2023 · 5 comments
Closed

Lint for generalizeable captures #10530

est31 opened this issue Mar 22, 2023 · 5 comments
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy

Comments

@est31
Copy link
Member

est31 commented Mar 22, 2023

What it does

Take this code:

fn foo(v: Option<char>) -> u32 {
    match v {
        Some(w @ 'a' | w @ 'c' | w @ 'd') => w as u32,
        Some(_) => 1,
        None => 0,
    }
}

The binding w of the first arm's pattern is repeated three times. One can simplify the pattern as follows:

fn foo(v: Option<char>) -> u32 {
    match v {
        Some(w @ ('a' | 'c' | 'd')) => w as u32,
        Some(_) => 1,
        None => 0,
    }
}

Lint Name

repetitive_capture

Category

style

Advantage

This saves repetition of the binding (less chars) and makes the code more maintainable as renames only affect one site instead of multiple

Drawbacks

It adds one layer of ()s, but this should be bearable

Example

provided above

@est31 est31 added the A-lint Area: New lints label Mar 22, 2023
@est31
Copy link
Member Author

est31 commented Mar 22, 2023

See also rust-lang/rust#109489 where I found instances of this in rustc.

@blyxyas
Copy link
Member

blyxyas commented Mar 26, 2023

@rustbot label: +good-first-issue

@rustbot rustbot added the good-first-issue These issues are a good way to get started with Clippy label Mar 26, 2023
@Almugra
Copy link

Almugra commented Mar 28, 2023

@rustbot claim

@Almugra Almugra removed their assignment Mar 31, 2023
@Almugra
Copy link

Almugra commented Mar 31, 2023

I think unnested_or_patterns should already handle this, or is it different because unnested_or_patterns is pedantic? If that is the case, I would like to claim it again.

@est31
Copy link
Member Author

est31 commented Mar 31, 2023

Hmm good point about unnested_or_patterns. I have tried enabling all the clippy lints before I filed the issue but it didn't show up, not sure what went wrong back then.

error: unnested or-patterns
 --> src/main.rs:5:9
  |
5 |         Some(w @ 'a' | w @ 'c' | w @ 'd') => w as u32,
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnested_or_patterns
note: the lint level is defined here
 --> src/main.rs:1:8
  |
1 | #[deny(clippy::pedantic)]
  |        ^^^^^^^^^^^^^^^^
  = note: `#[deny(clippy::unnested_or_patterns)]` implied by `#[deny(clippy::pedantic)]`
help: nest the patterns
  |
5 |         Some(w @ ('a' | 'c' | 'd')) => w as u32,
  |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~

Closing as it's already implemented in clippy.

@est31 est31 closed this as completed Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

No branches or pull requests

4 participants