Bug 472413 - [formatter] Wrap all arguments on new lines and prefer outer expressions result is inconsistent
Change-Id: Ia841df8bdecb06fe779e1bcc2f025f8517b8f7d8
Signed-off-by: Mateusz Matela <mateusz.matela@gmail.com>
diff --git a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/formatter/FormatterBugsTests.java b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/formatter/FormatterBugsTests.java
index a9771db..fc06e5f 100644
--- a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/formatter/FormatterBugsTests.java
+++ b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/formatter/FormatterBugsTests.java
@@ -11599,4 +11599,104 @@
"}"
);
}
+/**
+ * https://bugs.eclipse.org/472413 - [formatter] Wrap all arguments on new lines and prefer outer expressions result is inconsistent
+ */
+public void testBug472413() {
+ this.formatterPrefs.alignment_for_arguments_in_method_invocation =
+ DefaultCodeFormatterOptions.Alignment.M_ONE_PER_LINE_SPLIT
+ + DefaultCodeFormatterOptions.Alignment.M_INDENT_BY_ONE;
+ this.formatterPrefs.page_width = 80;
+ String source =
+ "class Snippet {\r\n" +
+ "\r\n" +
+ " void foo1() {\r\n" +
+ " Other.bar(\r\n" +
+ " 100,\r\n" +
+ " nestedMethod2Arg(\r\n" +
+ " nestedMethod1Arg(\r\n" +
+ " nestedMethod2Arg(nestedMethod1Arg(nestedMethod2Arg(\r\n" +
+ " nestedMethod1Arg(nestedMethod1Arg(nestedMethod1Arg(\r\n" +
+ " nested(200, 300, 400, 500, 600, 700, 800, 900)))),\r\n" +
+ " null)), null)),\r\n" +
+ " null),\r\n" +
+ " 100);\r\n" +
+ " }\r\n" +
+ "\r\n" +
+ " void foo2() {\r\n" +
+ " nestedMethodAAAA(\r\n" +
+ " nestedMethodBBBB(\r\n" +
+ " nestedMethodCCC(dddddd(200, 300, 400, 500, 600, 700, 800, 900)),\r\n" +
+ " null));\r\n" +
+ " nestedMethodAAAA(nestedMethodBBBB(\r\n" +
+ " nestedMethodCCC(dddddd(200, 300, 400, 500, 600, 700, 800, 900)),\r\n" +
+ " null));\r\n" +
+ " }\r\n" +
+ "\r\n" +
+ " void foo3() {\r\n" +
+ " nestedMethodAAAA(\r\n" +
+ " nestedMethodBBBB(\r\n" +
+ " nestedMethodCCC(dddddd(200, 300, 400, 500, 600, 700, 800, 900)),\r\n" +
+ " null),\r\n" +
+ " null);\r\n" +
+ " nestedMethodAAAA(nestedMethodBBBB(\r\n" +
+ " nestedMethodCCC(dddddd(200, 300, 400, 500, 600, 700, 800, 900)),\r\n" +
+ " null), null);\r\n" +
+ " }\r\n" +
+ "}";
+ formatSource(source,
+ "class Snippet {\r\n" +
+ "\r\n" +
+ " void foo1() {\r\n" +
+ " Other.bar(\r\n" +
+ " 100,\r\n" +
+ " nestedMethod2Arg(\r\n" +
+ " nestedMethod1Arg(\r\n" +
+ " nestedMethod2Arg(\r\n" +
+ " nestedMethod1Arg(\r\n" +
+ " nestedMethod2Arg(\r\n" +
+ " nestedMethod1Arg(\r\n" +
+ " nestedMethod1Arg(\r\n" +
+ " nestedMethod1Arg(\r\n" +
+ " nested(\r\n" +
+ " 200,\r\n" +
+ " 300,\r\n" +
+ " 400,\r\n" +
+ " 500,\r\n" +
+ " 600,\r\n" +
+ " 700,\r\n" +
+ " 800,\r\n" +
+ " 900)))),\r\n" +
+ " null)),\r\n" +
+ " null)),\r\n" +
+ " null),\r\n" +
+ " 100);\r\n" +
+ " }\r\n" +
+ "\r\n" +
+ " void foo2() {\r\n" +
+ " nestedMethodAAAA(\r\n" +
+ " nestedMethodBBBB(\r\n" +
+ " nestedMethodCCC(dddddd(200, 300, 400, 500, 600, 700, 800, 900)),\r\n" +
+ " null));\r\n" +
+ " nestedMethodAAAA(\r\n" +
+ " nestedMethodBBBB(\r\n" +
+ " nestedMethodCCC(dddddd(200, 300, 400, 500, 600, 700, 800, 900)),\r\n" +
+ " null));\r\n" +
+ " }\r\n" +
+ "\r\n" +
+ " void foo3() {\r\n" +
+ " nestedMethodAAAA(\r\n" +
+ " nestedMethodBBBB(\r\n" +
+ " nestedMethodCCC(dddddd(200, 300, 400, 500, 600, 700, 800, 900)),\r\n" +
+ " null),\r\n" +
+ " null);\r\n" +
+ " nestedMethodAAAA(\r\n" +
+ " nestedMethodBBBB(\r\n" +
+ " nestedMethodCCC(dddddd(200, 300, 400, 500, 600, 700, 800, 900)),\r\n" +
+ " null),\r\n" +
+ " null);\r\n" +
+ " }\r\n" +
+ "}"
+ );
+}
}
diff --git a/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/linewrap/WrapPreparator.java b/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/linewrap/WrapPreparator.java
index 51c8b38..3ac9ad2 100644
--- a/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/linewrap/WrapPreparator.java
+++ b/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/linewrap/WrapPreparator.java
@@ -133,6 +133,10 @@
@Override
public boolean preVisit2(ASTNode node) {
this.currentDepth++;
+
+ assert this.wrapIndexes.isEmpty() && this.wrapPenalties.isEmpty();
+ assert this.wrapParentIndex == -1 && this.wrapGroupEnd == -1;
+
boolean isMalformed = (node.getFlags() & ASTNode.MALFORMED) != 0;
if (isMalformed) {
this.tm.addDisableFormatTokenPair(this.tm.firstTokenIn(node, -1), this.tm.lastTokenIn(node, -1));
@@ -232,13 +236,16 @@
}
if (!node.isConstructor()) {
+ this.wrapParentIndex = this.tm.findFirstTokenInLine(this.tm.firstIndexIn(node.getName(), -1));
List<TypeParameter> typeParameters = node.typeParameters();
if (!typeParameters.isEmpty())
this.wrapIndexes.add(this.tm.firstIndexIn(typeParameters.get(0), -1));
- if (node.getReturnType2() != null && !node.modifiers().isEmpty())
- this.wrapIndexes.add(this.tm.firstIndexIn(node.getReturnType2(), -1));
+ if (node.getReturnType2() != null) {
+ int returTypeIndex = this.tm.firstIndexIn(node.getReturnType2(), -1);
+ if (returTypeIndex != this.wrapParentIndex)
+ this.wrapIndexes.add(returTypeIndex);
+ }
this.wrapIndexes.add(this.tm.firstIndexIn(node.getName(), -1));
- this.wrapParentIndex = this.tm.findFirstTokenInLine(this.tm.firstIndexIn(node.getName(), -1));
this.wrapGroupEnd = this.tm.lastIndexIn(node.getName(), -1);
handleWrap(this.options.alignment_for_method_declaration);
}
@@ -581,7 +588,6 @@
this.wrapParentIndex = this.tm.findIndex(firstToken.originalStart - 1, TokenNameLPAREN, false);
if (!arguments.isEmpty() && this.wrapGroupEnd < 0)
this.wrapGroupEnd = this.tm.lastIndexIn(arguments.get(arguments.size() - 1), -1);
- assert this.wrapGroupEnd >= 0;
handleWrap(wrappingOption, 1 / PREFERRED);
}
}
@@ -596,17 +602,22 @@
}
private void handleWrap(int wrappingOption, ASTNode parentNode) {
+ doHandleWrap(wrappingOption, parentNode);
+ this.wrapIndexes.clear();
+ this.wrapPenalties.clear();
+ this.wrapParentIndex = this.wrapGroupEnd = -1;
+ }
+
+ private void doHandleWrap(int wrappingOption, ASTNode parentNode) {
if (this.wrapIndexes.isEmpty())
return;
- assert this.wrapParentIndex >= 0;
+ assert this.wrapParentIndex >= 0 && this.wrapParentIndex < this.wrapIndexes.get(0);
+ assert this.wrapGroupEnd >= this.wrapIndexes.get(this.wrapIndexes.size() - 1);
float penalty = this.wrapPenalties.isEmpty() ? 1 : this.wrapPenalties.get(0);
WrapPolicy policy = getWrapPolicy(wrappingOption, penalty, true, parentNode);
- if (policy == null) {
- this.wrapIndexes.clear();
- this.wrapPenalties.clear();
- this.wrapParentIndex = this.wrapGroupEnd = -1;
+ if (policy == null)
return;
- }
+
setTokenWrapPolicy(this.wrapIndexes.get(0), policy, true);
boolean wrapPreceedingComments = !(parentNode instanceof InfixExpression)
@@ -634,9 +645,6 @@
this.tm.get(this.wrapIndexes.get(0)).breakBefore();
}
}
- this.wrapIndexes.clear();
- this.wrapPenalties.clear();
- this.wrapParentIndex = this.wrapGroupEnd = -1;
}
private void setTokenWrapPolicy(int index, WrapPolicy policy, boolean wrapPreceedingComments) {
