From 648b69f0748f37d9fcbeb4d2c2663b8fb0f42a10 Mon Sep 17 00:00:00 2001
From: Tim te Beek <tim@moderne.io>
Date: Mon, 13 Jan 2025 21:38:00 +0100
Subject: [PATCH] EqualsAvoidsNull should not change `compareTo` order

Fixes #442
---
 .../staticanalysis/EqualsAvoidsNull.java      |  5 +--
 .../EqualsAvoidsNullVisitor.java              |  7 +----
 .../staticanalysis/EqualsAvoidsNullTest.java  | 31 ++++++++++++++-----
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNull.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNull.java
index 23c9f99e8..d866a0548 100644
--- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNull.java
+++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNull.java
@@ -70,10 +70,7 @@ public J visit(@Nullable Tree tree, ExecutionContext ctx) {
             }
         };
         return Preconditions.check(
-                Preconditions.or(
-                        new UsesMethod<>("java.lang.String equals*(..)"),
-                        new UsesMethod<>("java.lang.String co*(..)")
-                ),
+                new UsesMethod<>("java.lang.String *quals*(..)"),
                 replacementVisitor
         );
     }
diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java
index 072ece297..d2d35fb7b 100644
--- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java
+++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java
@@ -49,8 +49,6 @@ public class EqualsAvoidsNullVisitor<P> extends JavaVisitor<P> {
     private static final String JAVA_LANG_STRING = "java.lang.String ";
     private static final MethodMatcher EQUALS = new MethodMatcher(JAVA_LANG_STRING + "equals(java.lang.Object)");
     private static final MethodMatcher EQUALS_IGNORE_CASE = new MethodMatcher(JAVA_LANG_STRING + "equalsIgnoreCase(java.lang.String)");
-    private static final MethodMatcher COMPARE_TO = new MethodMatcher(JAVA_LANG_STRING + "compareTo(java.lang.String)");
-    private static final MethodMatcher COMPARE_TO_IGNORE_CASE = new MethodMatcher(JAVA_LANG_STRING + "compareToIgnoreCase(java.lang.String)");
     private static final MethodMatcher CONTENT_EQUALS = new MethodMatcher(JAVA_LANG_STRING + "contentEquals(java.lang.CharSequence)");
 
     EqualsAvoidsNullStyle style;
@@ -91,10 +89,7 @@ private boolean hasCompatibleArgument(J.MethodInvocation m) {
 
     private boolean isStringComparisonMethod(J.MethodInvocation methodInvocation) {
         return EQUALS.matches(methodInvocation) ||
-                !style.getIgnoreEqualsIgnoreCase() &&
-                        EQUALS_IGNORE_CASE.matches(methodInvocation) ||
-                COMPARE_TO.matches(methodInvocation) ||
-                COMPARE_TO_IGNORE_CASE.matches(methodInvocation) ||
+                (!style.getIgnoreEqualsIgnoreCase() && EQUALS_IGNORE_CASE.matches(methodInvocation)) ||
                 CONTENT_EQUALS.matches(methodInvocation);
     }
 
diff --git a/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullTest.java b/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullTest.java
index b7d1cf64e..47e95519f 100644
--- a/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullTest.java
+++ b/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullTest.java
@@ -46,8 +46,6 @@ public class A {
                       String s = null;
                       if(s.equals("test")) {}
                       if(s.equalsIgnoreCase("test")) {}
-                      System.out.println(s.compareTo("test"));
-                      System.out.println(s.compareToIgnoreCase("test"));
                       System.out.println(s.contentEquals("test"));
                   }
               }
@@ -58,8 +56,6 @@ public class A {
                       String s = null;
                       if("test".equals(s)) {}
                       if("test".equalsIgnoreCase(s)) {}
-                      System.out.println("test".compareTo(s));
-                      System.out.println("test".compareToIgnoreCase(s));
                       System.out.println("test".contentEquals(s));
                   }
               }
@@ -243,8 +239,8 @@ public class Constants {
                   }
                   class A {
                       private boolean isFoo(String foo, String bar) {
-                          return foo.contentEquals(Constants.FOO)
-                              || bar.compareToIgnoreCase(Constants.FOO);
+                          return foo.equals(Constants.FOO)
+                              || bar.contentEquals(Constants.FOO);
                       }
                   }
                   """,
@@ -254,8 +250,8 @@ public class Constants {
                   }
                   class A {
                       private boolean isFoo(String foo, String bar) {
-                          return Constants.FOO.contentEquals(foo)
-                              || Constants.FOO.compareToIgnoreCase(bar);
+                          return Constants.FOO.equals(foo)
+                              || Constants.FOO.contentEquals(bar);
                       }
                   }
                   """
@@ -436,4 +432,23 @@ boolean withParentExpression(String foo) {
             );
         }
     }
+
+    @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/442")
+    @Test
+    void retainCompareToAsToNotChangeOrder() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              public class A {
+                  {
+                      String s = null;
+                      System.out.println(s.compareTo("test"));
+                      System.out.println(s.compareToIgnoreCase("test"));
+                  }
+              }
+              """
+          )
+        );
+    }
 }