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 false positive on 'else if' #6099

Closed
Wilfred opened this issue Sep 30, 2020 · 3 comments
Closed

if_same_then_else false positive on 'else if' #6099

Wilfred opened this issue Sep 30, 2020 · 3 comments
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@Wilfred
Copy link
Contributor

Wilfred commented Sep 30, 2020

I tried this code:

pub fn demo(net_loop_movement: i64) -> i64 {
    if net_loop_movement == 0 {
        0
    } else if net_loop_movement < 0 {
        // Net movement was negative, so conservatively assume
        // it was zero (e.g. the loop may run zero times).
        0
    } else {
        // Net loop movement was positive, so we can't
        // assume any bounds.
        1
    }
}

I wouldn't expect if_same_then_else to fire in this case. I'm explicitly documenting why the second case is a fallback. This is a reduced reproduction from this original source code.

It produces the following message:

error: this `if` has identical blocks
   --> src/bounds.rs:99:37
    |
99  |       } else if net_loop_movement < 0 {
    |  _____________________________________^
100 | |         // Net movement was negative, so conservatively assume
101 | |         // it was zero (e.g. the loop may run zero times).
102 | |         0
103 | |     } else {
    | |_____^
    |
    = note: `#[deny(clippy::if_same_then_else)]` on by default
note: same as this
   --> src/bounds.rs:97:31
    |
97  |       if net_loop_movement == 0 {
    |  _______________________________^
98  | |         0
99  | |     } else if net_loop_movement < 0 {
    | |_____^
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else

Could the check be more conservative? Perhaps not firing if there's an additional else later, or considering blocks to be different based on trivia like comments?

Meta

  • cargo clippy -V: clippy 0.0.212 (623fb90 2020-09-26)
  • rustc -Vv:
$ rustc -Vv
rustc 1.48.0-nightly (623fb90b5 2020-09-26)
binary: rustc
commit-hash: 623fb90b5a1f324e0ec44085116bf858cef19a00
commit-date: 2020-09-26
host: x86_64-unknown-linux-gnu
release: 1.48.0-nightly
LLVM version: 11.0
@Wilfred Wilfred added the C-bug Category: Clippy is not doing the correct thing label Sep 30, 2020
@MTRNord
Copy link

MTRNord commented Oct 1, 2020

Can reproduce with this tree:

else if msg_body.contains("!in") || msg_body.contains("!i") {

            } else if msg_body.contains("!out") || msg_body.contains("!o") {
                
            } else if msg_body.contains("!record") || msg_body.contains("!r") {
                
            } else if msg_body.contains("!break") || msg_body.contains("!b") {
                
            } else if msg_body.contains("!setTimeZoneDefault") {
                
            } else if msg_body.contains("!stats") || msg_body.contains("!s") {
                
            }

@ebroto
Copy link
Member

ebroto commented Oct 1, 2020

Duplicate of #3770, specifically this comment refering to the existence of comments in the blocks, and this other comment with a possible resolution.

Thanks for taking the time to report this in any case!

@ebroto ebroto closed this as completed Oct 1, 2020
@diepes
Copy link

diepes commented Nov 18, 2024

I ran into the same clippy lint.

When the else is followed by more logic e.g. "if" this should not trigger.

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
Projects
None yet
Development

No branches or pull requests

4 participants