Catch corner case in Java TabsAndIndentsVisitor

In case a method's return type and method name contain the type and name of the method's first parameter (as in `public String foobar(String foo, String bar)`), the parameter alignment formatting could go wrong.
openrewrite · Jan 22, 2025 · 2aadb97 · 2aadb97
1 parent 1f4956b
commit 2aadb97
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 14 deletions.
diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/format/TabsAndIndentsTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/format/TabsAndIndentsTest.java
@@ -173,6 +173,31 @@ private void firstArgOnNewLine(
         );
     }
 
+    @Test
+    void firstParameterNameConflictWithReturnTypeAndMethodName() {
+        rewriteRun(
+          tabsAndIndents(style -> style.withMethodDeclarationParameters(new TabsAndIndentsStyle.MethodDeclarationParameters(true))),
+          java(
+            """
+              class Test {
+                  private String first(String first,
+                                                int times,
+                                                String third) {
+                  }
+              }
+              """,
+            """
+              class Test {
+                  private String first(String first,
+                                       int times,
+                                       String third) {
+                  }
+              }
+              """
+          )
+        );
+    }
+
     @Test
     void alignMethodDeclarationParamsWhenContinuationIndentUsingTabs() {
         rewriteRun(

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/format/TabsAndIndentsVisitor.java b/rewrite-java/src/main/java/org/openrewrite/java/format/TabsAndIndentsVisitor.java
@@ -17,15 +17,19 @@
 
 import org.jspecify.annotations.Nullable;
 import org.openrewrite.Cursor;
+import org.openrewrite.PrintOutputCapture;
 import org.openrewrite.Tree;
+import org.openrewrite.TreeVisitor;
 import org.openrewrite.internal.ListUtils;
+import org.openrewrite.internal.RecipeRunException;
 import org.openrewrite.internal.StringUtils;
 import org.openrewrite.java.JavaIsoVisitor;
 import org.openrewrite.java.style.TabsAndIndentsStyle;
 import org.openrewrite.java.tree.*;
 
 import java.util.Iterator;
 import java.util.List;
+import java.util.concurrent.CancellationException;
 
 public class TabsAndIndentsVisitor<P> extends JavaIsoVisitor<P> {
     @Nullable
@@ -226,30 +230,27 @@ public Space visitSpace(Space space, Space.Location loc, P p) {
                         }
                         JContainer<J> container = getCursor().getParentOrThrow().getValue();
                         List<J> elements = container.getElements();
-                        J firstArg = elements.iterator().next();
                         J lastArg = elements.get(elements.size() - 1);
-                        if (style.getMethodDeclarationParameters().getAlignWhenMultiple()) {
+                        if (elements.size() > 1 && style.getMethodDeclarationParameters().getAlignWhenMultiple()) {
                             J.MethodDeclaration method = getCursor().firstEnclosing(J.MethodDeclaration.class);
                             if (method != null) {
-                                int alignTo;
-                                if (firstArg.getPrefix().getLastWhitespace().contains("\n")) {
-                                    alignTo = getLengthOfWhitespace(firstArg.getPrefix().getLastWhitespace());
+                                int alignTo = computeFirstParameterColumn(method);
+                                if (alignTo != -1) {
+                                    getCursor().getParentOrThrow().putMessage("lastIndent", alignTo - style.getContinuationIndent());
+                                    elem = visitAndCast(elem, p);
+                                    getCursor().getParentOrThrow().putMessage("lastIndent", indent);
+                                    after = indentTo(right.getAfter(), t == lastArg ? indent : alignTo, loc.getAfterLocation());
                                 } else {
-                                    String source = method.print(getCursor());
-                                    int firstArgIndex = source.indexOf(firstArg.print(getCursor()));
-                                    int lineBreakIndex = source.lastIndexOf('\n', firstArgIndex);
-                                    alignTo = (firstArgIndex - (lineBreakIndex == -1 ? 0 : lineBreakIndex)) - 1;
+                                    after = right.getAfter();
                                 }
-                                getCursor().getParentOrThrow().putMessage("lastIndent", alignTo - style.getContinuationIndent());
-                                elem = visitAndCast(elem, p);
-                                getCursor().getParentOrThrow().putMessage("lastIndent", indent);
-                                after = indentTo(right.getAfter(), t == lastArg ? indent : alignTo, loc.getAfterLocation());
                             } else {
                                 after = right.getAfter();
                             }
-                        } else {
+                        } else if (elements.size() > 1) {
                             elem = visitAndCast(elem, p);
                             after = indentTo(right.getAfter(), t == lastArg ? indent : style.getContinuationIndent(), loc.getAfterLocation());
+                        } else {
+                            after = right.getAfter();
                         }
                         break;
                     }
@@ -353,6 +354,37 @@ public Space visitSpace(Space space, Space.Location loc, P p) {
         return (after == right.getAfter() && t == right.getElement()) ? right : new JRightPadded<>(t, after, right.getMarkers());
     }
 
+    private int computeFirstParameterColumn(J.MethodDeclaration method) {
+        List<JRightPadded<Statement>> arguments = method.getPadding().getParameters().getPadding().getElements();
+        J firstArg = arguments.isEmpty() ? null : arguments.get(0).getElement();
+        if (firstArg == null || firstArg instanceof J.Empty) {
+            return -1;
+        }
+
+        if (firstArg.getPrefix().getLastWhitespace().contains("\n")) {
+            return getLengthOfWhitespace(firstArg.getPrefix().getLastWhitespace());
+        } else {
+            TreeVisitor<?, PrintOutputCapture<TreeVisitor<?, ?>>> printer = method.printer(getCursor());
+            PrintOutputCapture<TreeVisitor<?, ?>> capture = new PrintOutputCapture<TreeVisitor<?, ?>>(printer) {
+                @Override
+                public PrintOutputCapture<TreeVisitor<?, ?>> append(@Nullable String text) {
+                    if (getContext().getCursor().getValue() == firstArg) {
+                        throw new CancellationException();
+                    }
+                    return super.append(text);
+                }
+            };
+            try {
+                printer.visit(method, capture, getCursor().getParentOrThrow());
+                throw new IllegalStateException();
+            } catch (RecipeRunException e) {
+                String out = capture.getOut();
+                int lineBreakIndex = out.lastIndexOf('\n');
+                return (out.length() - (lineBreakIndex == -1 ? 0 : lineBreakIndex)) - 1;
+            }
+        }
+    }
+
     @Override
     public <J2 extends J> @Nullable JContainer<J2> visitContainer(@Nullable JContainer<J2> container, JContainer.Location loc, P p) {
         if (container == null) {