Handle blank lines before and within Markdown lists more accurately.

PiperOrigin-RevId: 956203301

Author: eamonnmcmanus

core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocFormatter.java

@@ -131,7 +131,7 @@ private static String render(List<Token> input, int blockIndent, boolean classic
         case MoeEndStripComment t -> output.writeMoeEndStripComment(t);
         case HtmlComment t -> output.writeHtmlComment(t);
         case BrTag t -> output.writeBr(standardizeBrToken(t));
-        case Whitespace unused -> output.requestWhitespace();
+        case Whitespace t -> output.requestWhitespaceOrBlankLine(t);
         case ForcedNewline unused -> output.writeLineBreakNoAutoIndent();
         case MarkdownHardLineBreak unused -> output.writeMarkdownHardLineBreak();
         case Literal t -> output.writeLiteral(t);

core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocLexer.java

@@ -388,9 +388,9 @@ private void checkMatchingTags() throws LexException {
    * ["<b>foo</b>"]}. See {@link #literalPattern()} for discussion of why those tokens are separate
    * to begin with.
    *
-   * <p>Whitespace tokens are treated analogously. We don't really "want" to join whitespace tokens,
-   * but in the course of joining literals, we incidentally join whitespace, too. We do take
-   * advantage of the joining later on: It simplifies {@link #inferParagraphTags}.
+   * <p>Whitespace tokens are treated analogously. The joining of whitespace tokens allows our
+   * Markdown output to detect "loose lists" and our Traditional output to {@linkplain
+   * #inferParagraphTags infer where to place paragraph tags}.
    *
    * <p>Note that we do <i>not</i> merge a literal token and a whitespace token together.
    */
@@ -419,14 +419,15 @@ private static ImmutableList<Token> joinAdjacentLiteralsAndAdjacentWhitespace(Li
        */
 
       if (accumulated.isEmpty()) {
-        output.add(tokens.next());
+        if (tokens.peek() instanceof Whitespace) {
+          output.add(new Whitespace(consumeAdjacentWhitespace(tokens)));
+        } else {
+          output.add(tokens.next());
+        }
         continue;
       }
 
-      StringBuilder seenWhitespace = new StringBuilder();
-      while (tokens.peek() instanceof Whitespace) {
-        seenWhitespace.append(tokens.next().value());
-      }
+      String seenWhitespace = consumeAdjacentWhitespace(tokens);
 
       if (tokens.peek() instanceof Literal literal && literal.value().startsWith("@")) {
         // OK, we're in the case described above.
@@ -439,10 +440,10 @@ private static ImmutableList<Token> joinAdjacentLiteralsAndAdjacentWhitespace(Li
       accumulated.setLength(0);
 
       if (!seenWhitespace.isEmpty()) {
-        output.add(new Whitespace(seenWhitespace.toString()));
+        output.add(new Whitespace(seenWhitespace));
       }
 
-      // We have another token coming, possibly of type OTHER. Leave it for the next iteration.
+      // We have another token coming. Leave it for the next iteration.
     }
 
     /*
@@ -452,6 +453,14 @@ private static ImmutableList<Token> joinAdjacentLiteralsAndAdjacentWhitespace(Li
     return output.build();
   }
 
+  private static String consumeAdjacentWhitespace(PeekingIterator<Token> tokens) {
+    StringBuilder seenWhitespace = new StringBuilder();
+    while (tokens.peek() instanceof Whitespace) {
+      seenWhitespace.append(tokens.next().value());
+    }
+    return seenWhitespace.toString();
+  }
+
   /**
    * Where the input has two consecutive line breaks between literals, insert a {@code <p>} tag
    * between the literals.
@@ -635,7 +644,7 @@ private static void deindentPreCodeBlock(
 
   private static final CharMatcher NEWLINE = CharMatcher.is('\n');
 
-  private static boolean hasMultipleNewlines(String s) {
+  static boolean hasMultipleNewlines(String s) {
     return NEWLINE.countIn(s) > 1;
   }
 

core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocWriter.java

@@ -47,6 +47,7 @@
 import com.google.googlejavaformat.java.javadoc.Token.StartOfLineToken;
 import com.google.googlejavaformat.java.javadoc.Token.TableCloseTag;
 import com.google.googlejavaformat.java.javadoc.Token.TableOpenTag;
+import com.google.googlejavaformat.java.javadoc.Token.Whitespace;
 import java.util.List;
 
 /**
@@ -100,6 +101,23 @@ private void requestWhitespace(RequestedWhitespace requestedWhitespace) {
     this.requestedWhitespace = max(requestedWhitespace, this.requestedWhitespace);
   }
 
+  /**
+   * Requests whitespace or a blank line depending on the whitespace token.
+   *
+   * <p>In Markdown Javadoc, if the whitespace token contains multiple newlines, it represents a
+   * blank line in the input (e.g., between a paragraph and a list, or between loose list items). We
+   * want to preserve these blank lines, so we request a blank line. Otherwise, or in classic
+   * Javadoc (where blank lines are handled via inferred {@code <p>} tags), we just request standard
+   * whitespace.
+   */
+  void requestWhitespaceOrBlankLine(Whitespace token) {
+    if (!classicJavadoc && JavadocLexer.hasMultipleNewlines(token.value())) {
+      requestBlankLine();
+    } else {
+      requestWhitespace();
+    }
+  }
+
   void requestMoeBeginStripComment(MoeBeginStripComment token) {
     // We queue this up so that we can put it after any requested whitespace.
     requestedMoeBeginStripComment = checkNotNull(token);

core/src/test/java/com/google/googlejavaformat/java/JavadocFormattingTest.java

@@ -1745,6 +1745,7 @@ class Test {}
 ///   1. nested thing 1 on more than one line
 ///   2. nested thing 2 on only one line but which is long enough that it is going to need to be
 ///      wrapped
+///
 ///   3. nested thing 3 after a blank line
 ///
 /// A following paragraph.
@@ -2081,13 +2082,7 @@ public void markdownLooseLists() {
         /// - item 2
         class Test {}
         """;
-    // TODO: the line break between items should be preserved.
-    String expected =
-        """
-        /// - item 1
-        /// - item 2
-        class Test {}
-        """;
+    String expected = input;
     doFormatTest(input, expected);
   }
 
@@ -2104,16 +2099,7 @@ public void markdownListPrecedingBlankLine() {
         /// - Item 2.
         class Test {}
         """;
-    // TODO(b/534219145): The blank line before the list should be preserved.
-    String expected =
-        """
-        /// Title.
-        ///
-        /// Some paragraph.
-        /// - Item 1.
-        /// - Item 2.
-        class Test {}
-        """;
+    String expected = input;
     doFormatTest(input, expected);
   }
 
@@ -2433,13 +2419,6 @@ class Test {}
   //   [foo]: /url "title"
   //   https://spec.commonmark.org/0.31.2/#link-reference-definitions
   //
-  // - Loose lists
-  //   "A list is loose if any of its constituent list items are separated by blank lines, or if any
-  //   of its constituent list items directly contain two block-level elements with a blank line
-  //   between them."
-  //   We should test that we do not remove blank lines from a loose list, which would make it a
-  //   tight one. https://spec.commonmark.org/0.31.2/#loose
-  //
   // - Block quotes
   //   > foo
   //   > bar