Skip to content

Commit bf9e0eb

Browse files
committed
Do not inline a lambda body whose closing paren carries a comment
A "//" comment on its own line before the "))" that close a lambda's parenthesized body and the call around it was joined onto the code line before it, and the "))" ended up inside the comment, so the output no longer parsed (#62, palantir#1792). The command line noticed only because its import pass parses the result again and reported a position in the output; a caller of formatSource got the text as it was. An expression lambda's body is laid out by handle_breakOnlyIfInnerLevelsThenFitOnOneLine: when the body does not fit after "->" but its first line does, tryInlinePrefixOntoCurrentLine lays the body's level out "on one line" through tryToLayOutLevelOnOneLine, which marks every break of that level as not taken and only lets the inner levels break. The comments before the closing ")" belong to that same level, with forced breaks around them, and a forced break laid out flat is what put the comment on the code line. The width of the docs before the last inner level was checked, and a forced break there fails that check; the docs after it were not checked at all. The check is now made for the trailing docs too, the way tryBreakInnerLevel already refuses a suffix of infinite width: a body with a comment before its closing token is not inlined and breaks normally, so the body starts on the line after "->" and the comments keep their own lines at the body's indent, as they already did when the body's first line did not fit. A block comment in the same place was written without a space in front of it and moved on a second run; it now gets its own line as well. The golden holds the reporter's input, the same shape as a plain call, and the block comment. The 15,747 files of the JDK 21 sources format exactly as before: none of them has a comment before "))".
1 parent 7f2a32d commit bf9e0eb

3 files changed

Lines changed: 70 additions & 1 deletion

File tree

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,9 +339,19 @@ private Optional<State> tryInlinePrefixOntoCurrentLine(
339339

340340
// Add the width of tokens, breaks before the lastLevel. We must always have space for
341341
// these.
342-
List<Doc> leadingDocs = docs.subList(0, docs.indexOf(lastLevel));
342+
int lastLevelIndex = docs.indexOf(lastLevel);
343+
List<Doc> leadingDocs = docs.subList(0, lastLevelIndex);
343344
float leadingWidth = getWidth(leadingDocs);
344345

346+
// A forced break after the lastLevel, such as the ones around a // comment that sits before this level's
347+
// closing token, cannot be laid out flat by tryToLayOutLevelOnOneLine: the comment would swallow every token
348+
// after it on the line. Such a level breaks normally instead, as tryBreakInnerLevel refuses it for the same
349+
// reason. (A forced break before the lastLevel makes leadingWidth infinite and fails the check below.)
350+
List<Doc> trailingDocs = docs.subList(lastLevelIndex + 1, docs.size());
351+
if (Float.isInfinite(getWidth(trailingDocs))) {
352+
return Optional.empty();
353+
}
354+
345355
// Potentially add the width of prefixes we want to consider as part of the width that
346356
// must fit on the same line, so that we don't accidentally break prefixes when we could
347357
// have avoided doing so.
@@ -592,6 +602,9 @@ private static Optional<State> tryBreakInnerLevel_checkInner(
592602
* Mark breaks in this level as not broken, but lay out the inner levels normally, according to their own
593603
* {@link BreakBehaviour}. The resulting {@link State#mustBreak} will be true if this level did not fit on exactly
594604
* one line.
605+
*
606+
* <p>The callers make sure that none of this level's own breaks is forced: a forced break laid out flat would put
607+
* the tokens after a {@code //} comment inside the comment.
595608
*/
596609
private State tryToLayOutLevelOnOneLine(
597610
CommentsHelper commentsHelper,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import java.util.stream.IntStream;
2+
3+
class Repro {
4+
int find(Item[] items) {
5+
return IntStream.range(0, items.length)
6+
.filter(i -> (items[i].getName().equals("alpha") || items[i].getName().equals("beta")
7+
|| items[i].getName().equals("gamma")
8+
// || (items[i].getName().equals("delta") && items.length > i
9+
// && items[i + 1].getName().equals("epsilon"))
10+
)).findFirst().orElse(-1);
11+
}
12+
13+
void plainCall() {
14+
check(i -> (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(i) || bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(i)
15+
// || c(i)
16+
));
17+
}
18+
19+
void blockComment() {
20+
check(i -> (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(i) || bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(i)
21+
/* block */
22+
));
23+
}
24+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import java.util.stream.IntStream;
2+
3+
class Repro {
4+
int find(Item[] items) {
5+
return IntStream.range(0, items.length)
6+
.filter(i ->
7+
(items[i].getName().equals("alpha")
8+
|| items[i].getName().equals("beta")
9+
|| items[i].getName().equals("gamma")
10+
// || (items[i].getName().equals("delta") && items.length > i
11+
// && items[i + 1].getName().equals("epsilon"))
12+
))
13+
.findFirst()
14+
.orElse(-1);
15+
}
16+
17+
void plainCall() {
18+
check(i ->
19+
(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(i)
20+
|| bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(i)
21+
// || c(i)
22+
));
23+
}
24+
25+
void blockComment() {
26+
check(i ->
27+
(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(i)
28+
|| bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(i)
29+
/* block */
30+
));
31+
}
32+
}

0 commit comments

Comments
 (0)