StringFormatted should not leave trailing whitespace after replacement (

#556) * issue-453: make First Argument Prefix As Empty for StringFormatted * issue-453: reformat class * Restore original formatting * Apply formatter and rename variable for clarity * Shorten code change by dropping method * Minimize diff * Inline excessive local arguments; drop unnecessary type hint --------- Co-authored-by: Tim te Beek <[email protected]>
openrewrite · Sep 15, 2024 · b5ab237 · b5ab237
1 parent 544581f
commit b5ab237
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 25 deletions.
diff --git a/src/main/java/org/openrewrite/java/migrate/lang/StringFormatted.java b/src/main/java/org/openrewrite/java/migrate/lang/StringFormatted.java
@@ -59,41 +59,39 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
 
     private static class StringFormattedVisitor extends JavaVisitor<ExecutionContext> {
         @Override
-        public J visitMethodInvocation(J.MethodInvocation m, ExecutionContext ctx) {
-            m = (J.MethodInvocation) super.visitMethodInvocation(m, ctx);
-            if (!STRING_FORMAT.matches(m) || m.getMethodType() == null) {
-                return m;
+        public J visitMethodInvocation(J.MethodInvocation methodInvocation, ExecutionContext ctx) {
+            methodInvocation = (J.MethodInvocation) super.visitMethodInvocation(methodInvocation, ctx);
+            if (!STRING_FORMAT.matches(methodInvocation) || methodInvocation.getMethodType() == null) {
+                return methodInvocation;
             }
 
-            List<Expression> arguments = m.getArguments();
-            boolean wrapperNotNeeded = wrapperNotNeeded(arguments.get(0));
             maybeRemoveImport("java.lang.String.format");
-            J.MethodInvocation mi = m.withName(m.getName().withSimpleName("formatted"));
-            JavaType.Method formatted = m.getMethodType().getDeclaringType().getMethods().stream()
+            J.MethodInvocation mi = methodInvocation.withName(methodInvocation.getName().withSimpleName("formatted"));
+            mi = mi.withMethodType(methodInvocation.getMethodType().getDeclaringType().getMethods().stream()
                     .filter(it -> it.getName().equals("formatted"))
                     .findAny()
-                    .orElse(null);
-            mi = mi.withMethodType(formatted);
+                    .orElse(null));
             if (mi.getName().getType() != null) {
                 mi = mi.withName(mi.getName().withType(mi.getMethodType()));
             }
-            Expression select = wrapperNotNeeded ? arguments.get(0) :
-                new J.Parentheses<>(randomId(), Space.EMPTY, Markers.EMPTY, JRightPadded.build(arguments.get(0)));
-            mi = mi.withSelect(select);
+            List<Expression> arguments = methodInvocation.getArguments();
+            mi = mi.withSelect(wrapperNotNeeded(arguments.get(0)) ? arguments.get(0).withPrefix(Space.EMPTY) :
+                    new J.Parentheses<>(randomId(), Space.EMPTY, Markers.EMPTY,
+                            JRightPadded.build(arguments.get(0))));
             mi = mi.withArguments(arguments.subList(1, arguments.size()));
-            if(mi.getArguments().isEmpty()) {
+            if (mi.getArguments().isEmpty()) {
                 // To store spaces between the parenthesis of a method invocation argument list
                 // Ensures formatting recipes chained together with this one will still work as expected
                 mi = mi.withArguments(singletonList(new J.Empty(randomId(), Space.EMPTY, Markers.EMPTY)));
             }
-            return maybeAutoFormat(m, mi, ctx);
+            return maybeAutoFormat(methodInvocation, mi, ctx);
         }
 
         private static boolean wrapperNotNeeded(Expression expression) {
             return expression instanceof J.Identifier
-                    || expression instanceof J.Literal
-                    || expression instanceof J.MethodInvocation
-                    || expression instanceof J.FieldAccess;
+                   || expression instanceof J.Literal
+                   || expression instanceof J.MethodInvocation
+                   || expression instanceof J.FieldAccess;
         }
     }
 

diff --git a/src/test/java/org/openrewrite/java/migrate/lang/StringFormattedTest.java b/src/test/java/org/openrewrite/java/migrate/lang/StringFormattedTest.java
@@ -19,7 +19,6 @@
 import org.openrewrite.Issue;
 import org.openrewrite.test.RecipeSpec;
 import org.openrewrite.test.RewriteTest;
-import org.openrewrite.test.TypeValidation;
 
 import static org.openrewrite.java.Assertions.java;
 import static org.openrewrite.java.Assertions.version;
@@ -28,8 +27,7 @@
 class StringFormattedTest implements RewriteTest {
     @Override
     public void defaults(RecipeSpec spec) {
-        spec.recipe(new StringFormatted())
-          .typeValidationOptions(new TypeValidation().methodInvocations(false));
+        spec.recipe(new StringFormatted());
     }
 
     @Test
@@ -94,21 +92,21 @@ void callingFunction() {
             java(
               """
                 package com.example.app;
-
+                
                 class A {
                     String str = String.format(getTemplateString(), "a");
-
+                
                     private String getTemplateString() {
                         return "foo %s";
                     }
                 }
                 """,
               """
                 package com.example.app;
-
+                
                 class A {
                     String str = getTemplateString().formatted("a");
-
+                
                     private String getTemplateString() {
                         return "foo %s";
                     }
@@ -477,4 +475,32 @@ class A {
           )
         );
     }
+
+    @Test
+    @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/453")
+    void doNotRetainTrailingWhitespaceWhenFirstArgumentOnNewLine() {
+        //language=java
+        rewriteRun(
+          version(
+            java(
+              """
+                class A {
+                    String str = String.format(
+                    "foo %s %s",
+                        "a",
+                        "b");
+                }
+                """,
+              """
+                class A {
+                    String str = "foo %s %s".formatted(
+                            "a",
+                            "b");
+                }
+                """
+            ),
+            17
+          )
+        );
+    }
 }