Fix op_ref in case of nonexisting operations

diff --git a/clippy_lints/src/eq_op.rs b/clippy_lints/src/eq_op.rs
index dccfed87269222f8edb0624b6ef976729d44cff7..002725a4d6d90f312602eb276a25fb3dda2cbdaa 100644 (file)
--- a/clippy_lints/src/eq_op.rs
+++ b/clippy_lints/src/eq_op.rs
@@ -1,6 +1,6 @@
 use rustc::hir::*;
 use rustc::lint::*;
-use utils::{SpanlessEq, span_lint, span_lint_and_then, multispan_sugg, snippet};
+use utils::{SpanlessEq, span_lint, span_lint_and_then, multispan_sugg, snippet, implements_trait};
 use utils::sugg::Sugg;
 
 /// **What it does:** Checks for equal operands to comparison, logical and
@@ -60,53 +60,87 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
                             e.span,
                             &format!("equal expressions as operands to `{}`", op.node.as_str()));
                 } else {
-                    match (&left.node, &right.node) {
-                        (&ExprAddrOf(_, ref l), &ExprAddrOf(_, ref r)) => {
-                            span_lint_and_then(cx,
-                                OP_REF,
-                                e.span,
-                                "taken reference of both operands, which is done automatically by the operator anyway",
-                                |db| {
-                                    let lsnip = snippet(cx, l.span, "...").to_string();
-                                    let rsnip = snippet(cx, r.span, "...").to_string();
-                                    multispan_sugg(db,
-                                                "use the values directly".to_string(),
-                                                vec![(left.span, lsnip),
-                                                     (right.span, rsnip)]);
+                    let trait_id = match op.node {
+                        BiAdd => cx.tcx.lang_items.add_trait(),
+                        BiSub => cx.tcx.lang_items.sub_trait(),
+                        BiMul => cx.tcx.lang_items.mul_trait(),
+                        BiDiv => cx.tcx.lang_items.div_trait(),
+                        BiRem => cx.tcx.lang_items.rem_trait(),
+                        BiAnd => None,
+                        BiOr => None,
+                        BiBitXor => cx.tcx.lang_items.bitxor_trait(),
+                        BiBitAnd => cx.tcx.lang_items.bitand_trait(),
+                        BiBitOr => cx.tcx.lang_items.bitor_trait(),
+                        BiShl => cx.tcx.lang_items.shl_trait(),
+                        BiShr => cx.tcx.lang_items.shr_trait(),
+                        BiNe |
+                        BiEq => cx.tcx.lang_items.eq_trait(),
+                        BiLt |
+                        BiLe |
+                        BiGe |
+                        BiGt => cx.tcx.lang_items.ord_trait(),
+                    };
+                    if let Some(trait_id) = trait_id {
+                        match (&left.node, &right.node) {
+                            // do not suggest to dereference literals
+                            (&ExprLit(..), _) |
+                            (_, &ExprLit(..)) => {},
+                            // &foo == &bar
+                            (&ExprAddrOf(_, ref l), &ExprAddrOf(_, ref r)) => {
+                                if implements_trait(cx, cx.tables.expr_ty(l), trait_id, &[cx.tables.expr_ty(r)], None) {
+                                    span_lint_and_then(cx,
+                                        OP_REF,
+                                        e.span,
+                                        "taken reference of both operands, which is done automatically by the operator anyway",
+                                        |db| {
+                                            let lsnip = snippet(cx, l.span, "...").to_string();
+                                            let rsnip = snippet(cx, r.span, "...").to_string();
+                                            multispan_sugg(db,
+                                                        "use the values directly".to_string(),
+                                                        vec![(left.span, lsnip),
+                                                            (right.span, rsnip)]);
+                                        }
+                                    )
                                 }
-                            )
-                        }
-                        (&ExprAddrOf(_, ref l), _) => {
-                            span_lint_and_then(cx,
-                                OP_REF,
-                                e.span,
-                                "taken reference of left operand",
-                                |db| {
-                                    let lsnip = snippet(cx, l.span, "...").to_string();
-                                    let rsnip = Sugg::hir(cx, right, "...").deref().to_string();
-                                    multispan_sugg(db,
-                                                "dereference the right operand instead".to_string(),
-                                                vec![(left.span, lsnip),
-                                                     (right.span, rsnip)]);
+                            }
+                            // &foo == bar
+                            (&ExprAddrOf(_, ref l), _) => {
+                                if implements_trait(cx, cx.tables.expr_ty(l), trait_id, &[cx.tables.expr_ty(right)], None) {
+                                    span_lint_and_then(cx,
+                                        OP_REF,
+                                        e.span,
+                                        "taken reference of left operand",
+                                        |db| {
+                                            let lsnip = snippet(cx, l.span, "...").to_string();
+                                            let rsnip = Sugg::hir(cx, right, "...").deref().to_string();
+                                            multispan_sugg(db,
+                                                        "dereference the right operand instead".to_string(),
+                                                        vec![(left.span, lsnip),
+                                                            (right.span, rsnip)]);
+                                        }
+                                    )
                                 }
-                            )
-                        }
-                        (_, &ExprAddrOf(_, ref r)) => {
-                            span_lint_and_then(cx,
-                                OP_REF,
-                                e.span,
-                                "taken reference of right operand",
-                                |db| {
-                                    let lsnip = Sugg::hir(cx, left, "...").deref().to_string();
-                                    let rsnip = snippet(cx, r.span, "...").to_string();
-                                    multispan_sugg(db,
-                                                "dereference the left operand instead".to_string(),
-                                                vec![(left.span, lsnip),
-                                                     (right.span, rsnip)]);
+                            }
+                            // foo == &bar
+                            (_, &ExprAddrOf(_, ref r)) => {
+                                if implements_trait(cx, cx.tables.expr_ty(left), trait_id, &[cx.tables.expr_ty(r)], None) {
+                                    span_lint_and_then(cx,
+                                        OP_REF,
+                                        e.span,
+                                        "taken reference of right operand",
+                                        |db| {
+                                            let lsnip = Sugg::hir(cx, left, "...").deref().to_string();
+                                            let rsnip = snippet(cx, r.span, "...").to_string();
+                                            multispan_sugg(db,
+                                                        "dereference the left operand instead".to_string(),
+                                                        vec![(left.span, lsnip),
+                                                            (right.span, rsnip)]);
+                                        }
+                                    )
                                 }
-                            )
+                            }
+                            _ => {}
                         }
-                        _ => {}
                     }
                 }
             }

diff --git a/tests/ui/op_ref.rs b/tests/ui/op_ref.rs
new file mode 100644 (file)
index 0000000..315e653
--- /dev/null
+++ b/tests/ui/op_ref.rs
@@ -0,0 +1,24 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![allow(unused_variables, blacklisted_name)]
+
+use std::collections::HashSet;
+
+fn main() {
+    let tracked_fds: HashSet<i32> = HashSet::new();
+    let new_fds = HashSet::new();
+    let unwanted = &tracked_fds - &new_fds;
+    
+    let foo = &5 - &6;
+
+    let bar = String::new();
+    let bar = "foo" == &bar;
+
+    let a = "a".to_string();
+    let b = "a";
+
+    if b < &a {
+        println!("OK");
+    }
+}
\ No newline at end of file

diff --git a/tests/ui/op_ref.stderr b/tests/ui/op_ref.stderr
new file mode 100644 (file)
index 0000000..6e6ca45
--- /dev/null
+++ b/tests/ui/op_ref.stderr
@@ -0,0 +1,10 @@
+warning: taken reference of both operands, which is done automatically by the operator anyway
+  --> $DIR/op_ref.rs:13:15
+   |
+13 |     let foo = &5 - &6;
+   |               ^^^^^^^
+   |
+   = note: #[warn(op_ref)] on by default
+help: use the values directly
+   |     let foo = 5 - 6;
+