Should not use equals when comparing object identity (#274)

fixes #273 Co-authored-by: tiesjr <ties@jdriven.com>
openrewrite · Oct 20, 2022 · 5e832a5 · 5e832a5
1 parent 5fa4dce
commit 5e832a5
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 1 deletion.
diff --git a/src/main/java/org/openrewrite/java/testing/cleanup/AssertTrueComparisonToAssertEquals.java b/src/main/java/org/openrewrite/java/testing/cleanup/AssertTrueComparisonToAssertEquals.java
@@ -22,6 +22,7 @@
 import org.openrewrite.java.search.UsesMethod;
 import org.openrewrite.java.tree.Expression;
 import org.openrewrite.java.tree.J;
+import org.openrewrite.java.tree.JavaType;
 
 import java.util.function.Supplier;
 
@@ -106,7 +107,18 @@ private boolean isEqualBinary(J.MethodInvocation method) {
 
                 J.Binary binary = (J.Binary) firstArgument;
                 J.Binary.Type operator = binary.getOperator();
-                return operator.equals(J.Binary.Type.Equal);
+
+                if (!operator.equals(J.Binary.Type.Equal)) {
+                    return false;
+                }
+
+                // Prevent breaking identity comparison.
+                // Objects that are compared with == should not be compared with `.equals()` instead.
+                // Out of the primitives == is not allowed when both are of type String
+                return binary.getLeft().getType() instanceof JavaType.Primitive
+                        && binary.getRight().getType() instanceof JavaType.Primitive
+                        && !(binary.getLeft().getType() == JavaType.Primitive.String
+                        && binary.getRight().getType() == JavaType.Primitive.String);
             }
         };
     }

diff --git a/...est/kotlin/org/openrewrite/java/testing/cleanup/AssertTrueComparisonToAssertEqualsTest.kt b/...est/kotlin/org/openrewrite/java/testing/cleanup/AssertTrueComparisonToAssertEqualsTest.kt
@@ -112,4 +112,39 @@ class AssertTrueComparisonToAssertEqualsTest : JavaRecipeTest {
             }
         """,
     )
+
+    @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/273")
+    @Suppress("ConstantConditions", "SimplifiableAssertion")
+    @Test
+    fun doNotChangeToEqualsWhenCheckingOnObjectIdentityWithStrings() = assertUnchanged(
+        before = """
+        import org.junit.jupiter.api.Assertions;
+        
+        public class Test {
+            void test() {
+                String a = "a";
+                String b = "a";
+                Assertions.assertTrue(a == b);
+            }
+        }
+    """
+    )
+
+    @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/273")
+    @Suppress("ConstantConditions", "SimplifiableAssertion")
+    @Test
+    fun doNotChangeToEqualsWhenCheckingOnObjectIdentityWithObjects() = assertUnchanged(
+        before = """
+        import org.junit.jupiter.api.Assertions;
+        
+        public class Test {
+            void test() {
+                Object a = new Object();
+                Object b = a;
+                Assertions.assertTrue(a == b);
+            }
+        }
+    """
+    )
+
 }