Auto merge of #3590 - jorpic:i3559-if_same_then_else, r=phansch

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.
This commit is contained in:
bors 2018-12-31 09:25:18 +00:00
commit 6f3912850a
3 changed files with 20 additions and 7 deletions

View File

@ -54,7 +54,9 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
match (&left.node, &right.node) {
(&StmtKind::Decl(ref l, _), &StmtKind::Decl(ref r, _)) => {
if let (&DeclKind::Local(ref l), &DeclKind::Local(ref r)) = (&l.node, &r.node) {
both(&l.ty, &r.ty, |l, r| self.eq_ty(l, r)) && both(&l.init, &r.init, |l, r| self.eq_expr(l, r))
self.eq_pat(&l.pat, &r.pat)
&& both(&l.ty, &r.ty, |l, r| self.eq_ty(l, r))
&& both(&l.init, &r.init, |l, r| self.eq_expr(l, r))
} else {
false
}

View File

@ -335,6 +335,17 @@ fn if_same_then_else() -> Result<&'static str, ()> {
let foo = "";
return Ok(&foo[0..]);
}
// false positive if_same_then_else, let(x,y) vs let(y,x), see #3559
if true {
let foo = "";
let (x, y) = (1, 2);
return Ok(&foo[x..y]);
} else {
let foo = "";
let (y, x) = (1, 2);
return Ok(&foo[x..y]);
}
}
#[warn(clippy::ifs_same_cond)]

View File

@ -352,38 +352,38 @@ LL | | } else {
| |_____^
error: this `if` has the same condition as a previous if
--> $DIR/copies.rs:347:15
--> $DIR/copies.rs:358:15
|
LL | } else if b {
| ^
|
= note: `-D clippy::ifs-same-cond` implied by `-D warnings`
note: same as this
--> $DIR/copies.rs:346:8
--> $DIR/copies.rs:357:8
|
LL | if b {
| ^
error: this `if` has the same condition as a previous if
--> $DIR/copies.rs:352:15
--> $DIR/copies.rs:363:15
|
LL | } else if a == 1 {
| ^^^^^^
|
note: same as this
--> $DIR/copies.rs:351:8
--> $DIR/copies.rs:362:8
|
LL | if a == 1 {
| ^^^^^^
error: this `if` has the same condition as a previous if
--> $DIR/copies.rs:358:15
--> $DIR/copies.rs:369:15
|
LL | } else if 2 * a == 1 {
| ^^^^^^^^^^
|
note: same as this
--> $DIR/copies.rs:356:8
--> $DIR/copies.rs:367:8
|
LL | if 2 * a == 1 {
| ^^^^^^^^^^