Implement Expr spanless-hashing

src/consts.rs

@@ -17,7 +17,7 @@ use syntax::ast::{UintTy, FloatTy, StrStyle};
 use syntax::ast::Sign::{self, Plus, Minus};
 
 
-#[derive(PartialEq, Eq, Debug, Copy, Clone)]
+#[derive(PartialEq, Eq, Debug, Copy, Clone, Hash)]
 pub enum FloatWidth {
     Fw32,
     Fw64,
@@ -34,7 +34,7 @@ impl From<FloatTy> for FloatWidth {
 }
 
 /// a Lit_-like enum to fold constant `Expr`s into
-#[derive(Eq, Debug, Clone)]
+#[derive(Eq, Debug, Clone, Hash)]
 pub enum Constant {
     /// a String "abc"
     Str(String, StrStyle),

src/copies.rs

@@ -1,6 +1,8 @@
 use rustc::lint::*;
 use rustc_front::hir::*;
-use utils::SpanlessEq;
+use std::collections::HashMap;
+use std::collections::hash_map::Entry;
+use utils::{SpanlessEq, SpanlessHash};
 use utils::{get_parent_expr, in_macro, span_lint, span_note_and_lint};
 
 /// **What it does:** This lint checks for consecutive `ifs` with the same condition. This lint is
@@ -46,8 +48,16 @@ impl LintPass for CopyAndPaste {
 impl LateLintPass for CopyAndPaste {
     fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
         if !in_macro(cx, expr.span) {
+            // 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, blocks) = if_sequence(expr);
             lint_same_then_else(cx, expr);
-            lint_same_cond(cx, expr);
+            lint_same_cond(cx, &conds);
         }
     }
 }
@@ -64,32 +74,22 @@ fn lint_same_then_else(cx: &LateContext, expr: &Expr) {
 }
 
 /// Implementation of `IFS_SAME_COND`.
-fn lint_same_cond(cx: &LateContext, expr: &Expr) {
+fn lint_same_cond(cx: &LateContext, conds: &[&Expr]) {
-    // skip ifs directly in else, it will be checked in the parent if
+    if let Some((i, j)) = search_same(cx, conds) {
-    if let Some(&Expr{node: ExprIf(_, _, Some(ref else_expr)), ..}) = get_parent_expr(cx, expr) {
+        span_note_and_lint(cx, IFS_SAME_COND, j.span, "this if has the same condition as a previous if", i.span, "same as this");
-        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 SpanlessEq::new(cx).ignore_fn().eq_expr(i, j) {
-                span_note_and_lint(cx, IFS_SAME_COND, j.span, "this if has the same condition as a previous if", i.span, "same as this");
-            }
-        }
     }
 }
 
-/// Return the list of condition expressions in a sequence of `if/else`.
+/// Return the list of condition expressions and the list of blocks in a sequence of `if/else`.
-/// Eg. would return `[a, b]` for the expression `if a {..} else if b {..}`.
+/// Eg. would return `([a, b], [c, d, e])` for the expression
-fn condition_sequence(mut expr: &Expr) -> Vec<&Expr> {
+/// `if a { c } else if b { d } else { e }`.
-    let mut result = vec![];
+fn if_sequence(mut expr: &Expr) -> (Vec<&Expr>, Vec<&Block>) {
+    let mut conds = vec![];
+    let mut blocks = vec![];
 
-    while let ExprIf(ref cond, _, ref else_expr) = expr.node {
+    while let ExprIf(ref cond, ref then_block, ref else_expr) = expr.node {
-        result.push(&**cond);
+        conds.push(&**cond);
+        blocks.push(&**then_block);
 
         if let Some(ref else_expr) = *else_expr {
             expr = else_expr;
@@ -99,5 +99,48 @@ fn condition_sequence(mut expr: &Expr) -> Vec<&Expr> {
         }
     }
 
-    result
+    // final `else {..}`
+    if !blocks.is_empty() {
+        if let ExprBlock(ref block) = expr.node {
+            blocks.push(&**block);
+        }
+    }
+
+    (conds, blocks)
+}
+
+fn search_same<'a>(cx: &LateContext, exprs: &[&'a Expr]) -> Option<(&'a Expr, &'a Expr)> {
+    // common cases
+    if exprs.len() < 2 {
+        return None;
+    }
+    else if exprs.len() == 2 {
+        return if SpanlessEq::new(cx).ignore_fn().eq_expr(&exprs[0], &exprs[1]) {
+            Some((&exprs[0], &exprs[1]))
+        }
+        else {
+            None
+        }
+    }
+
+    let mut map : HashMap<_, Vec<&'a _>> = HashMap::with_capacity(exprs.len());
+
+    for &expr in exprs {
+        let mut h = SpanlessHash::new(cx);
+        h.hash_expr(expr);
+        let h = h.finish();
+
+        match map.entry(h) {
+            Entry::Occupied(o) => {
+                for o in o.get() {
+                    if SpanlessEq::new(cx).ignore_fn().eq_expr(o, expr) {
+                        return Some((o, expr))
+                    }
+                }
+            }
+            Entry::Vacant(v) => { v.insert(vec![expr]); }
+        }
+    }
+
+    None
 }

src/utils/hir.rs

@@ -1,6 +1,8 @@
 use consts::constant;
 use rustc::lint::*;
 use rustc_front::hir::*;
+use std::hash::{Hash, Hasher, SipHasher};
+use syntax::ast::Name;
 use syntax::ptr::P;
 
 /// Type used to check whether two ast are the same. This is different from the operator
@@ -242,3 +244,288 @@ fn over<X, F>(left: &[X], right: &[X], mut eq_fn: F) -> bool
 {
     left.len() == right.len() && left.iter().zip(right).all(|(x, y)| eq_fn(x, y))
 }
+
+
+pub struct SpanlessHash<'a, 'tcx: 'a> {
+    /// Context used to evaluate constant expressions.
+    cx: &'a LateContext<'a, 'tcx>,
+    s: SipHasher,
+}
+
+impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
+    pub fn new(cx: &'a LateContext<'a, 'tcx>) -> Self {
+        SpanlessHash { cx: cx, s: SipHasher::new() }
+    }
+
+    pub fn finish(&self) -> u64 {
+        self.s.finish()
+    }
+
+    pub fn hash_block(&mut self, b: &Block) {
+        for s in &b.stmts {
+            self.hash_stmt(s);
+        }
+
+        if let Some(ref e) = b.expr {
+            self.hash_expr(e);
+        }
+
+        b.rules.hash(&mut self.s);
+    }
+
+    pub fn hash_expr(&mut self, e: &Expr) {
+        if let Some(e) = constant(self.cx, e) {
+            return e.hash(&mut self.s);
+        }
+
+        match e.node {
+            ExprAddrOf(m, ref e) => {
+                let c: fn(_, _) -> _ = ExprAddrOf;
+                c.hash(&mut self.s);
+                m.hash(&mut self.s);
+                self.hash_expr(e);
+            }
+            ExprAgain(i) => {
+                let c: fn(_) -> _ = ExprAgain;
+                c.hash(&mut self.s);
+                if let Some(i) = i {
+                    self.hash_name(&i.node.name);
+                }
+            }
+            ExprAssign(ref l, ref r) => {
+                let c: fn(_, _) -> _ = ExprAssign;
+                c.hash(&mut self.s);
+                self.hash_expr(l);
+                self.hash_expr(r);
+            }
+            ExprAssignOp(ref o, ref l, ref r) => {
+                let c: fn(_, _, _) -> _ = ExprAssignOp;
+                c.hash(&mut self.s);
+                o.hash(&mut self.s);
+                self.hash_expr(l);
+                self.hash_expr(r);
+            }
+            ExprBlock(ref b) => {
+                let c: fn(_) -> _ = ExprBlock;
+                c.hash(&mut self.s);
+                self.hash_block(b);
+            }
+            ExprBinary(op, ref l, ref r) => {
+                let c: fn(_, _, _) -> _ = ExprBinary;
+                c.hash(&mut self.s);
+                op.node.hash(&mut self.s);
+                self.hash_expr(l);
+                self.hash_expr(r);
+            }
+            ExprBreak(i) => {
+                let c: fn(_) -> _ = ExprBreak;
+                c.hash(&mut self.s);
+                if let Some(i) = i {
+                    self.hash_name(&i.node.name);
+                }
+            }
+            ExprBox(ref e) => {
+                let c: fn(_) -> _ = ExprBox;
+                c.hash(&mut self.s);
+                self.hash_expr(e);
+            }
+            ExprCall(ref fun, ref args) => {
+                let c: fn(_, _) -> _ = ExprCall;
+                c.hash(&mut self.s);
+                self.hash_expr(fun);
+                self.hash_exprs(args);
+            }
+            ExprCast(ref e, ref _ty) => {
+                let c: fn(_, _) -> _ = ExprCast;
+                c.hash(&mut self.s);
+                self.hash_expr(e);
+                // TODO: _ty
+            }
+            ExprClosure(cap, _, ref b) => {
+                let c: fn(_, _, _) -> _ = ExprClosure;
+                c.hash(&mut self.s);
+                cap.hash(&mut self.s);
+                self.hash_block(b);
+            }
+            ExprField(ref e, ref f) => {
+                let c: fn(_, _) -> _ = ExprField;
+                c.hash(&mut self.s);
+                self.hash_expr(e);
+                self.hash_name(&f.node);
+            }
+            ExprIndex(ref a, ref i) => {
+                let c: fn(_, _) -> _ = ExprIndex;
+                c.hash(&mut self.s);
+                self.hash_expr(a);
+                self.hash_expr(i);
+            }
+            ExprInlineAsm(_) => {
+                let c: fn(_) -> _ = ExprInlineAsm;
+                c.hash(&mut self.s);
+            }
+            ExprIf(ref cond, ref t, ref e) => {
+                let c: fn(_, _, _) -> _ = ExprIf;
+                c.hash(&mut self.s);
+                self.hash_expr(cond);
+                self.hash_block(t);
+                if let Some(ref e) = *e {
+                    self.hash_expr(e);
+                }
+            }
+            ExprLit(ref l) => {
+                let c: fn(_) -> _ = ExprLit;
+                c.hash(&mut self.s);
+                l.hash(&mut self.s);
+            },
+            ExprLoop(ref b, ref i) => {
+                let c: fn(_, _) -> _ = ExprLoop;
+                c.hash(&mut self.s);
+                self.hash_block(b);
+                if let Some(i) = *i {
+                    self.hash_name(&i.name);
+                }
+            }
+            ExprMatch(ref e, ref arms, ref s) => {
+                let c: fn(_, _, _) -> _ = ExprMatch;
+                c.hash(&mut self.s);
+                self.hash_expr(e);
+
+                for arm in arms {
+                    // TODO: arm.pat?
+                    if let Some(ref e) = arm.guard {
+                        self.hash_expr(e);
+                    }
+                    self.hash_expr(&arm.body);
+                }
+
+                s.hash(&mut self.s);
+            }
+            ExprMethodCall(ref name, ref _tys, ref args) => {
+                let c: fn(_, _, _) -> _ = ExprMethodCall;
+                c.hash(&mut self.s);
+                self.hash_name(&name.node);
+                self.hash_exprs(args);
+            }
+            ExprRange(ref b, ref e) => {
+                let c: fn(_, _) -> _ = ExprRange;
+                c.hash(&mut self.s);
+                if let Some(ref b) = *b {
+                    self.hash_expr(b);
+                }
+                if let Some(ref e) = *e {
+                    self.hash_expr(e);
+                }
+            }
+            ExprRepeat(ref e, ref l) => {
+                let c: fn(_, _) -> _ = ExprRepeat;
+                c.hash(&mut self.s);
+                self.hash_expr(e);
+                self.hash_expr(l);
+            }
+            ExprRet(ref e) => {
+                let c: fn(_) -> _ = ExprRet;
+                c.hash(&mut self.s);
+                if let Some(ref e) = *e {
+                    self.hash_expr(e);
+                }
+            }
+            ExprPath(ref _qself, ref subpath) => {
+                let c: fn(_, _) -> _ = ExprPath;
+                c.hash(&mut self.s);
+                self.hash_path(subpath);
+            }
+            ExprStruct(ref path, ref fields, ref expr) => {
+                let c: fn(_, _, _) -> _ = ExprStruct;
+                c.hash(&mut self.s);
+
+                self.hash_path(path);
+
+                for f in fields {
+                    self.hash_name(&f.name.node);
+                    self.hash_expr(&f.expr);
+                }
+
+                if let Some(ref e) = *expr {
+                    self.hash_expr(e);
+                }
+            }
+            ExprTup(ref tup) => {
+                let c: fn(_) -> _ = ExprTup;
+                c.hash(&mut self.s);
+                self.hash_exprs(tup);
+            },
+            ExprTupField(ref le, li) => {
+                let c: fn(_, _) -> _ = ExprTupField;
+                c.hash(&mut self.s);
+
+                self.hash_expr(le);
+                li.node.hash(&mut self.s);
+            }
+            ExprType(_, _) => {
+                let c: fn(_, _) -> _ = ExprType;
+                c.hash(&mut self.s);
+                // what’s an ExprType anyway?
+            }
+            ExprUnary(lop, ref le) => {
+                let c: fn(_, _) -> _ = ExprUnary;
+                c.hash(&mut self.s);
+
+                lop.hash(&mut self.s);
+                self.hash_expr(le);
+            }
+            ExprVec(ref v) => {
+                let c: fn(_) -> _ = ExprVec;
+                c.hash(&mut self.s);
+
+                self.hash_exprs(v);
+            },
+            ExprWhile(ref cond, ref b, l) => {
+                let c: fn(_, _, _) -> _ = ExprWhile;
+                c.hash(&mut self.s);
+
+                self.hash_expr(cond);
+                self.hash_block(b);
+                if let Some(l) = l {
+                    self.hash_name(&l.name);
+                }
+            }
+        }
+    }
+
+    pub fn hash_exprs(&mut self, e: &[P<Expr>]) {
+        for e in e {
+            self.hash_expr(e);
+        }
+    }
+
+    pub fn hash_name(&mut self, n: &Name) {
+        n.as_str().hash(&mut self.s);
+    }
+
+    pub fn hash_path(&mut self, p: &Path) {
+        p.global.hash(&mut self.s);
+        for p in &p.segments {
+            self.hash_name(&p.identifier.name);
+        }
+    }
+
+    pub fn hash_stmt(&mut self, b: &Stmt) {
+        match b.node {
+            StmtDecl(ref _decl, _) => {
+                let c: fn(_, _) -> _ = StmtDecl;
+                c.hash(&mut self.s);
+                // TODO: decl
+            }
+            StmtExpr(ref expr, _) => {
+                let c: fn(_, _) -> _ = StmtExpr;
+                c.hash(&mut self.s);
+                self.hash_expr(expr);
+            }
+            StmtSemi(ref expr, _) => {
+                let c: fn(_, _) -> _ = StmtSemi;
+                c.hash(&mut self.s);
+                self.hash_expr(expr);
+            }
+        }
+    }
+}

src/utils/mod.rs

@@ -16,7 +16,7 @@ use syntax::errors::DiagnosticBuilder;
 use syntax::ptr::P;
 
 mod hir;
-pub use self::hir::SpanlessEq;
+pub use self::hir::{SpanlessEq, SpanlessHash};
 pub type MethodArgs = HirVec<P<Expr>>;
 
 // module DefPaths for certain structs/enums we check for

tests/compile-fail/copies.rs

@@ -105,6 +105,12 @@ fn if_same_then_else() -> &'static str {
 
 fn ifs_same_cond() {
     let a = 0;
+    let b = false;
+
+    if b {
+    }
+    else if b { //~ERROR this if has the same condition as a previous if
+    }
 
     if a == 1 {
     }