From a689423d8fae63de9b3eba7a118c550523a6b1e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Kautler?= Date: Tue, 22 May 2018 19:46:38 +0200 Subject: [PATCH] Fix indentation realignment when wrapping call-chains --- .../javaparser/printer/PrettyPrinterTest.java | 69 ++++++---- .../printer/PrettyPrintVisitor.java | 124 ++++++++++++++---- .../printer/PrettyPrinterConfiguration.java | 2 +- .../javaparser/printer/SourcePrinter.java | 31 +++-- 4 files changed, 166 insertions(+), 60 deletions(-) diff --git a/javaparser-core-testing/src/test/java/com/github/javaparser/printer/PrettyPrinterTest.java b/javaparser-core-testing/src/test/java/com/github/javaparser/printer/PrettyPrinterTest.java index e1288091fe..b9171e7dff 100644 --- a/javaparser-core-testing/src/test/java/com/github/javaparser/printer/PrettyPrinterTest.java +++ b/javaparser-core-testing/src/test/java/com/github/javaparser/printer/PrettyPrinterTest.java @@ -216,20 +216,42 @@ public void printingInconsistentVariables() { @Test public void prettyAlignMethodCallChainsIndentsArgumentsWithBlocksCorrectly() { - CompilationUnit cu = JavaParser.parse("class Foo { void bar() { foo().bar().baz(() -> { boo().baa().bee(); }).bam(); } }"); - String printed = new PrettyPrinter(new PrettyPrinterConfiguration().setColumnAlignFirstMethodChain(true)) + CompilationUnit cu = JavaParser.parse("class Foo { void bar() { a.b.c.d.e; a.b.c().d().e(); a.b.c().d.e(); foo().bar().baz(boo().baa().bee()).bam(); foo().bar().baz(boo().baa().bee()).bam; foo().bar(Long.foo().b.bar(), bam).baz(); foo().bar().baz(foo, () -> { boo().baa().bee(); }).baz(() -> { boo().baa().bee(); }).bam(() -> { boo().baa().bee(); }); } }"); + String printed = new PrettyPrinter(new PrettyPrinterConfiguration().setColumnAlignFirstMethodChain(true).setColumnAlignParameters(true).setIndentSize(1).setIndentType(TABS_WITH_SPACE_ALIGN)) .print(cu); assertEqualsNoEol("class Foo {\n" + "\n" + - " void bar() {\n" + - " foo().bar()\n" + - " .baz(() -> {\n" + - " boo().baa()\n" + - " .bee();\n" + - " })\n" + - " .bam();\n" + - " }\n" + + "\tvoid bar() {\n" + + "\t\ta.b.c.d.e;\n" + + "\t\ta.b.c()\n" + + "\t\t .d()\n" + + "\t\t .e();\n" + + "\t\ta.b.c().d\n" + + "\t\t .e();\n" + + "\t\tfoo().bar()\n" + + "\t\t .baz(boo().baa().bee())\n" + + "\t\t .bam();\n" + + "\t\tfoo().bar()\n" + + "\t\t .baz(boo().baa().bee()).bam;\n" + + "\t\tfoo().bar(Long.foo().b.bar(),\n" + + "\t\t bam)\n" + + "\t\t .baz();\n" + + "\t\tfoo().bar()\n" + + "\t\t .baz(foo,\n" + + "\t\t () -> {\n" + + "\t\t \tboo().baa()\n" + + "\t\t \t .bee();\n" + + "\t\t })\n" + + "\t\t .baz(() -> {\n" + + "\t\t \tboo().baa()\n" + + "\t\t \t .bee();\n" + + "\t\t })\n" + + "\t\t .bam(() -> {\n" + + "\t\t \tboo().baa()\n" + + "\t\t \t .bee();\n" + + "\t\t });\n" + + "\t}\n" + "}\n", printed); } @@ -275,7 +297,7 @@ public void noChainsIndentsInWhile() { @Test public void indentWithTabsAsFarAsPossible() { - CompilationUnit cu = JavaParser.parse("class Foo { void bar() { foo().bar().baz(() -> { boo().baa().bees(a, b, c); }).bam(); } }"); + CompilationUnit cu = JavaParser.parse("class Foo { void bar() { foo().bar().baz(() -> { boo().baa().bee(a, b, c); }).bam(); } }"); String printed = new PrettyPrinter(new PrettyPrinterConfiguration() .setColumnAlignFirstMethodChain(true) .setColumnAlignParameters(true) @@ -288,11 +310,11 @@ public void indentWithTabsAsFarAsPossible() { "\tvoid bar() {\n" + "\t\tfoo().bar()\n" + "\t\t\t .baz(() -> {\n" + - "\t\t\t\t\t boo().baa()\n" + - "\t\t\t\t\t\t .bees(a,\n" + - "\t\t\t\t\t\t\t\t b,\n" + - "\t\t\t\t\t\t\t\t c);\n" + - "\t\t\t\t })\n" + + "\t\t\t\t boo().baa()\n" + + "\t\t\t\t\t .bee(a,\n" + + "\t\t\t\t\t\t b,\n" + + "\t\t\t\t\t\t c);\n" + + "\t\t\t })\n" + "\t\t\t .bam();\n" + "\t}\n" + "}\n", printed); @@ -301,7 +323,7 @@ public void indentWithTabsAsFarAsPossible() { @Test public void indentWithTabsAlignWithSpaces() { - CompilationUnit cu = JavaParser.parse("class Foo { void bar() { foo().bar().baz(() -> { boo().baa().bee(a, b, c); }).bam(); } }"); + CompilationUnit cu = JavaParser.parse("class Foo { void bar() { foo().bar().baz(() -> { boo().baa().bee(a, b, c); }).baz(() -> { return boo().baa(); }).bam(); } }"); String printed = new PrettyPrinter(new PrettyPrinterConfiguration() .setColumnAlignFirstMethodChain(true) .setColumnAlignParameters(true) @@ -314,11 +336,14 @@ public void indentWithTabsAlignWithSpaces() { "\tvoid bar() {\n" + "\t\tfoo().bar()\n" + "\t\t .baz(() -> {\n" + - "\t\t \tboo().baa()\n" + - "\t\t \t .bee(a,\n" + - "\t\t \t b,\n" + - "\t\t \t c);\n" + - "\t\t })\n" + + "\t\t \tboo().baa()\n" + + "\t\t \t .bee(a,\n" + + "\t\t \t b,\n" + + "\t\t \t c);\n" + + "\t\t })\n" + + "\t\t .baz(() -> {\n" + + "\t\t \treturn boo().baa();\n" + + "\t\t })\n" + "\t\t .bam();\n" + "\t}\n" + "}\n", printed); diff --git a/javaparser-core/src/main/java/com/github/javaparser/printer/PrettyPrintVisitor.java b/javaparser-core/src/main/java/com/github/javaparser/printer/PrettyPrintVisitor.java index 54e6a9095b..336e120f3c 100644 --- a/javaparser-core/src/main/java/com/github/javaparser/printer/PrettyPrintVisitor.java +++ b/javaparser-core/src/main/java/com/github/javaparser/printer/PrettyPrintVisitor.java @@ -30,6 +30,7 @@ import com.github.javaparser.ast.expr.*; import com.github.javaparser.ast.modules.*; import com.github.javaparser.ast.nodeTypes.NodeWithName; +import com.github.javaparser.ast.nodeTypes.NodeWithTraversableScope; import com.github.javaparser.ast.nodeTypes.NodeWithTypeArguments; import com.github.javaparser.ast.nodeTypes.NodeWithVariables; import com.github.javaparser.ast.stmt.*; @@ -38,6 +39,7 @@ import com.github.javaparser.ast.visitor.VoidVisitor; import java.util.*; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; import static com.github.javaparser.ast.Node.Parsedness.UNPARSABLE; @@ -132,27 +134,28 @@ private void printTypeParameters(final NodeList args, final Void private void printArguments(final NodeList args, final Void arg) { printer.print("("); - if (configuration.isColumnAlignParameters()) { - printer.indentWithAlignTo(printer.getCursor().column); - } if (!isNullOrEmpty(args)) { + boolean columnAlignParameters = (args.size() > 1) && configuration.isColumnAlignParameters(); + if (columnAlignParameters) { + printer.indentWithAlignTo(printer.getCursor().column); + } for (final Iterator i = args.iterator(); i.hasNext(); ) { final Expression e = i.next(); e.accept(this, arg); if (i.hasNext()) { printer.print(","); - if (configuration.isColumnAlignParameters()) { + if (columnAlignParameters) { printer.println(); } else { printer.print(" "); } } } + if (columnAlignParameters) { + printer.unindent(); + } } printer.print(")"); - if (configuration.isColumnAlignParameters()) { - printer.unindent(); - } } private void printPrePostFixOptionalList(final NodeList args, final Void arg, String prefix, String separator, String postfix) { @@ -712,37 +715,104 @@ public void visit(final SuperExpr n, final Void arg) { @Override public void visit(final MethodCallExpr n, final Void arg) { printComment(n.getComment(), arg); + + // determine whether we do reindenting for aligmnent at all + // - is it enabled? + // - are we in a statement where we want the alignment? + // - are we not directly in the argument list of a method call expression? + AtomicBoolean columnAlignFirstMethodChain = new AtomicBoolean(); + if (configuration.isColumnAlignFirstMethodChain()) { + // pick the kind of expressions where vertically aligning method calls is okay. + if (n.findParent(Statement.class).map(p -> p.isReturnStmt() + || p.isThrowStmt() + || p.isAssertStmt() + || p.isExpressionStmt()).orElse(false)) { + // search for first parent that does not have its child as scope + Node c = n; + Optional p = c.getParentNode(); + while (p.isPresent() && p.filter(NodeWithTraversableScope.class::isInstance) + .map(NodeWithTraversableScope.class::cast) + .flatMap(NodeWithTraversableScope::traverseScope) + .map(c::equals) + .orElse(false)) { + c = p.get(); + p = c.getParentNode(); + } + + // check if the parent is a method call and thus we are in an argument list + columnAlignFirstMethodChain.set(!p.filter(MethodCallExpr.class::isInstance).isPresent()); + } + } + + // we are at the last method call of a call chain + // this means we do not start reindenting for alignment or we undo it + AtomicBoolean lastMethodInCallChain = new AtomicBoolean(true); + if (columnAlignFirstMethodChain.get()) { + Node node = n; + while (node.getParentNode() + .filter(NodeWithTraversableScope.class::isInstance) + .map(NodeWithTraversableScope.class::cast) + .flatMap(NodeWithTraversableScope::traverseScope) + .map(node::equals) + .orElse(false)) { + node = node.getParentNode().orElseThrow(AssertionError::new); + if (node instanceof MethodCallExpr) { + lastMethodInCallChain.set(false); + break; + } + } + } + + // search whether there is a method call with scope in the scope already + // this means that we probably started reindenting for alignment there + AtomicBoolean methodCallWithScopeInScope = new AtomicBoolean(); + if (columnAlignFirstMethodChain.get()) { + Optional s = n.getScope(); + while (s.filter(NodeWithTraversableScope.class::isInstance).isPresent()) { + Optional parentScope = s.map(NodeWithTraversableScope.class::cast) + .flatMap(NodeWithTraversableScope::traverseScope); + if (s.filter(MethodCallExpr.class::isInstance).isPresent() && parentScope.isPresent()) { + methodCallWithScopeInScope.set(true); + break; + } + s = parentScope; + } + } + + // we have a scope + // this means we are not the first method in the chain n.getScope().ifPresent(scope -> { scope.accept(this, arg); - if (configuration.isColumnAlignFirstMethodChain()) { - // pick the kind of expressions where vertically aligning method calls is okay. - if (n.findParent(Statement.class).map(p -> p.isReturnStmt() - || p.isThrowStmt() - || p.isAssertStmt() - || p.isExpressionStmt()).orElse(false)) { - if (scope instanceof MethodCallExpr && ((MethodCallExpr) scope).getScope().isPresent()) { - /* We're a method call on the result of a method call that is not stand alone, like: - we're x() in a.b().x(), or in a=b().c[15].d.e().x(). - That means that the "else" has been executed by one of the methods in the scope chain, so that the alignment - is set to the "." of that method. - That means we will align to that "." when we start a new line: */ - printer.println(); - } else { - /* We're the first method call on the result of a method call in the chain, like: - we're x() in a().x(). That means we get to dictate the indent of following method - calls in this chain by setting the cursor to where we are now: just before the "." - that start this method call. */ - printer.reindentWithAlignToCursor(); - } + if (columnAlignFirstMethodChain.get()) { + if (methodCallWithScopeInScope.get()) { + /* We're a method call on the result of something (method call, property access, ...) that is not stand alone, + and not the first one with scope, like: + we're x() in a.b().x(), or in a=b().c[15].d.e().x(). + That means that the "else" has been executed by one of the methods in the scope chain, so that the alignment + is set to the "." of that method. + That means we will align to that "." when we start a new line: */ + printer.println(); + } else if (!lastMethodInCallChain.get()) { + /* We're the first method call on the result of something in the chain (method call, property access, ...), + but we are not at the same time the last method call in that chain, like: + we're x() in a().x().y(), or in Long.x().y.z(). That means we get to dictate the indent of following method + calls in this chain by setting the cursor to where we are now: just before the "." + that start this method call. */ + printer.reindentWithAlignToCursor(); } } printer.print("."); }); + printTypeArgs(n, arg); n.getName().accept(this, arg); printer.duplicateIndent(); printArguments(n.getArguments(), arg); printer.unindent(); + if (columnAlignFirstMethodChain.get() && methodCallWithScopeInScope.get() && lastMethodInCallChain.get()) { + // undo the aligning after the arguments of the last method call are printed + printer.reindentToPreviousLevel(); + } } @Override diff --git a/javaparser-core/src/main/java/com/github/javaparser/printer/PrettyPrinterConfiguration.java b/javaparser-core/src/main/java/com/github/javaparser/printer/PrettyPrinterConfiguration.java index 51df5589ea..be5c1608c4 100644 --- a/javaparser-core/src/main/java/com/github/javaparser/printer/PrettyPrinterConfiguration.java +++ b/javaparser-core/src/main/java/com/github/javaparser/printer/PrettyPrinterConfiguration.java @@ -89,7 +89,7 @@ public enum IndentType { /** * Set the string to use for indenting. For example: "\t", " ", "". * - * @deprecated use {@link #setIndentSize(int)} and {@link #setIndentType(IndentType)}. + * @deprecated use {@link #setIndentSize(int)}, {@link #setIndentType(IndentType)} and {@link #setTabWidth(int)}. */ @Deprecated public PrettyPrinterConfiguration setIndent(String indent) { diff --git a/javaparser-core/src/main/java/com/github/javaparser/printer/SourcePrinter.java b/javaparser-core/src/main/java/com/github/javaparser/printer/SourcePrinter.java index 825c923e0f..85172e221a 100644 --- a/javaparser-core/src/main/java/com/github/javaparser/printer/SourcePrinter.java +++ b/javaparser-core/src/main/java/com/github/javaparser/printer/SourcePrinter.java @@ -37,6 +37,8 @@ public class SourcePrinter { private final IndentType indentType; private final Deque indents = new LinkedList<>(); + private final Deque reindentedIndents = new LinkedList<>(); + private String lastPrintedIndent = ""; private final StringBuilder buf = new StringBuilder(); private Position cursor = new Position(1, 0); private boolean indented = false; @@ -85,15 +87,11 @@ public SourcePrinter indentWithAlignTo(int column) { } private String calculateIndentWithAlignTo(int column) { - if (indents.isEmpty()) { - throw new IllegalStateException("Indent/unindent calls are not well-balanced."); - } - final String lastIndent = indents.peek(); - if(column< lastIndent.length()){ + if (column < lastPrintedIndent.length()){ throw new IllegalStateException("Attempt to indent less than the previous indent."); } - StringBuilder newIndent = new StringBuilder(lastIndent); + StringBuilder newIndent = new StringBuilder(lastPrintedIndent); switch (indentType) { case SPACES: case TABS_WITH_SPACE_ALIGN: @@ -165,7 +163,8 @@ private void append(String arg) { */ public SourcePrinter print(final String arg) { if (!indented) { - append(indents.peek()); + lastPrintedIndent = indents.peek(); + append(lastPrintedIndent); indented = true; } append(arg); @@ -238,15 +237,27 @@ public String normalizeEolInTextBlock(String content) { } /** - * Set the top-most indent to the column the cursor is currently in. - * Does not actually output anything. + * Set the top-most indent to the column the cursor is currently in, can be undone with + * {@link #reindentToPreviousLevel()}. Does not actually output anything. */ public void reindentWithAlignToCursor() { String newIndent = calculateIndentWithAlignTo(cursor.column); - indents.pop(); + reindentedIndents.push(indents.pop()); indents.push(newIndent); } + /** + * Set the top-most indent to the column the cursor was before the last {@link #reindentWithAlignToCursor()} call. + * Does not actually output anything. + */ + public void reindentToPreviousLevel() { + if (reindentedIndents.isEmpty()) { + throw new IllegalStateException("Reindent calls are not well-balanced."); + } + indents.pop(); + indents.push(reindentedIndents.pop()); + } + /** * Adds an indent to the top of the stack that is a copy of the current top indent. * With this you announce "I'm going to indent the next line(s)" but not how far yet.