diff --git a/clippy_lints/src/eq_op.rs b/clippy_lints/src/eq_op.rs index bfc16a504570..dccfed872692 100644 --- a/clippy_lints/src/eq_op.rs +++ b/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)]); + } + ) + } + _ => {} + } + } } } }