Bug 512095 - [formatter] Unstable wrap on a line with wrapped code and wrapped block comment
diff --git a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/formatter/FormatterCommentsBugsTest.java b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/formatter/FormatterCommentsBugsTest.java
index 28ef2d7..4c6f07e 100644
--- a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/formatter/FormatterCommentsBugsTest.java
+++ b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/formatter/FormatterCommentsBugsTest.java
@@ -7577,4 +7577,26 @@
"}"
);
}
+/**
+ * https://bugs.eclipse.org/512095 - [formatter] Unstable wrap on a line with wrapped code and wrapped block comment
+ */
+public void testBug512095() {
+ String source =
+ "class Test1 {\n" +
+ " void f() {\n" +
+ " String c = \"aaaaaaaaaaaaaaaa\" + \"aaaaaaaaaaaaaaa\"; /* 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 */\n" +
+ " }\n" +
+ "}";
+ formatSource(source,
+ "class Test1 {\n" +
+ " void f() {\n" +
+ " String c = \"aaaaaaaaaaaaaaaa\"\n" +
+ " + \"aaaaaaaaaaaaaaa\"; /*\n" +
+ " * 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15\n" +
+ " * 16 17\n" +
+ " */\n" +
+ " }\n" +
+ "}"
+ );
+}
}
diff --git a/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/CommentsPreparator.java b/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/CommentsPreparator.java
index 29b8007..014638a 100644
--- a/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/CommentsPreparator.java
+++ b/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/CommentsPreparator.java
@@ -1008,11 +1008,14 @@
boolean newLinesAtBoundries = commentToken.tokenType == TokenNameCOMMENT_JAVADOC
? this.options.comment_new_lines_at_javadoc_boundaries
: this.options.comment_new_lines_at_block_boundaries;
- if (newLinesAtBoundries && this.tm.countLineBreaksBetween(first, last) > 0) {
+ if (!newLinesAtBoundries) {
+ structure.get(1).clearLineBreaksBefore();
+ last.clearLineBreaksBefore();
+ } else if (this.tm.countLineBreaksBetween(first, last) > 0) {
first.breakAfter();
last.breakBefore();
- last.setAlign(1);
}
+ last.setAlign(1);
if (structure.size() == 2)
return false;
diff --git a/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/linewrap/CommentWrapExecutor.java b/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/linewrap/CommentWrapExecutor.java
index bd659f3..07333ee 100644
--- a/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/linewrap/CommentWrapExecutor.java
+++ b/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/linewrap/CommentWrapExecutor.java
@@ -36,9 +36,9 @@
private final ArrayList<Token> nlsTags = new ArrayList<>();
private int lineStartPosition;
- private List<Token> blockStructure;
private boolean simulation;
private boolean wrapDisabled;
+ private boolean newLinesAtBoundries;
private Token potentialWrapToken, potentialWrapTokenSubstitute;
private int counterIfWrapped, counterIfWrappedSubstitute;
@@ -61,40 +61,26 @@
public int wrapMultiLineComment(Token commentToken, int startPosition, boolean simulate, boolean noWrap) {
this.lineCounter = 1;
this.counter = startPosition;
-
- List<Token> structure = commentToken.getInternalStructure();
- if (structure == null || structure.isEmpty())
- return startPosition + this.tm.getLength(commentToken, startPosition);
-
commentToken.setIndent(this.tm.toIndent(startPosition, true));
this.lineStartPosition = commentToken.getIndent();
this.simulation = simulate;
this.wrapDisabled = noWrap;
this.potentialWrapToken = this.potentialWrapTokenSubstitute = null;
- this.blockStructure = structure;
- traverse(structure, 0);
-
- boolean newLinesAtBoundries = commentToken.tokenType == TokenNameCOMMENT_JAVADOC
+ this.newLinesAtBoundries = commentToken.tokenType == TokenNameCOMMENT_JAVADOC
? this.options.comment_new_lines_at_javadoc_boundaries
: this.options.comment_new_lines_at_block_boundaries;
- if (this.lineCounter > 1 && newLinesAtBoundries) {
- Token endingToken = structure.get(structure.size() - 1);
- if (!simulate && endingToken.tokenType != TokenNameNotAToken) {
- structure.get(0).breakAfter();
- endingToken.breakBefore();
- endingToken.setAlign(1);
- }
- return startPosition + this.tm.getLength(endingToken, startPosition);
- } else if (this.lineCounter > 1 && !newLinesAtBoundries) {
- // the rest of this code assumes that newLinesAtBoundries==true, so now subtract the additional lines
- this.lineCounter -= 2;
- this.lineCounter -= structure.get(1).getLineBreaksBefore();
- structure.get(1).clearLineBreaksBefore();
- Token last = structure.get(structure.size() - 1);
- this.lineCounter -= last.getLineBreaksBefore();
- last.clearLineBreaksBefore();
- }
+ List<Token> structure = commentToken.getInternalStructure();
+ if (structure == null || structure.isEmpty())
+ return startPosition + this.tm.getLength(commentToken, startPosition);
+
+ int position = tryToFitInOneLine(structure, startPosition, noWrap);
+ if (position > 0)
+ return position;
+
+ traverse(structure, 0);
+ if (this.newLinesAtBoundries)
+ return this.lineStartPosition + 1 + this.tm.getLength(structure.get(structure.size() - 1), 0);
return this.counter;
}
@@ -102,19 +88,58 @@
return this.lineCounter;
}
+ private int tryToFitInOneLine(List<Token> structure, int startPosition, boolean noWrap) {
+ int position = startPosition;
+ boolean hasWrapPotential = false;
+ boolean wasSpaceAfter = false;
+ for (int i = 0; i < structure.size(); i++) {
+ Token token = structure.get(i);
+ if (token.getLineBreaksBefore() > 0 || token.getLineBreaksAfter() > 0) {
+ assert !noWrap; // comment already wrapped
+ return -1;
+ }
+ if (!wasSpaceAfter && token.isSpaceBefore())
+ position++;
+ position += this.tm.getLength(token, position);
+ wasSpaceAfter = token.isSpaceAfter();
+ if (wasSpaceAfter)
+ position++;
+
+ WrapPolicy policy = token.getWrapPolicy();
+ if (i > 1 && (policy == null || policy == WrapPolicy.SUBSTITUTE_ONLY))
+ hasWrapPotential = true;
+ }
+ if (position <= this.options.comment_line_length || noWrap || !hasWrapPotential)
+ return position;
+ return -1;
+ }
+
+ private int getStartingPosition(Token token) {
+ int position = this.lineStartPosition + token.getAlign() + token.getIndent();
+ if (token.tokenType != TokenNameNotAToken)
+ position += COMMENT_LINE_SEPARATOR_LENGTH;
+ return position;
+ }
+
@Override
protected boolean token(Token token, int index) {
- int positionIfNewLine = this.lineStartPosition + token.getAlign() + token.getIndent();
- if (token.tokenType != TokenNameNotAToken)
- positionIfNewLine += COMMENT_LINE_SEPARATOR_LENGTH;
- if (getLineBreaksBefore() > 0) {
- this.lineCounter = Math.max(this.lineCounter + getLineBreaksBefore(), 4);
+ final int positionIfNewLine = getStartingPosition(token);
+
+ int lineBreaksBefore = getLineBreaksBefore();
+ if ((index == 1 || getNext() == null) && this.newLinesAtBoundries && lineBreaksBefore == 0) {
+ if (!this.simulation)
+ token.breakBefore();
+ lineBreaksBefore = 1;
+ }
+
+ if (lineBreaksBefore > 0) {
+ this.lineCounter += lineBreaksBefore;
this.counter = positionIfNewLine;
this.potentialWrapToken = this.potentialWrapTokenSubstitute = null;
boolean isFormattedCode = token.getWrapPolicy() != null
&& token.getWrapPolicy() != WrapPolicy.SUBSTITUTE_ONLY;
- if (!isFormattedCode && token.getAlign() == 0) {
+ if (!isFormattedCode && token.getAlign() == 0 && !this.simulation) {
// Indents are reserved for code inside <pre>.
// Indentation of javadoc tags can be achieved with align
token.setAlign(token.getIndent());
@@ -122,8 +147,7 @@
}
}
- boolean canWrap = getNext() != null && getLineBreaksBefore() == 0 && index > 1
- && positionIfNewLine < this.counter;
+ boolean canWrap = getNext() != null && lineBreaksBefore == 0 && index > 1 && positionIfNewLine < this.counter;
if (canWrap) {
if (token.getWrapPolicy() == null) {
this.potentialWrapToken = token;
@@ -143,7 +167,6 @@
this.potentialWrapToken = this.potentialWrapTokenSubstitute;
this.counterIfWrapped = this.counterIfWrappedSubstitute;
}
- this.counter = this.counterIfWrapped;
if (!this.simulation) {
this.potentialWrapToken.breakBefore();
// Indents are reserved for code inside <pre>.
@@ -151,7 +174,8 @@
this.potentialWrapToken.setAlign(this.potentialWrapToken.getIndent());
this.potentialWrapToken.setIndent(0);
}
- this.lineCounter = Math.max(this.lineCounter + 1, 4);
+ this.counter = this.counterIfWrapped;
+ this.lineCounter++;
this.potentialWrapToken = this.potentialWrapTokenSubstitute = null;
}
@@ -178,31 +202,9 @@
this.potentialWrapToken = null;
}
if (this.potentialWrapToken == null && this.potentialWrapTokenSubstitute == null) {
- boolean isFormattingEnabled = this.blockStructure.size() > 1
- && this.blockStructure.get(1).tokenType == TokenNameNotAToken;
- if (isFormattingEnabled) {
- // can't wrap, but the long comment cannot stay in one line
- this.lineCounter = Math.max(this.lineCounter, 3);
- }
return false;
}
- if (this.options.comment_new_lines_at_javadoc_boundaries) {
- if (getNext() == null) { // the closing token will go to the next line anyway
- this.lineCounter = Math.max(this.lineCounter, 3);
- return false;
- }
- if (this.lineCounter == 1) {
- // when wrapping the first line of javadoc (more asterisks in opening token), the line will
- // move to the left so it may not need wrapping in the end
- int openingTokenLength = this.tm.getLength(this.blockStructure.get(0), 0);
- if (this.counter - (openingTokenLength - 2) <= lineLenght) {
- this.counter -= (openingTokenLength - 2);
- this.lineCounter = Math.max(this.lineCounter, 3);
- return false;
- }
- }
- }
return true;
}
