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

if_same_then_else should not trigger when one branch is "if let <pattern> = thing" #7834

Closed
DanielT opened this issue Oct 17, 2021 · 3 comments
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@DanielT
Copy link

DanielT commented Oct 17, 2021

Lint name: if_same_then_else

I've simplified my code some, but it basically looks like this

if let Some(MyEnum::Variant(_)) = value {
    cleanup_some_state();
} else if other_condition {
    cleanup_some_state();
} else {
    // do more work
}

Clippy generates the if_same_then_else diagnostic for this code.
As a matter of fact, I would love to unify the two identical branches, but as far as I know, I cannot add any other conditions to the "if let".

There should be no message from clippy here, since writing the code the way clippy suggests is not possible

Meta

Rust version (rustc -Vv):

rustc 1.57.0-nightly (a8f2463c6 2021-10-09)
binary: rustc
commit-hash: a8f2463c68a6532d74a13ec402ec5b513e4e2726
commit-date: 2021-10-09
host: x86_64-pc-windows-msvc
release: 1.57.0-nightly
LLVM version: 13.0.0
@DanielT DanielT added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Oct 17, 2021
@PatchMixolydic
Copy link
Contributor

PatchMixolydic commented Oct 17, 2021

It's possible to rewrite this code like this:

if matches!(value, Some(MyEnum::Variant(_))) || other_condition {
    cleanup_some_state();
} else {
    // do more work
}

It might be possible to emit a suggestion to this effect. Note that this will work even if a new binding is introduced since the binding couldn't actually be used aside from it being dropped (otherwise, the two blocks would be different, so this lint wouldn't trip in the first place). I believe drop order should still be the same so long as any bindings created in the if let arm remain intact.

@DanielT
Copy link
Author

DanielT commented Oct 17, 2021

I wasn't even aware that matches! existed. It definitely solves my problem.
Very interesting, thanks!

@camsteffen
Copy link
Contributor

Duplicate of #7579

@camsteffen camsteffen marked this as a duplicate of #7579 Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

No branches or pull requests

3 participants