Merge pull request #2412 from topecongiro/double-comparison

Add double comparison lint

Author: oli-obk

clippy_lints/src/double_comparison.rs

@@ -0,0 +1,85 @@
+//! Lint on unnecessary double comparisons. Some examples:
+
+use rustc::hir::*;
+use rustc::lint::*;
+use syntax::codemap::Span;
+
+use utils::{snippet, span_lint_and_sugg, SpanlessEq};
+
+/// **What it does:** Checks for double comparions that could be simpified to a single expression.
+///
+///
+/// **Why is this bad?** Readability.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// x == y || x < y
+/// ```
+///
+/// Could be written as:
+///
+/// ```rust
+/// x <= y
+/// ```
+declare_lint! {
+    pub DOUBLE_COMPARISONS,
+    Deny,
+    "unnecessary double comparisons that can be simplified"
+}
+
+pub struct DoubleComparisonPass;
+
+impl LintPass for DoubleComparisonPass {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(DOUBLE_COMPARISONS)
+    }
+}
+
+impl<'a, 'tcx> DoubleComparisonPass {
+    fn check_binop(
+        &self,
+        cx: &LateContext<'a, 'tcx>,
+        op: BinOp_,
+        lhs: &'tcx Expr,
+        rhs: &'tcx Expr,
+        span: Span,
+    ) {
+        let (lkind, llhs, lrhs, rkind, rlhs, rrhs) = match (lhs.node.clone(), rhs.node.clone()) {
+            (ExprBinary(lb, llhs, lrhs), ExprBinary(rb, rlhs, rrhs)) => {
+                (lb.node, llhs, lrhs, rb.node, rlhs, rrhs)
+            }
+            _ => return,
+        };
+        let spanless_eq = SpanlessEq::new(cx).ignore_fn();
+        if !(spanless_eq.eq_expr(&llhs, &rlhs) && spanless_eq.eq_expr(&lrhs, &rrhs)) {
+            return;
+        }
+        macro_rules! lint_double_comparison {
+            ($op:tt) => {{
+                let lhs_str = snippet(cx, llhs.span, "");
+                let rhs_str = snippet(cx, lrhs.span, "");
+                let sugg = format!("{} {} {}", lhs_str, stringify!($op), rhs_str);
+                span_lint_and_sugg(cx, DOUBLE_COMPARISONS, span,
+                                   "This binary expression can be simplified",
+                                   "try", sugg);
+            }}
+        }
+        match (op, lkind, rkind) {
+            (BiOr, BiEq, BiLt) | (BiOr, BiLt, BiEq) => lint_double_comparison!(<=),
+            (BiOr, BiEq, BiGt) | (BiOr, BiGt, BiEq) => lint_double_comparison!(>=),
+            (BiOr, BiLt, BiGt) | (BiOr, BiGt, BiLt) => lint_double_comparison!(!=),
+            (BiAnd, BiLe, BiGe) | (BiAnd, BiGe, BiLe) => lint_double_comparison!(==),
+            _ => (),
+        };
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DoubleComparisonPass {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
+        if let ExprBinary(ref kind, ref lhs, ref rhs) = expr.node {
+            self.check_binop(cx, kind.node, lhs, rhs, expr.span);
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -86,6 +86,7 @@ pub mod copies;
 pub mod cyclomatic_complexity;
 pub mod derive;
 pub mod doc;
+pub mod double_comparison;
 pub mod double_parens;
 pub mod drop_forget_ref;
 pub mod else_if_without_else;
@@ -369,6 +370,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_late_lint_pass(box fallible_impl_from::FallibleImplFrom);
     reg.register_late_lint_pass(box replace_consts::ReplaceConsts);
     reg.register_late_lint_pass(box types::UnitArg);
+    reg.register_late_lint_pass(box double_comparison::DoubleComparisonPass);
 
     reg.register_lint_group("clippy_restrictions", vec![
         arithmetic::FLOAT_ARITHMETIC,

clippy_lints/src/misc.rs

@@ -545,11 +545,11 @@ fn check_to_owned(cx: &LateContext, expr: &Expr, other: &Expr) {
 
     // *arg impls PartialEq<other>
     if !arg_ty
-        .builtin_deref(true, ty::LvaluePreference::NoPreference)
+        .builtin_deref(true)
         .map_or(false, |tam| implements_trait(cx, tam.ty, partial_eq_trait_id, &[other_ty]))
         // arg impls PartialEq<*other>
         && !other_ty
-        .builtin_deref(true, ty::LvaluePreference::NoPreference)
+        .builtin_deref(true)
         .map_or(false, |tam| implements_trait(cx, arg_ty, partial_eq_trait_id, &[tam.ty]))
         // arg impls PartialEq<other>
         && !implements_trait(cx, arg_ty, partial_eq_trait_id, &[other_ty])

tests/ui/double_comparison.rs

@@ -0,0 +1,28 @@
+fn main() {
+    let x = 1;
+    let y = 2;
+    if x == y || x < y {
+        // do something
+    }
+    if x < y || x == y {
+        // do something
+    }
+    if x == y || x > y {
+        // do something
+    }
+    if x > y || x == y {
+        // do something
+    }
+    if x < y || x > y {
+        // do something
+    }
+    if x > y || x < y {
+        // do something
+    }
+    if x <= y && x >= y {
+        // do something
+    }
+    if x >= y && x <= y {
+        // do something
+    }
+}

tests/ui/double_comparison.stderr

@@ -0,0 +1,52 @@
+error: This binary expression can be simplified
+ --> $DIR/double_comparison.rs:4:8
+  |
+4 |     if x == y || x < y {
+  |        ^^^^^^^^^^^^^^^ help: try: `x <= y`
+  |
+  = note: #[deny(double_comparisons)] on by default
+
+error: This binary expression can be simplified
+ --> $DIR/double_comparison.rs:7:8
+  |
+7 |     if x < y || x == y {
+  |        ^^^^^^^^^^^^^^^ help: try: `x <= y`
+
+error: This binary expression can be simplified
+  --> $DIR/double_comparison.rs:10:8
+   |
+10 |     if x == y || x > y {
+   |        ^^^^^^^^^^^^^^^ help: try: `x >= y`
+
+error: This binary expression can be simplified
+  --> $DIR/double_comparison.rs:13:8
+   |
+13 |     if x > y || x == y {
+   |        ^^^^^^^^^^^^^^^ help: try: `x >= y`
+
+error: This binary expression can be simplified
+  --> $DIR/double_comparison.rs:16:8
+   |
+16 |     if x < y || x > y {
+   |        ^^^^^^^^^^^^^^ help: try: `x != y`
+
+error: This binary expression can be simplified
+  --> $DIR/double_comparison.rs:19:8
+   |
+19 |     if x > y || x < y {
+   |        ^^^^^^^^^^^^^^ help: try: `x != y`
+
+error: This binary expression can be simplified
+  --> $DIR/double_comparison.rs:22:8
+   |
+22 |     if x <= y && x >= y {
+   |        ^^^^^^^^^^^^^^^^ help: try: `x == y`
+
+error: This binary expression can be simplified
+  --> $DIR/double_comparison.rs:25:8
+   |
+25 |     if x >= y && x <= y {
+   |        ^^^^^^^^^^^^^^^^ help: try: `x == y`
+
+error: aborting due to 8 previous errors
+