Skip to content

Commit 7f98ec8

Browse files
tanvir-uxabashev
authored andcommitted
Indent a block comment that trails { like the statements after it
A `/* ... */` comment left on the line of an opening brace is moved onto its own line, and landed one indent deeper than the statements of the block, or the elements of the array initializer, that followed it. A second run moved it back, so the output was not stable. A lambda body such as `() -> { /* c */ f(); }` and a block after `default:` had the same defect. visitBlock and the non-empty branch of visitArrayInitializer open a plusTwo level and then asked for another plusTwo on the break before the comment. OpsBuilder.build puts that comment right after the token, inside the level, so the two indents stacked. The break now gets ZERO there. An empty block keeps plusTwo: it closes the level right after `{`, and build moves the comment past that close, so without the indent the comment would sit at the column of the braces. Ported from google/google-java-format#1464 (commit ea7b0c40), which fixes google/google-java-format#1260. The tests were written first: the I1260 fixture (upstream's input plus a lambda body) and trailingCommentAfterOpenBraceIsIdempotent in GOOGLE style failed on the first pass before the change. The I1260EmptyBlocks fixture pins the empty-block shapes that the change leaves as they were. Fixes #129.
1 parent 3e13850 commit 7f98ec8

6 files changed

Lines changed: 143 additions & 2 deletions

File tree

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,8 @@ public boolean visitArrayInitializer(List<? extends ExpressionTree> expressions)
541541

542542
builder.open(ZERO, BreakBehaviour.breakThisLevel(), LastLevelBreakability.ACCEPT_INLINE_CHAIN);
543543
builder.open(plusTwo, BreakBehaviour.breakThisLevel(), LastLevelBreakability.ACCEPT_INLINE_CHAIN);
544-
tokenBreakTrailingComment("{", plusTwo);
544+
// Same as visitBlock: the break is already inside the plusTwo level, and this branch has elements.
545+
tokenBreakTrailingComment("{", ZERO);
545546
boolean hasTrailingComma = hasTrailingToken(builder.getInput(), expressions, ",");
546547
builder.breakOp(hasTrailingComma ? FillMode.FORCED : FillMode.UNIFIED, "", ZERO);
547548
if (allowFilledElementsOnOwnLine) {
@@ -2154,7 +2155,11 @@ protected void visitBlock(
21542155
} else {
21552156
builder.open(ZERO);
21562157
builder.open(plusTwo);
2157-
tokenBreakTrailingComment("{", plusTwo);
2158+
// OpsBuilder.build puts a comment that trails "{" right after the token, inside the plusTwo level, so a
2159+
// non-empty block adds no indent of its own: the comment lines up with the statements, and a second run
2160+
// leaves it there. An empty block closes the level right after "{", and build moves the comment past that
2161+
// close, so it keeps plusTwo to sit inside the braces.
2162+
tokenBreakTrailingComment("{", node.getStatements().isEmpty() ? plusTwo : ZERO);
21582163
if (allowLeadingBlankLine == AllowLeadingBlankLine.NO) {
21592164
builder.blankLineWanted(BlankLineWanted.NO);
21602165
} else {

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,4 +636,40 @@ void f(Map<String, Integer> map) {
636636

637637
assertThat(formatter.formatSource(input)).isEqualTo(input);
638638
}
639+
640+
@Test
641+
void trailingCommentAfterOpenBraceIsIdempotent() throws FormatterException {
642+
// A block comment that trails `{` is broken onto its own line. It used to land one indent deeper than the
643+
// statements that follow, and only a second run moved it back (google-java-format#1260, #129).
644+
String input = """
645+
class T {
646+
void f() {
647+
{ /* c */ int x = 1; }
648+
}
649+
650+
int[] a = { /* c */ 1 };
651+
}
652+
""";
653+
String expected = """
654+
class T {
655+
void f() {
656+
{
657+
/* c */
658+
int x = 1;
659+
}
660+
}
661+
662+
int[] a = {
663+
/* c */
664+
1
665+
};
666+
}
667+
""";
668+
Formatter formatter = Formatter.createFormatter(
669+
JavaFormatterOptions.builder().style(Style.GOOGLE).build());
670+
671+
String once = formatter.formatSource(input);
672+
assertThat(once).isEqualTo(expected);
673+
assertThat(formatter.formatSource(once)).isEqualTo(once);
674+
}
639675
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
class I1260 {
2+
void block() {
3+
{ /* trailing on open brace */ int x = 1; }
4+
}
5+
6+
void switchCase() {
7+
switch (0) {
8+
case 1: { /* Break so we don't hit fall-through warning: */ break;/* ignore STRING */
9+
}
10+
}
11+
}
12+
13+
void lambda() {
14+
Runnable r = () -> { /* trailing on lambda brace */ block(); };
15+
}
16+
17+
int[] array = { /* trailing on array brace */ 1, 2 };
18+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
class I1260 {
2+
void block() {
3+
{
4+
/* trailing on open brace */
5+
int x = 1;
6+
}
7+
}
8+
9+
void switchCase() {
10+
switch (0) {
11+
case 1: {
12+
/* Break so we don't hit fall-through warning: */
13+
break; /* ignore STRING */
14+
}
15+
}
16+
}
17+
18+
void lambda() {
19+
Runnable r = () -> {
20+
/* trailing on lambda brace */
21+
block();
22+
};
23+
}
24+
25+
int[] array = {
26+
/* trailing on array brace */
27+
1, 2
28+
};
29+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
class I1260EmptyBlocks {
2+
void f() {
3+
{ /* c */ }
4+
try { /* t */ } catch (Exception e) { /* c */ }
5+
try { /* t */ } finally { /* f */ }
6+
if (true) { /* i */ }
7+
while (true) { /* w */ }
8+
switch (0) { case 1: { /* s */ } }
9+
int[] a = { /* a */ };
10+
}
11+
void g() { /* body */ }
12+
void h() { /* body */ int x; }
13+
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
class I1260EmptyBlocks {
2+
void f() {
3+
{
4+
/* c */
5+
}
6+
try {
7+
/* t */
8+
} catch (Exception e) {
9+
/* c */
10+
}
11+
try {
12+
/* t */
13+
} finally {
14+
/* f */
15+
}
16+
if (true) {
17+
/* i */
18+
}
19+
while (true) {
20+
/* w */
21+
}
22+
switch (0) {
23+
case 1: {
24+
/* s */
25+
}
26+
}
27+
int[] a = {
28+
/* a */
29+
};
30+
}
31+
32+
void g() {
33+
/* body */
34+
}
35+
36+
void h() {
37+
/* body */
38+
int x;
39+
}
40+
}

0 commit comments

Comments
 (0)