Don't create additional references when invoking binary operators

Author: oli-obk

clippy_lints/src/eq_op.rs

@@ -1,6 +1,7 @@
 use rustc::hir::*;
 use rustc::lint::*;
-use utils::{SpanlessEq, span_lint};
+use utils::{SpanlessEq, span_lint, span_lint_and_then, multispan_sugg, snippet};
+use utils::sugg::Sugg;
 
 /// **What it does:** Checks for equal operands to comparison, logical and
 /// bitwise, difference and division binary operators (`==`, `>`, etc., `&&`,
@@ -23,23 +24,91 @@ declare_lint! {
     "equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`)"
 }
 
+/// **What it does:** Checks for arguments to `==` which have their address taken to satisfy a bound
+/// and suggests to dereference the other argument instead
+///
+/// **Why is this bad?** It is more idiomatic to dereference the other argument.
+///
+/// **Known problems:** None
+///
+/// **Example:**
+/// ```rust
+/// &x == y
+/// ```
+declare_lint! {
+    pub OP_REF,
+    Warn,
+    "taking a reference to satisfy the type constraints on `==`"
+}
+
 #[derive(Copy,Clone)]
 pub struct EqOp;
 
 impl LintPass for EqOp {
     fn get_lints(&self) -> LintArray {
-        lint_array!(EQ_OP)
+        lint_array!(EQ_OP, OP_REF)
     }
 }
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EqOp {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
         if let ExprBinary(ref op, ref left, ref right) = e.node {
-            if is_valid_operator(op) && SpanlessEq::new(cx).ignore_fn().eq_expr(left, right) {
-                span_lint(cx,
-                          EQ_OP,
-                          e.span,
-                          &format!("equal expressions as operands to `{}`", op.node.as_str()));
+            if is_valid_operator(op) {
+                if SpanlessEq::new(cx).ignore_fn().eq_expr(left, right) {
+                    span_lint(cx,
+                            EQ_OP,
+                            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)]);
+                                }
+                            )
+                        }
+                        (&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)]);
+                                }
+                            )
+                        }
+                        (_, &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)]);
+                                }
+                            )
+                        }
+                        _ => {}
+                    }
+                }
             }
         }
     }