Skip to content

Commit

Permalink
Fix indentation realignment when wrapping call-chains
Browse files Browse the repository at this point in the history
  • Loading branch information
@Vampire
Vampire committed May 26, 2018
1 parent 1626ba1 commit a689423
Show file tree
Hide file tree
Showing 4 changed files with 166 additions and 60 deletions.
69 changes: 47 additions & 22 deletions javaparser-core-testing/src/test/java/com/github/javaparser/printer/PrettyPrinterTest.java
View file
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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)
Expand All @@ -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);
Expand All @@ -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)
Expand All @@ -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);
Expand Down
124 changes: 97 additions & 27 deletions javaparser-core/src/main/java/com/github/javaparser/printer/PrettyPrintVisitor.java
View file
Expand Up @@ -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.*;
Expand All @@ -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;
Expand Down Expand Up @@ -132,27 +134,28 @@ private void printTypeParameters(final NodeList<TypeParameter> args, final Void

private void printArguments(final NodeList<Expression> 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<Expression> 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<? extends Visitable> args, final Void arg, String prefix, String separator, String postfix) {
Expand Down Expand Up @@ -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<Node> 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<Expression> s = n.getScope();
while (s.filter(NodeWithTraversableScope.class::isInstance).isPresent()) {
Optional<Expression> 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
Expand Down
2 changes: 1 addition & 1 deletion javaparser-core/src/main/java/com/github/javaparser/printer/PrettyPrinterConfiguration.java
View file
Expand Up @@ -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) {
Expand Down
31 changes: 21 additions & 10 deletions javaparser-core/src/main/java/com/github/javaparser/printer/SourcePrinter.java
View file
Expand Up @@ -37,6 +37,8 @@ public class SourcePrinter {
private final IndentType indentType;

private final Deque<String> indents = new LinkedList<>();
private final Deque<String> reindentedIndents = new LinkedList<>();
private String lastPrintedIndent = "";
private final StringBuilder buf = new StringBuilder();
private Position cursor = new Position(1, 0);
private boolean indented = false;
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit a689423

Please sign in to comment.