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

False positive in if_same_then_else lint #7383

Closed
Shnatsel opened this issue Jun 20, 2021 · 8 comments
Closed

False positive in if_same_then_else lint #7383

Shnatsel opened this issue Jun 20, 2021 · 8 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

@Shnatsel
Copy link
Member

Lint name: if_same_then_else

I tried this code:

fn main() {
    let a = 0;
    let b = 5;
    let some_check: bool = if a == 0 && b == 0 {
        true
    } else if a < b {
        true
    } else {
        match (a, b) {
            (16, 0) => true,
            _ => false
        }
    };
}

(this is a minimal example adapted from real code; real code can be found here)

I expected to see this happen: no warnings

Instead, this happened:

error: this `if` has identical blocks
 --> src/main.rs:4:48
  |
4 |       let some_check: bool = if a == 0 && b == 0 {
  |  ________________________________________________^
5 | |         true
6 | |     } else if a < b {
  | |_____^
  |
  = note: `#[deny(clippy::if_same_then_else)]` on by default
note: same as this
 --> src/main.rs:6:21
  |
6 |       } else if a < b {
  |  _____________________^
7 | |         true
8 | |     } else {
  | |_____^
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else

Meta

  • cargo clippy -V: both 0.1.54 (2021-06-19 150fad3) and clippy 0.1.52 (9bc8c42 2021-05-09)
  • rustc -Vv:
rustc 1.52.1 (9bc8c42bb 2021-05-09)
binary: rustc
commit-hash: 9bc8c42bb2f19e745a63f3445f1ac248fb015e53
commit-date: 2021-05-09
host: x86_64-unknown-linux-gnu
release: 1.52.1
LLVM version: 12.0.0
@Shnatsel Shnatsel 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 Jun 20, 2021
@extrawurst
Copy link

extrawurst commented Aug 17, 2021

I get the same false positive for another smaller repro (only since 0.1.56):

#[allow(unused_variables)]
fn main() {
    let a = true;
    let b = Some(true);
    
    if let Some(b) = b {
        println!("hello");
    }
    else if a {
        println!("hello");
    }
}

see https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=c49e249051ac25681b4fe4522994a524

@xFrednet
Copy link
Member

Duplicate of #3770.

TL;DR: This is working as intended, as two adjacent branches are the same. The fact that both branches are so simple makes it seem like an FP. We thought about allowing such "simple" cases, but it would be hard to determine if a statement/expression is actually simple or not.

For now, I would recommend allowing the lint for that specific block and maybe also branches_sharing_code:

#[allow(clippy::if_same_then_else, clippy::branches_sharing_code)]

@extrawurst
Copy link

extrawurst commented Aug 17, 2021

Duplicate of #3770.

TL;DR: This is working as intended, as two adjacent branches are the same. The fact that both branches are so simple makes it seem like an FP. We thought about allowing such "simple" cases, but it would be hard to determine if a statement/expression is actually simple or not.

For now, I would recommend allowing the lint for that specific block and maybe also branches_sharing_code:

#[allow(clippy::if_same_then_else, clippy::branches_sharing_code)]

@xFrednet but how is one supposed to combine both into one if? combining multiple conditions with if-let is not supported yet, right?

@xFrednet
Copy link
Member

xFrednet commented Aug 17, 2021

Ahh, in your case, it's actually an if-let and if block. I missed that, as that is not part of the main issue description. Thank you for the clarification. Currently, this is not possible, but should soon be with RFC 2497 that allows if let chains.

@xFrednet xFrednet reopened this Aug 17, 2021
@extrawurst
Copy link

extrawurst commented Aug 17, 2021

Ahh, in your case, it's actually an if-let and if block. I missed that, as that is not part of the main issue description. Thank you for the clarification. Currently, this is not possible, but should soon be with RFC 2497 that allows if let chains.

Ok but that means that until if-let chaining works this is a false-positive warning, right? Should I make a new issue with a more specific description to my case?

@xFrednet
Copy link
Member

Ok but that means that until if-let chaining works this is a false-positive warning, right?

Correct! And it might take some time until that feature gets stabilized.

Should I make a new issue with a more specific description to my case?

It would be awesome if you could do that @extrawurst. Could you also ping me in it? Then I would close this issue in favor of #3770 and the newly created issue 🙃.

@extrawurst
Copy link

@xFrednet done: #7579

@xFrednet
Copy link
Member

Thank you very much! I'm closing this in favor of #3770 and #7579

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