EqualsAvoidsNull should not change compareTo order

Fixes #442
openrewrite · Jan 13, 2025 · 648b69f · 648b69f
1 parent 3609cd7
commit 648b69f
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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"));
+                  }
+              }
+              """
+          )
+        );
+    }
 }