Merge pull request #987 from Manishearth/fix-985

Fix false-positive in `USELESS_LET_IF_SEQ`

CHANGELOG.md

@@ -1,6 +1,9 @@
 # Change Log
 All notable changes to this project will be documented in this file.
 
+## 0.0.73 — 2016-06-05
+* Fix false positives in [`useless_let_if_seq`]
+
 ## 0.0.72 — 2016-06-04
 * Fix false positives in [`useless_let_if_seq`]
 

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "clippy"
-version = "0.0.72"
+version = "0.0.73"
 authors = [
 	"Manish Goregaokar <manishsmail@gmail.com>",
 	"Andre Bogus <bogusandre@gmail.com>",
@@ -30,7 +30,7 @@ toml = "0.1"
 unicode-normalization = "0.1"
 quine-mc_cluskey = "0.2.2"
 # begin automatic update
-clippy_lints = { version = "0.0.72", path = "clippy_lints" }
+clippy_lints = { version = "0.0.73", path = "clippy_lints" }
 # end automatic update
 rustc-serialize = "0.3"
 

clippy_lints/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "clippy_lints"
 # begin automatic update
-version = "0.0.72"
+version = "0.0.73"
 # end automatic update
 authors = [
 	"Manish Goregaokar <manishsmail@gmail.com>",

clippy_lints/src/let_if_seq.rs

@@ -69,12 +69,9 @@ impl LateLintPass for LetIfSeq {
                 let Some(def) = cx.tcx.def_map.borrow().get(&decl.pat.id),
                 let hir::StmtExpr(ref if_, _) = expr.node,
                 let hir::ExprIf(ref cond, ref then, ref else_) = if_.node,
-                {
-                    let mut v = UsedVisitor { cx: cx, id: def.def_id(), used: false };
-                    hir::intravisit::walk_expr(&mut v, cond);
-                    !v.used
-                },
+                !used_in_expr(cx, def.def_id(), cond),
                 let Some(value) = check_assign(cx, def.def_id(), then),
+                !used_in_expr(cx, def.def_id(), value),
             ], {
                 let span = codemap::mk_sp(stmt.span.lo, if_.span.hi);
 
@@ -179,3 +176,13 @@ fn check_assign<'e>(cx: &LateContext, decl: hir::def_id::DefId, block: &'e hir::
 
     None
 }
+
+fn used_in_expr(cx: &LateContext, id: hir::def_id::DefId, expr: &hir::Expr) -> bool {
+    let mut v = UsedVisitor {
+        cx: cx,
+        id: id,
+        used: false
+    };
+    hir::intravisit::walk_expr(&mut v, expr);
+    v.used
+}

tests/compile-fail/let_if_seq.rs

@@ -5,6 +5,27 @@
 #![deny(useless_let_if_seq)]
 
 fn f() -> bool { true }
+fn g(x: i32) -> i32 { x + 1 }
+
+fn issue985() -> i32 {
+    let mut x = 42;
+    if f() {
+        x = g(x);
+    }
+
+    x
+}
+
+fn issue985_alt() -> i32 {
+    let mut x = 42;
+    if f() {
+        f();
+    } else {
+        x = g(x);
+    }
+
+    x
+}
 
 fn issue975() -> String {
     let mut udn = "dummy".to_string();
@@ -30,6 +51,8 @@ fn early_return() -> u8 {
 fn main() {
     early_return();
     issue975();
+    issue985();
+    issue985_alt();
 
     let mut foo = 0;
     //~^ ERROR `if _ { .. } else { .. }` is an expression