Lint about consecutive ifs with same condition

README.md

@@ -47,6 +47,7 @@ name
 [for_loop_over_option](https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_option)                   | warn    | for-looping over an `Option`, which is more clearly expressed as an `if let`
 [for_loop_over_result](https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_result)                   | warn    | for-looping over a `Result`, which is more clearly expressed as an `if let`
 [identity_op](https://github.com/Manishearth/rust-clippy/wiki#identity_op)                                     | warn    | using identity operations, e.g. `x + 0` or `y / 1`
+[ifs_same_cond](https://github.com/Manishearth/rust-clippy/wiki#ifs_same_cond)                                 | warn    | consecutive `ifs` with the same condition
 [ineffective_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#ineffective_bit_mask)                   | warn    | expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2`
 [inline_always](https://github.com/Manishearth/rust-clippy/wiki#inline_always)                                 | warn    | `#[inline(always)]` is a bad idea in most cases
 [invalid_regex](https://github.com/Manishearth/rust-clippy/wiki#invalid_regex)                                 | deny    | finds invalid regular expressions in `Regex::new(_)` invocations

src/copies.rs

@@ -0,0 +1,68 @@
+use rustc::lint::*;
+use rustc_front::hir::*;
+use utils::{get_parent_expr, is_exp_equal, span_lint};
+
+/// **What it does:** This lint checks for consecutive `ifs` with the same condition. This lint is
+/// `Warn` by default.
+///
+/// **Why is this bad?** This is probably a copy & paste error.
+///
+/// **Known problems:** Hopefully none.
+///
+/// **Example:** `if a == b { .. } else if a == b { .. }`
+declare_lint! {
+    pub IFS_SAME_COND,
+    Warn,
+    "consecutive `ifs` with the same condition"
+}
+
+#[derive(Copy, Clone, Debug)]
+pub struct CopyAndPaste;
+
+impl LintPass for CopyAndPaste {
+    fn get_lints(&self) -> LintArray {
+        lint_array![
+            IFS_SAME_COND
+        ]
+    }
+}
+
+impl LateLintPass for CopyAndPaste {
+    fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
+        // skip ifs directly in else, it will be checked in the parent if
+        if let Some(&Expr{node: ExprIf(_, _, Some(ref else_expr)), ..}) = get_parent_expr(cx, expr) {
+            if else_expr.id == expr.id {
+                return;
+            }
+        }
+
+        let conds = condition_sequence(expr);
+
+        for (n, i) in conds.iter().enumerate() {
+            for j in conds.iter().skip(n+1) {
+                if is_exp_equal(cx, i, j) {
+                    span_lint(cx, IFS_SAME_COND, j.span, "this if as the same condition as a previous if");
+                }
+            }
+        }
+    }
+}
+
+/// Return the list of conditions expression in a sequence of `if/else`.
+/// Eg. would return `[a, b]` for the expression `if a {..} else if b {..}`.
+fn condition_sequence(mut expr: &Expr) -> Vec<&Expr> {
+    let mut result = vec![];
+
+    while let ExprIf(ref cond, _, ref else_expr) = expr.node {
+        result.push(&**cond);
+
+        if let Some(ref else_expr) = *else_expr {
+            expr = else_expr;
+        }
+        else {
+            break;
+        }
+    }
+
+    result
+}

src/lib.rs

@@ -37,6 +37,7 @@ use rustc_plugin::Registry;
 
 #[macro_use]
 pub mod utils;
+pub mod copies;
 pub mod consts;
 pub mod types;
 pub mod misc;
@@ -157,6 +158,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_late_lint_pass(box drop_ref::DropRefPass);
     reg.register_late_lint_pass(box types::AbsurdUnsignedComparisons);
     reg.register_late_lint_pass(box regex::RegexPass);
+    reg.register_late_lint_pass(box copies::CopyAndPaste);
 
     reg.register_lint_group("clippy_pedantic", vec![
         enum_glob_use::ENUM_GLOB_USE,
@@ -190,6 +192,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR,
         block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT,
         collapsible_if::COLLAPSIBLE_IF,
+        copies::IFS_SAME_COND,
         cyclomatic_complexity::CYCLOMATIC_COMPLEXITY,
         derive::DERIVE_HASH_NOT_EQ,
         derive::EXPL_IMPL_CLONE_ON_COPY,

src/utils.rs

@@ -596,16 +596,38 @@ pub fn is_exp_equal(cx: &LateContext, left: &Expr, right: &Expr) -> bool {
         }
     }
     match (&left.node, &right.node) {
+        (&ExprAddrOf(ref lmut, ref le), &ExprAddrOf(ref rmut, ref re)) => {
+            lmut == rmut && is_exp_equal(cx, le, re)
+        }
+        (&ExprBinary(lop, ref ll, ref lr), &ExprBinary(rop, ref rl, ref rr)) => {
+            lop.node == rop.node && is_exp_equal(cx, ll, rl) && is_exp_equal(cx, lr, rr)
+        }
+        (&ExprCall(ref lfun, ref largs), &ExprCall(ref rfun, ref rargs)) => {
+            is_exp_equal(cx, lfun, rfun) && is_exps_equal(cx, largs, rargs)
+        }
+        (&ExprCast(ref lx, ref lt), &ExprCast(ref rx, ref rt)) => is_exp_equal(cx, lx, rx) && is_cast_ty_equal(lt, rt),
         (&ExprField(ref lfexp, ref lfident), &ExprField(ref rfexp, ref rfident)) => {
             lfident.node == rfident.node && is_exp_equal(cx, lfexp, rfexp)
         }
+        (&ExprIndex(ref la, ref li), &ExprIndex(ref ra, ref ri)) => {
+            is_exp_equal(cx, la, ra) && is_exp_equal(cx, li, ri)
+        }
         (&ExprLit(ref l), &ExprLit(ref r)) => l.node == r.node,
+        (&ExprMethodCall(ref lname, ref ltys, ref largs), &ExprMethodCall(ref rname, ref rtys, ref rargs)) => {
+            // TODO: tys
+            lname.node == rname.node && ltys.is_empty() && rtys.is_empty() && is_exps_equal(cx, largs, rargs)
+        }
         (&ExprPath(ref lqself, ref lsubpath), &ExprPath(ref rqself, ref rsubpath)) => {
             both(lqself, rqself, is_qself_equal) && is_path_equal(lsubpath, rsubpath)
         }
         (&ExprTup(ref ltup), &ExprTup(ref rtup)) => is_exps_equal(cx, ltup, rtup),
+        (&ExprTupField(ref le, li), &ExprTupField(ref re, ri)) => {
+            li.node == ri.node && is_exp_equal(cx, le, re)
+        }
+        (&ExprUnary(lop, ref le), &ExprUnary(rop, ref re)) => {
+            lop == rop && is_exp_equal(cx, le, re)
+        }
         (&ExprVec(ref l), &ExprVec(ref r)) => is_exps_equal(cx, l, r),
-        (&ExprCast(ref lx, ref lt), &ExprCast(ref rx, ref rt)) => is_exp_equal(cx, lx, rx) && is_cast_ty_equal(lt, rt),
         _ => false,
     }
 }

tests/compile-fail/copies.rs

@@ -0,0 +1,31 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![deny(clippy)]
+
+fn foo() -> bool { unimplemented!() }
+
+fn main() {
+    let a = 0;
+
+    if a == 1 {
+    }
+    else if a == 1 { //~ERROR this if as the same condition as a previous if
+    }
+
+    if 2*a == 1 {
+    }
+    else if 2*a == 2 {
+    }
+    else if 2*a == 1 { //~ERROR this if as the same condition as a previous if
+    }
+    else if a == 1 {
+    }
+
+    // Ok, maybe `foo` isn’t pure and this actually makes sense. But you should probably refactor
+    // this to make the intention clearer anyway.
+    if foo() {
+    }
+    else if foo() { //~ERROR this if as the same condition as a previous if
+    }
+}