Skip to content

Commit 30b63ba

Browse files
committed
Stop comments that share a line from doubling each other's indent
Comment.computeBreaks moved the column past a comment by adding the length of the comment's last line to the column the comment started at. For a comment that spans lines, JavaCommentsHelper.rewrite has already indented that last line to the start column, so the column was counted twice, and each further comment on the line was re-indented from the doubled column. Commented-out code with javadoc in it, where every "*//** javadoc *//*" puts several multi-line comments on one line, grew from 817 bytes to 4.3 MB with 16 of them and ran out of a 2 GB heap with 26 (#36, from google/google-java-format#413). After a comment that spans lines the column is now the length of its last line; after a one-line comment it is still the start column plus the comment. The 26 comments now format into 12.5 kB in 0.33 s, each indented to the column where it starts. A golden of the reported file and a test with 16 such comments fail without the change. The 15,747 files of the JDK 21 sources format exactly as before.
1 parent 53ac7d8 commit 30b63ba

4 files changed

Lines changed: 52 additions & 2 deletions

File tree

‎open-java-format/src/main/java/com/palantir/javaformat/doc/Comment.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,13 @@ public State computeBreaks(
8888
CommentsHelper commentsHelper, int maxWidth, State state, Obs.ExplorationNode observationNode) {
8989
String text = commentsHelper.rewrite(tok, maxWidth, state.column());
9090
@SuppressWarnings("for-rollout:NullAway")
91-
int firstLineLength = text.length() - Iterators.getLast(Newlines.lineOffsetIterator(text));
92-
return state.withColumn(state.column() + firstLineLength)
91+
int lastLineStart = Iterators.getLast(Newlines.lineOffsetIterator(text));
92+
int lastLineLength = text.length() - lastLineStart;
93+
// rewrite() indents every line after the first to state.column(), so after a comment that spans lines the
94+
// column is the length of its last line. Adding state.column() to it as well doubled the column for each
95+
// further comment on that line.
96+
int column = lastLineStart == 0 ? state.column() + lastLineLength : lastLineLength;
97+
return state.withColumn(column)
9398
.addNewLines(Iterators.size(Newlines.lineOffsetIterator(text)))
9499
.withTokState(this, ImmutableTokState.of(text));
95100
}

‎open-java-format/src/test/java/com/palantir/javaformat/java/FormatterTest.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,4 +491,21 @@ void formatsDeeplyNestedCallsQuickly() {
491491

492492
assertTimeoutPreemptively(Duration.ofSeconds(10), () -> formatter.formatSource(input));
493493
}
494+
495+
@Test
496+
void indentsCommentsThatShareALineLinearly() throws FormatterException {
497+
// Commented-out code that had javadoc in it puts many multi-line comments on one line: "*//** javadoc *//*".
498+
// Each comment's continuation lines are indented to the column where the comment starts, and the column after
499+
// a comment used to be counted from where it started plus its last line, which already holds that indent. So
500+
// every comment started twice as far right as the one before, and these sixteen came out as 4.3 MB.
501+
StringBuilder input = new StringBuilder("/*\nclass X {\n");
502+
for (int i = 0; i < 16; i++) {
503+
input.append("\t*//** javadoc *//*\n\tpublic void foo(Bar bar) {}\n\n");
504+
}
505+
input.append("}*/\n");
506+
Formatter formatter = Formatter.createFormatter(
507+
JavaFormatterOptions.builder().style(Style.OJF).build());
508+
509+
assertThat(formatter.formatSource(input.toString()).length()).isLessThan(10_000);
510+
}
494511
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/*
2+
class X {
3+
*//** javadoc *//*
4+
public void foo(Bar bar) {}
5+
6+
*//** javadoc *//*
7+
public void foo(Bar bar) {}
8+
9+
*//** javadoc *//*
10+
public void foo(Bar bar) {}
11+
12+
*//** javadoc *//*
13+
public void foo(Bar bar) {}
14+
}*/
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/*
2+
class X {
3+
*//** javadoc *//*
4+
public void foo(Bar bar) {}
5+
6+
*//** javadoc *//*
7+
public void foo(Bar bar) {}
8+
9+
*//** javadoc *//*
10+
public void foo(Bar bar) {}
11+
12+
*//** javadoc *//*
13+
public void foo(Bar bar) {}
14+
}*/

0 commit comments

Comments
 (0)