diff --git a/clippy_lints/src/eq_op.rs b/clippy_lints/src/eq_op.rs index dccfed87269..002725a4d6d 100644 --- 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 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EqOp { 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 index 00000000000..315e6535ef6 --- /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 = 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 index 00000000000..6e6ca457e03 --- /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; +