Merge pull request #143 from openjavaformat/fix/trailing-comment-after-open-brace

Indent a block comment that trails `{` like the statements after it

Author: abashev

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

@@ -541,7 +541,8 @@ public boolean visitArrayInitializer(List<? extends ExpressionTree> expressions)
 
             builder.open(ZERO, BreakBehaviour.breakThisLevel(), LastLevelBreakability.ACCEPT_INLINE_CHAIN);
             builder.open(plusTwo, BreakBehaviour.breakThisLevel(), LastLevelBreakability.ACCEPT_INLINE_CHAIN);
-            tokenBreakTrailingComment("{", plusTwo);
+            // Same as visitBlock: the break is already inside the plusTwo level, and this branch has elements.
+            tokenBreakTrailingComment("{", ZERO);
             boolean hasTrailingComma = hasTrailingToken(builder.getInput(), expressions, ",");
             builder.breakOp(hasTrailingComma ? FillMode.FORCED : FillMode.UNIFIED, "", ZERO);
             if (allowFilledElementsOnOwnLine) {
@@ -2154,7 +2155,11 @@ protected void visitBlock(
         } else {
             builder.open(ZERO);
             builder.open(plusTwo);
-            tokenBreakTrailingComment("{", plusTwo);
+            // OpsBuilder.build puts a comment that trails "{" right after the token, inside the plusTwo level, so a
+            // non-empty block adds no indent of its own: the comment lines up with the statements, and a second run
+            // leaves it there. An empty block closes the level right after "{", and build moves the comment past that
+            // close, so it keeps plusTwo to sit inside the braces.
+            tokenBreakTrailingComment("{", node.getStatements().isEmpty() ? plusTwo : ZERO);
             if (allowLeadingBlankLine == AllowLeadingBlankLine.NO) {
                 builder.blankLineWanted(BlankLineWanted.NO);
             } else {

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

@@ -636,4 +636,40 @@ void f(Map<String, Integer> map) {
 
         assertThat(formatter.formatSource(input)).isEqualTo(input);
     }
+
+    @Test
+    void trailingCommentAfterOpenBraceIsIdempotent() throws FormatterException {
+        // A block comment that trails `{` is broken onto its own line. It used to land one indent deeper than the
+        // statements that follow, and only a second run moved it back (google-java-format#1260, #129).
+        String input = """
+            class T {
+              void f() {
+                { /* c */ int x = 1; }
+              }
+
+              int[] a = { /* c */ 1 };
+            }
+            """;
+        String expected = """
+            class T {
+              void f() {
+                {
+                  /* c */
+                  int x = 1;
+                }
+              }
+
+              int[] a = {
+                /* c */
+                1
+              };
+            }
+            """;
+        Formatter formatter = Formatter.createFormatter(
+                JavaFormatterOptions.builder().style(Style.GOOGLE).build());
+
+        String once = formatter.formatSource(input);
+        assertThat(once).isEqualTo(expected);
+        assertThat(formatter.formatSource(once)).isEqualTo(once);
+    }
 }

open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/I1260.input

@@ -0,0 +1,18 @@
+class I1260 {
+  void block() {
+    { /* trailing on open brace */ int x = 1; }
+  }
+
+  void switchCase() {
+    switch (0) {
+      case 1: { /* Break so we don't hit fall-through warning: */ break;/* ignore STRING */
+      }
+    }
+  }
+
+  void lambda() {
+    Runnable r = () -> { /* trailing on lambda brace */ block(); };
+  }
+
+  int[] array = { /* trailing on array brace */ 1, 2 };
+}

open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/I1260.output

@@ -0,0 +1,29 @@
+class I1260 {
+    void block() {
+        {
+            /* trailing on open brace */
+            int x = 1;
+        }
+    }
+
+    void switchCase() {
+        switch (0) {
+            case 1: {
+                /* Break so we don't hit fall-through warning: */
+                break; /* ignore STRING */
+            }
+        }
+    }
+
+    void lambda() {
+        Runnable r = () -> {
+            /* trailing on lambda brace */
+            block();
+        };
+    }
+
+    int[] array = {
+        /* trailing on array brace */
+        1, 2
+    };
+}

open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/I1260EmptyBlocks.input

@@ -0,0 +1,13 @@
+class I1260EmptyBlocks {
+  void f() {
+    { /* c */ }
+    try { /* t */ } catch (Exception e) { /* c */ }
+    try { /* t */ } finally { /* f */ }
+    if (true) { /* i */ }
+    while (true) { /* w */ }
+    switch (0) { case 1: { /* s */ } }
+    int[] a = { /* a */ };
+  }
+  void g() { /* body */ }
+  void h() { /* body */ int x; }
+}

open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/I1260EmptyBlocks.output

@@ -0,0 +1,40 @@
+class I1260EmptyBlocks {
+    void f() {
+        {
+            /* c */
+        }
+        try {
+            /* t */
+        } catch (Exception e) {
+            /* c */
+        }
+        try {
+            /* t */
+        } finally {
+            /* f */
+        }
+        if (true) {
+            /* i */
+        }
+        while (true) {
+            /* w */
+        }
+        switch (0) {
+            case 1: {
+                /* s */
+            }
+        }
+        int[] a = {
+            /* a */
+        };
+    }
+
+    void g() {
+        /* body */
+    }
+
+    void h() {
+        /* body */
+        int x;
+    }
+}