PMD: LiteralsFirstInComparisons for compareTo* and contentEquals

src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java

@@ -17,7 +17,6 @@
 
 import lombok.EqualsAndHashCode;
 import lombok.Value;
-import org.jspecify.annotations.Nullable;
 import org.openrewrite.Tree;
 import org.openrewrite.java.JavaVisitor;
 import org.openrewrite.java.MethodMatcher;
@@ -27,11 +26,29 @@
 
 import static java.util.Collections.singletonList;
 
+/**
+ * A visitor that identifies and addresses potential issues related to
+ * the use of {@code equals} methods in Java, particularly to avoid
+ * null pointer exceptions when comparing strings.
+ * <p>
+ * This visitor looks for method invocations of {@code equals},
+ * {@code equalsIgnoreCase}, {@code compareTo}, and {@code contentEquals},
+ * and performs optimizations to ensure null checks are correctly applied.
+ * <p>
+ * For more details, refer to the PMD best practices:
+ * <a href="https://pmd.github.io/pmd/pmd_rules_java_bestpractices.html#LiteralsFirstInComparisons">Literals First in Comparisons</a>
+ *
+ * @param <P> The type of the parent context used for visiting the AST.
+ */
 @Value
 @EqualsAndHashCode(callSuper = false)
 public class EqualsAvoidsNullVisitor<P> extends JavaVisitor<P> {
-    private static final MethodMatcher STRING_EQUALS = new MethodMatcher("String equals(java.lang.Object)");
-    private static final MethodMatcher STRING_EQUALS_IGNORE_CASE = new MethodMatcher("String equalsIgnoreCase(java.lang.String)");
+
+    MethodMatcher EQUALS = new MethodMatcher("java.lang.String equals(java.lang.Object)");
+    MethodMatcher EQUALS_IGNORE_CASE = new MethodMatcher("java.lang.String equalsIgnoreCase(java.lang.String)");
+    MethodMatcher COMPARE_TO = new MethodMatcher("java.lang.String compareTo(java.lang.String)");
+    MethodMatcher COMPARE_TO_IGNORE_CASE = new MethodMatcher("java.lang.String compareToIgnoreCase(java.lang.String)");
+    MethodMatcher CONTENT_EQUALS = new MethodMatcher("java.lang.String contentEquals(java.lang.CharSequence)");
 
     EqualsAvoidsNullStyle style;
 
@@ -45,34 +62,39 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) {
         if (m.getSelect() == null) {
             return m;
         }
-
-        if ((STRING_EQUALS.matches(m) || (!Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) && STRING_EQUALS_IGNORE_CASE.matches(m))) &&
-            m.getArguments().get(0) instanceof J.Literal &&
-            !(m.getSelect() instanceof J.Literal)) {
-            Tree parent = getCursor().getParentTreeCursor().getValue();
+        if (!(m.getSelect() instanceof J.Literal)
+                && m.getArguments().get(0) instanceof J.Literal
+                && (EQUALS.matches(m)
+                || !style.getIgnoreEqualsIgnoreCase()
+                && EQUALS_IGNORE_CASE.matches(m)
+                || COMPARE_TO.matches(m)
+                || COMPARE_TO_IGNORE_CASE.matches(m)
+                || CONTENT_EQUALS.matches(m))) {
+            final Object parent = getCursor().getParentTreeCursor().getValue();
             if (parent instanceof J.Binary) {
-                J.Binary binary = (J.Binary) parent;
-                if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof J.Binary) {
-                    J.Binary potentialNullCheck = (J.Binary) binary.getLeft();
-                    if ((isNullLiteral(potentialNullCheck.getLeft()) && matchesSelect(potentialNullCheck.getRight(), m.getSelect())) ||
-                        (isNullLiteral(potentialNullCheck.getRight()) && matchesSelect(potentialNullCheck.getLeft(), m.getSelect()))) {
+                final J.Binary binary = (J.Binary) parent;
+                if (binary.getLeft() instanceof J.Binary
+                        && binary.getOperator() == J.Binary.Type.And) {
+                    final J.Binary potentialNullCheck = (J.Binary) binary.getLeft();
+                    if (isNullLiteral(potentialNullCheck.getLeft())
+                            && matchesSelect(potentialNullCheck.getRight(), m.getSelect())
+                            || isNullLiteral(potentialNullCheck.getRight())
+                            && matchesSelect(potentialNullCheck.getLeft(), m.getSelect())) {
                         doAfterVisit(new RemoveUnnecessaryNullCheck<>(binary));
                     }
                 }
             }
-
             if (m.getArguments().get(0).getType() == JavaType.Primitive.Null) {
                 return new J.Binary(Tree.randomId(), m.getPrefix(), Markers.EMPTY,
                         m.getSelect(),
                         JLeftPadded.build(J.Binary.Type.Equal).withBefore(Space.SINGLE_SPACE),
                         m.getArguments().get(0).withPrefix(Space.SINGLE_SPACE),
                         JavaType.Primitive.Boolean);
             } else {
-                m = m.withSelect(((J.Literal) m.getArguments().get(0)).withPrefix(m.getSelect().getPrefix()))
+                return m.withSelect(m.getArguments().get(0).withPrefix(m.getSelect().getPrefix()))
                         .withArguments(singletonList(m.getSelect().withPrefix(Space.EMPTY)));
             }
         }
-
         return m;
     }
 
@@ -88,14 +110,6 @@ private static class RemoveUnnecessaryNullCheck<P> extends JavaVisitor<P> {
         private final J.Binary scope;
         boolean done;
 
-        @Override
-        public @Nullable J visit(@Nullable Tree tree, P p) {
-            if (done) {
-                return (J) tree;
-            }
-            return super.visit(tree, p);
-        }
-
         public RemoveUnnecessaryNullCheck(J.Binary scope) {
             this.scope = scope;
         }
@@ -106,7 +120,6 @@ public J visitBinary(J.Binary binary, P p) {
                 done = true;
                 return binary.getRight().withPrefix(Space.EMPTY);
             }
-
             return super.visitBinary(binary, p);
         }
     }

src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullTest.java

@@ -39,18 +39,24 @@ void invertConditional() {
             """
               public class A {
                   {
-                      String s = null;
-                      if(s.equals("test")) {}
-                      if(s.equalsIgnoreCase("test")) {}
+                      String s = "LiteralsFirstInComparisons";
+                      System.out.println(s.compareTo("LiteralsFirstInComparisons"));
+                      System.out.println(s.compareToIgnoreCase("LiteralsFirstInComparisons"));
+                      System.out.println(s.contentEquals("LiteralsFirstInComparisons"));
+                      System.out.println(s.equals("LiteralsFirstInComparisons"));
+                      System.out.println(s.equalsIgnoreCase("LiteralsFirstInComparisons"));
                   }
               }
               """,
             """
               public class A {
                   {
-                      String s = null;
-                      if("test".equals(s)) {}
-                      if("test".equalsIgnoreCase(s)) {}
+                      String s = "LiteralsFirstInComparisons";
+                      System.out.println("LiteralsFirstInComparisons".compareTo(s));
+                      System.out.println("LiteralsFirstInComparisons".compareToIgnoreCase(s));
+                      System.out.println("LiteralsFirstInComparisons".contentEquals(s));
+                      System.out.println("LiteralsFirstInComparisons".equals(s));
+                      System.out.println("LiteralsFirstInComparisons".equalsIgnoreCase(s));
                   }
               }
               """
@@ -67,17 +73,17 @@ void removeUnnecessaryNullCheck() {
               public class A {
                   {
                       String s = null;
-                      if(s != null && s.equals("test")) {}
-                      if(null != s && s.equals("test")) {}
+                      if(s != null && s.equals("LiteralsFirstInComparisons")) {}
+                      if(null != s && s.equals("LiteralsFirstInComparisons")) {}
                   }
               }
               """,
             """
               public class A {
                   {
                       String s = null;
-                      if("test".equals(s)) {}
-                      if("test".equals(s)) {}
+                      if("LiteralsFirstInComparisons".equals(s)) {}
+                      if("LiteralsFirstInComparisons".equals(s)) {}
                   }
               }
               """
@@ -88,17 +94,17 @@ public class A {
     @Test
     void nullLiteral() {
         rewriteRun(
-            //language=java
-            java("""
+          //language=java
+          java("""
               public class A {
                     void foo(String s) {
                         if(s.equals(null)) {
                         }
                     }
                 }
               """,
-              """
-
+            """
+              
               public class A {
                     void foo(String s) {
                         if(s == null) {