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 for if_same_then_else #3559

Closed
Vzaa opened this issue Dec 18, 2018 · 2 comments
Closed

False positive for if_same_then_else #3559

Vzaa opened this issue Dec 18, 2018 · 2 comments
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@Vzaa
Copy link

Vzaa commented Dec 18, 2018

Hello,

I was using clippy to check some code I wrote for advent of code 2018 and I believe I have a false positive for if_same_then_else.

The piece of code I get the error for:

use std::collections::HashSet;

fn main() {
    let text = "x\ny\n";
    let mut clays = HashSet::new();

    for line in text.lines() {
        let (from, to, p) = (10, 20, 10);

        if line.starts_with('x') {
            let x = p;
            for y in from..=to {
                clays.insert((x, y));
            }
        } else if line.starts_with('y') {
            let y = p;
            for x in from..=to {
                clays.insert((x, y));
            }
        }
    }
}

cargo clippy output:

$ cargo clippy                                                                                                                                                                                                                      (master ?)
    Checking cl v0.1.0 (/tmp/cl)                                                                                                                                                                                                               
error: this `if` has identical blocks                                                                                                                                                                                                          
  --> src/main.rs:15:41                                                                                                                                                                                                                        
   |                                                                                                                                                                                                                                           
15 |           } else if line.starts_with('y') {                                                                                                                                                                                               
   |  _________________________________________^                                                                                                                                                                                               
16 | |             let y = p;                                                                                                                                                                                                                  
17 | |             for x in from..=to {                                                                                                                                                                                                        
18 | |                 clays.insert((x, y));                                                                                                                                                                                                   
19 | |             }                                                                                                                                                                                                                           
20 | |         }                                                                                                                                                                                                                               
   | |_________^                                                                                                                                                                                                                               
   |                                                                                                                                                                                                                                           
   = note: #[deny(clippy::if_same_then_else)] on by default                                                                                                                                                                                    
note: same as this                                                                                                                                                                                                                             
  --> src/main.rs:10:34                                                                                                                                                                                                                        
   |                                                                                                                                                                                                                                           
10 |           if line.starts_with('x') {                                                                                                                                                                                                      
   |  __________________________________^                                                                                                                                                                                                      
11 | |             let x = p;                                                                                                                                                                                                                  
12 | |             for y in from..=to {                                                                                                                                                                                                        
13 | |                 clays.insert((x, y));                                                                                                                                                                                                   
14 | |             }                                                                                                                                                                                                                           
15 | |         } else if line.starts_with('y') {                                                                                                                                                                                               
   | |_________^                                                                                                                                                                                                                               
   = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/master/index.html#if_same_then_else                                                                                                                   
                                                                                                                                                                                                                                               
error: aborting due to previous error                                                                                                                                                                                                          
                                                                                                                                                                                                                                               
error: Could not compile `cl`.   

In the code it either iterates fromfrom to to for x or y while p determines the stationary point. The blocks actually have different outcomes.

cargo clippy -V output:

$ cargo clippy -V                                                                                                                                                                                                                   (master ?)
clippy 0.0.212 (2e26fdc 2018-11-22)

The same error is reported on the version on Rust Playground:
Playground Link

@thomwiggers
Copy link

If you substitute the let x and let y into the tuple, clippy won't complain:

use std::collections::HashSet;

fn main() {
    let text = "x\ny\n";
    let mut clays = HashSet::new();

    for line in text.lines() {
        let (from, to, p) = (10, 20, 10);

        if line.starts_with('x') {
            for y in from..=to {
                clays.insert((p, y));
            }
        } else if line.starts_with('y') {
            for x in from..=to {
                clays.insert((x, p));
            }
        }
    }
}

(Playground)

@oli-obk oli-obk added the C-bug Category: Clippy is not doing the correct thing label Dec 18, 2018
bors added a commit that referenced this issue Dec 31, 2018
Fix if_same_then_else false positive

This fixes false positive in #3559.
The problem was that `SpanlessEq` does not check patterns in declarations. So this two blocks considered equal.
```rust
if true {
    let (x, y) = foo();
} else {
   let (y, x) = foo();
}
```
Not sure if the proposed change is safe as `SpanlessEq` is used extensively in other lints, but I tried hard to come up with counterexample and failed.
@detrumi
Copy link
Member

detrumi commented Jan 4, 2019

Fixed by #3590, so can be closed.

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

5 participants