Do not rewrite String.replaceAll with special chars in replacement st…

…ring (#306)

* Do not rewrite String.replaceAll with special chars in replacement string

If the replacement string of String.replaceAll contains $ or \, we
should not rewrite it as these indicate special replacements:
https://docs.oracle.com/en/java/javase/22/docs/api/java.base/java/util/regex/Matcher.html#replaceAll(java.lang.String)

Fixes #301

* Only replace if second argument is also a literal

---------

Co-authored-by: Sam Snyder <[email protected]>
Co-authored-by: Tim te Beek <[email protected]>

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

@@ -52,7 +52,7 @@ public String getDisplayName() {
     @Override
     public String getDescription() {
         return "When `String::replaceAll` is used, the first argument should be a real regular expression. " +
-                "If it’s not the case, `String::replace` does exactly the same thing as `String::replaceAll` without the performance drawback of the regex.";
+               "If it’s not the case, `String::replace` does exactly the same thing as `String::replaceAll` without the performance drawback of the regex.";
     }
 
     @Override
@@ -81,19 +81,27 @@ private static class UseStringReplaceVisitor extends JavaVisitor<ExecutionContex
         public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
             J.MethodInvocation invocation = (J.MethodInvocation) super.visitMethodInvocation(method, ctx);
 
-            //Checks if method invocation matches with String#replaceAll
+            // Checks if method invocation matches with String#replaceAll
             if (REPLACE_ALL.matches(invocation)) {
-                Expression firstArgument = invocation.getArguments().get(0);
+                // Checks if the second argument is a string literal with $ or \ in it as this has special meaning
+                // https://docs.oracle.com/en/java/javase/22/docs/api/java.base/java/util/regex/Matcher.html#replaceAll(java.lang.String)
+                Expression secondArgument = invocation.getArguments().get(1);
+                if (!isStringLiteral(secondArgument)) {
+                    return invocation; // Might contain special characters; unsafe to replace
+                }
+                String secondValue = (String) ((J.Literal) secondArgument).getValue();
+                if (Objects.nonNull(secondValue) && (secondValue.contains("$") || secondValue.contains("\\"))) {
+                    return invocation; // Does contain special characters; unsafe to replace
+                }
 
-                //Checks if the first argument is a String literal
+                // Checks if the first argument is a String literal
+                Expression firstArgument = invocation.getArguments().get(0);
                 if (isStringLiteral(firstArgument)) {
-                    J.Literal literal = (J.Literal) firstArgument;
-                    String value = (String) literal.getValue();
-
-                    //Checks if the String literal may not be a regular expression,
-                    //if so, then change the method invocation name
-                    if (Objects.nonNull(value) && !mayBeRegExp(value)) {
-                        String unEscapedLiteral = unEscapeCharacters(value);
+                    // Checks if the String literal may not be a regular expression,
+                    // if so, then change the method invocation name
+                    String firstValue = (String) ((J.Literal) firstArgument).getValue();
+                    if (Objects.nonNull(firstValue) && !mayBeRegExp(firstValue)) {
+                        String unEscapedLiteral = unEscapeCharacters(firstValue);
                         invocation = invocation
                                 .withName(invocation.getName().withSimpleName("replace"))
                                 .withArguments(ListUtils.mapFirst(invocation.getArguments(), arg -> ((J.Literal) arg)

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

@@ -146,4 +146,45 @@ public void method() {
           )
         );
     }
+
+    @Test
+    @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/301")
+    @DisplayName("String#replaceAll is not replaced by String#replace, because second argument has a backslash in it")
+    void replaceAllUnchangedIfBackslashInReplacementString() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class Test {
+                public String method() {
+                  return "abc".replaceAll("b", "\\\\\\\\\\\\\\\\");
+                }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/301")
+    @DisplayName("String#replaceAll is not replaced by String#replace, because second argument has a dollar sign in it")
+    void replaceAllUnchangedIfDollarInReplacementString() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class Test {
+                public String method1() {
+                  return "abc".replaceAll("b", "$0");
+                }
+
+                public String method2() {
+                  String s = "$0";
+                  return "abc".replaceAll("b", s);
+                }
+              }
+              """
+          )
+        );
+    }
 }