No longer remove String.valueof when it's called on a method invocation (#752)

* Fix CSA issue

* Explain the need to exclude method invocations for now

---------

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

Author: Laurens-W

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

@@ -70,7 +70,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx)
                 J.MethodInvocation mi = (J.MethodInvocation) super.visitMethodInvocation(method, ctx);
                 if (VALUE_OF.matches(mi) && mi.getArguments().size() == 1) {
                     Expression argument = mi.getArguments().get(0);
-                    if (TypeUtils.isString(argument.getType()) || removeValueOfForStringConcatenation(argument)) {
+                    if ((TypeUtils.isString(argument.getType()) && !(argument instanceof J.MethodInvocation)) || removeValueOfForStringConcatenation(argument)) {
                         return maybeParenthesize(argument.withPrefix(mi.getPrefix()), updateCursor(mi));
                     }
                 }

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

@@ -249,24 +249,22 @@ void valueOfOnMethodInvocation() {
           java(
             """
               class Test {
-                  static void method1() {
+                  static void unnecessary() {
                       String a = String.valueOf(method2());
                   }
 
-                  static String method2() {
-                      return "";
-                  }
-              }
-              """,
-            """
-              class Test {
-                  static void method1() {
-                      String a = method2();
+                  static void necessary() {
+                      // `"null"` with `String.valueOf` vs `null` without
+                      String b = String.valueOf(method3());
                   }
 
                   static String method2() {
                       return "";
                   }
+
+                  static String method3() {
+                      return null;
+                  }
               }
               """
           )
@@ -296,4 +294,21 @@ static void method(int i) {
           )
         );
     }
+
+    @Test
+    void doNotRemoveValueOfForNullableStrings() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class Test {
+
+                  String method(Object some) {
+                      return String.valueOf(some.toString());
+                  }
+              }
+              """
+          )
+        );
+    }
 }

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

@@ -417,4 +417,28 @@ boolean notOne(A a) {
             )
         );
     }
+
+    @Test
+    void correctlySimplifyNegatedTernaryEqualsNull() {
+        rewriteRun(
+          java(
+            """
+              class A {
+                  void doSome(String o1, String o2) {
+                      if (!(o1 == null ? o2 == null : o1.equals(o2))) {
+                      }
+                  }
+              }
+              """,
+            """
+              class A {
+                  void doSome(String o1, String o2) {
+                      if (o1 == null ? o2 != null : !o1.equals(o2)) {
+                      }
+                  }
+              }
+              """
+          )
+        );
+    }
 }