Auto merge of rust-lang#5596 - ebroto:issue_5212, r=phansch

Fix comparison_chain false positive

changelog: comparison_chain: fix false positives when the binary operation is the same.

Fixes rust-lang#5212

Author: bors

clippy_lints/src/comparison_chain.rs

@@ -81,12 +81,23 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ComparisonChain {
 
                 // Check that both sets of operands are equal
                 let mut spanless_eq = SpanlessEq::new(cx);
-                if (!spanless_eq.eq_expr(lhs1, lhs2) || !spanless_eq.eq_expr(rhs1, rhs2))
-                    && (!spanless_eq.eq_expr(lhs1, rhs2) || !spanless_eq.eq_expr(rhs1, lhs2))
-                {
+                let same_fixed_operands = spanless_eq.eq_expr(lhs1, lhs2) && spanless_eq.eq_expr(rhs1, rhs2);
+                let same_transposed_operands = spanless_eq.eq_expr(lhs1, rhs2) && spanless_eq.eq_expr(rhs1, lhs2);
+
+                if !same_fixed_operands && !same_transposed_operands {
                     return;
                 }
 
+                // Check that if the operation is the same, either it's not `==` or the operands are transposed
+                if kind1.node == kind2.node {
+                    if kind1.node == BinOpKind::Eq {
+                        return;
+                    }
+                    if !same_transposed_operands {
+                        return;
+                    }
+                }
+
                 // Check that the type being compared implements `core::cmp::Ord`
                 let ty = cx.tables.expr_ty(lhs1);
                 let is_ord = get_trait_def_id(cx, &paths::ORD).map_or(false, |id| implements_trait(cx, ty, id, &[]));

tests/ui/comparison_chain.rs

@@ -137,4 +137,70 @@ fn h<T: Ord>(x: T, y: T, z: T) {
     }
 }
 
+// The following uses should be ignored
+mod issue_5212 {
+    use super::{a, b, c};
+    fn foo() -> u8 {
+        21
+    }
+
+    fn same_operation_equals() {
+        // operands are fixed
+
+        if foo() == 42 {
+            a()
+        } else if foo() == 42 {
+            b()
+        }
+
+        if foo() == 42 {
+            a()
+        } else if foo() == 42 {
+            b()
+        } else {
+            c()
+        }
+
+        // operands are transposed
+
+        if foo() == 42 {
+            a()
+        } else if 42 == foo() {
+            b()
+        }
+    }
+
+    fn same_operation_not_equals() {
+        // operands are fixed
+
+        if foo() > 42 {
+            a()
+        } else if foo() > 42 {
+            b()
+        }
+
+        if foo() > 42 {
+            a()
+        } else if foo() > 42 {
+            b()
+        } else {
+            c()
+        }
+
+        if foo() < 42 {
+            a()
+        } else if foo() < 42 {
+            b()
+        }
+
+        if foo() < 42 {
+            a()
+        } else if foo() < 42 {
+            b()
+        } else {
+            c()
+        }
+    }
+}
+
 fn main() {}