Avoid unsafe changes in JulParameterizedArguments when JavaType.Array argument is not a J.NewArray (#244)

pdelagrave · timtebeek · web-flow · commit aa3f14f59634 · 2025-08-04T17:54:25.000+02:00
* support migrating `JUL.log("{0}", arrayIdentifier)` to slf4j

* applied multiple review suggestions

* Use static import for Collections

* Apply suggestions from code review

* Revert the array unpacking improvement

First time testing for a negative case shows that the recipe isn't producing valid code by itself and that `org.openrewrite.java.logging.slf4j.JulToSlf4j` isn't either. Fixed in the next commit.

* Skip `JulParameterizedArguments` to avoid unsafe change there

---------

Co-authored-by: Tim te Beek &lt;tim@moderne.io&gt;
diff --git a/src/main/java/org/openrewrite/java/logging/slf4j/JulParameterizedArguments.java b/src/main/java/org/openrewrite/java/logging/slf4j/JulParameterizedArguments.java
@@ -92,6 +92,11 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
                 Expression messageArgument = originalArguments.get(1);
                 Expression stringFormatArgument = originalArguments.get(2);
 
+                if (stringFormatArgument.getType() instanceof JavaType.Array &&
+                        !(stringFormatArgument instanceof J.NewArray)) {
+                    return method;
+                }
+
                 if (!(levelArgument instanceof J.FieldAccess || levelArgument instanceof J.Identifier) ||
                         !isStringLiteral(messageArgument)) {
                     return method;
diff --git a/src/test/java/org/openrewrite/java/logging/slf4j/JulParameterizedArgumentsTest.java b/src/test/java/org/openrewrite/java/logging/slf4j/JulParameterizedArgumentsTest.java
@@ -15,8 +15,10 @@
  */
 package org.openrewrite.java.logging.slf4j;
 
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.openrewrite.DocumentExample;
+import org.openrewrite.Issue;
 import org.openrewrite.test.RecipeSpec;
 import org.openrewrite.test.RewriteTest;
 
@@ -88,6 +90,34 @@ void method(Logger logger, String param1, String param2) {
         );
     }
 
+    @Test
+    void parameterizedArgumentArrayWithNoInitializer() {
+        rewriteRun(
+          // language=java
+          java(
+            """
+              import java.util.logging.Level;
+              import java.util.logging.Logger;
+
+              class Test {
+                  void method(Logger logger) {
+                      logger.log(Level.INFO, "INFO Log entry", new String[]{});
+                  }
+              }
+              """,
+            """
+              import org.slf4j.Logger;
+
+              class Test {
+                  void method(Logger logger) {
+                      logger.info("INFO Log entry");
+                  }
+              }
+              """
+          )
+        );
+    }
+
     @Test
     void retainLoggedArgumentOrder() {
         rewriteRun(
@@ -116,6 +146,28 @@ void method(Logger logger, String param1, String param2) {
         );
     }
 
+    @Disabled("Skipped by `JulParameterizedArguments`, but incomplete changes seen from JUL -> Log4j -> Slf4j")
+    @Issue("https://github.com/openrewrite/rewrite-logging-frameworks/pull/244#issuecomment-3140661425")
+    @Test
+    void arrayIdentifierArgument() {
+        rewriteRun(
+          // language=java
+          java(
+            """
+              import java.util.logging.Level;
+              import java.util.logging.Logger;
+
+              class Test {
+                  void method(Logger logger, String[] params) {
+                      logger.log(Level.INFO, "INFO Log entry, param2: {1}, param1: {0}, etc", params);
+                      logger.log(Level.INFO, "INFO Log entry, param1: {0}, param2: {1}, etc", params);
+                  }
+              }
+              """
+          )
+        );
+    }
+
     @Test
     void repeatLoggedArgumentAsNeeded() {
         rewriteRun(
