From 9217675c7f6ccd2efa18047ebb0c86841683b6a5 Mon Sep 17 00:00:00 2001
From: Eduardo Broto <ebroto@tutanota.com>
Date: Thu, 14 May 2020 00:26:09 +0200
Subject: [PATCH] Fix comparison_chain false positive

---
 clippy_lints/src/comparison_chain.rs | 17 +++++--
 tests/ui/comparison_chain.rs         | 66 ++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/clippy_lints/src/comparison_chain.rs b/clippy_lints/src/comparison_chain.rs
index 96df3ffe3ce..93e29edcaa5 100644
--- a/clippy_lints/src/comparison_chain.rs
+++ b/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, &[]));
diff --git a/tests/ui/comparison_chain.rs b/tests/ui/comparison_chain.rs
index 9c2128469de..3b03f8c7dfe 100644
--- a/tests/ui/comparison_chain.rs
+++ b/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() {}