From ecba4fb24fd064a64d2918c5fdb45546d08e3063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Fri, 7 Aug 2026 12:13:12 -0700 Subject: [PATCH] Make the logic for the two forms of Javadoc comments more similar. Previously, for Traditional comments (`/** ... */`) we retained the initial `*` that typically was present on each line of the input, but for Markdown comments (`/// ...`) we removed the initial characters (along with indentation common to all lines). Now we remove initial characters from both forms of comment. Also improve the formatting of HTML comments (``) in both forms of comment. PiperOrigin-RevId: 961056758 --- .../java/javadoc/JavadocFormatter.java | 47 ++++++++++++++++++- .../java/javadoc/JavadocLexer.java | 30 ++---------- .../java/javadoc/JavadocWriter.java | 8 +++- .../java/JavadocFormattingTest.java | 21 +++++++-- 4 files changed, 73 insertions(+), 33 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocFormatter.java b/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocFormatter.java index e2206a8d1..eeb8569ca 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocFormatter.java +++ b/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocFormatter.java @@ -14,6 +14,7 @@ package com.google.googlejavaformat.java.javadoc; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; import static com.google.googlejavaformat.java.javadoc.JavadocLexer.lex; import static java.util.regex.Pattern.CASE_INSENSITIVE; @@ -60,6 +61,7 @@ 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.ArrayList; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -90,7 +92,7 @@ public static String formatJavadoc(String input, int blockIndent) { default -> throw new IllegalArgumentException("Input does not start with /** or ///: " + input); }; - String inputForLexer = classicJavadoc ? input : ("///" + markdownCommentText(input)); + String inputForLexer = classicJavadoc ? classicCommentText(input) : markdownCommentText(input); ImmutableList tokens; try { tokens = lex(inputForLexer, classicJavadoc); @@ -208,6 +210,45 @@ private static boolean oneLineJavadoc(String line, int blockIndent) { private static final CharMatcher NOT_SPACE_OR_TAB = CharMatcher.noneOf(" \t"); + private static final Pattern CLASSIC_PREFIX_PATTERN = Pattern.compile("^[ \\t]*[*][ \\t]?"); + + private static String stripJavadocBeginAndEnd(String input) { + checkArgument(input.startsWith("/**"), "Missing /**: %s", input); + checkArgument(input.endsWith("*/") && input.length() > 4, "Missing */: %s", input); + return input.substring("/**".length(), input.length() - "*/".length()); + } + + /** + * Returns the given classic Javadoc comment after removing the leading ∕✱✱, trailing ✱∕, and any + * leading asterisks and common leading whitespace on each line. + */ + private static String classicCommentText(String input) { + String stripped = stripJavadocBeginAndEnd(input); + List lines = stripped.lines().toList(); + if (lines.isEmpty()) { + // Can't happen: it would only happen for `/***/`, but we filter out comments starting `/***`. + return ""; + } + // The first line is handled specially in case we have something like `/** * foo\n * bar\n */`. + // The end result should not strip the `*` from `* foo`. + List processedLines = new ArrayList<>(); + processedLines.add(lines.get(0)); + for (String line : lines.subList(1, lines.size())) { + Matcher m = CLASSIC_PREFIX_PATTERN.matcher(line); + if (m.find()) { + processedLines.add(m.replaceFirst("")); + } else { + // Input line did not have leading `*`. In that case, it's hard to know what is supposed to + // be indentation of the comment as a whole and what is supposed to be indentation of the + // content. We just strip all leading whitespace. + processedLines.add(line.stripLeading()); + } + } + // Unlike Markdown comments, stripping common leading whitespace is not mandated by any + // specification. But it's not forbidden either. + return stripCommonLeadingWhitespace(processedLines); + } + /** * Returns the given string with the leading /// and any common leading whitespace removed from * each line. The resultant string can then be fed to a standard Markdown parser. @@ -219,6 +260,10 @@ private static String markdownCommentText(String input) { .peek(line -> checkState(line.contains("///"), "Line does not contain ///: %s", line)) .map(line -> line.substring(line.indexOf("///") + 3)) .toList(); + return stripCommonLeadingWhitespace(lines); + } + + private static String stripCommonLeadingWhitespace(List lines) { int leadingSpace = lines.stream() .filter(line -> NOT_SPACE_OR_TAB.matchesAnyOf(line)) diff --git a/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocLexer.java b/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocLexer.java index 60a6054ee..c34b4f583 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocLexer.java +++ b/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocLexer.java @@ -14,7 +14,6 @@ package com.google.googlejavaformat.java.javadoc; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Verify.verify; import static com.google.common.collect.Iterators.peekingIterator; @@ -76,16 +75,8 @@ static ImmutableList lex(String input, boolean classicJavadoc) throws Lex input = normalizeLineEndings(input); MarkdownPositions markdownPositions; if (classicJavadoc) { - /* - * TODO(cpovirk): In theory, we should interpret Unicode escapes (yet output them in their - * original form). This would mean mean everything from an encoded ∕✱✱ to an encoded 
-       * tag, so we'll probably never bother.
-       */
-      input = stripJavadocBeginAndEnd(input);
       markdownPositions = MarkdownPositions.EMPTY;
     } else {
-      checkArgument(input.startsWith("///"));
-      input = input.substring("///".length());
       try {
         markdownPositions = MarkdownPositions.parse(input);
       } catch (UnsupportedOperationException e) {
@@ -104,16 +95,6 @@ private static String normalizeLineEndings(String input) {
 
   private static final Pattern NON_UNIX_LINE_ENDING = Pattern.compile("\r\n?");
 
-  private static String stripJavadocBeginAndEnd(String input) {
-    /*
-     * We do this ahead of time so that the main part of the lexer need not say things like
-     * "(?![*]/)" to avoid accidentally swallowing ✱∕ when consuming a newline.
-     */
-    checkArgument(input.startsWith("/**"), "Missing /**: %s", input);
-    checkArgument(input.endsWith("*/") && input.length() > 4, "Missing */: %s", input);
-    return input.substring("/**".length(), input.length() - "*/".length());
-  }
-
   /**
    * An element of the nested contexts we might be in. For example, if we are inside {@code
    * 
{@code ...}
} then the stack of nested contexts would be {@code PRE} plus {@code
@@ -248,8 +229,7 @@ private Token readToken() throws LexException {
   private Function consumeToken() throws LexException {
     boolean preserveExistingFormatting = preserveExistingFormatting();
 
-    Pattern newlinePattern = classicJavadoc ? CLASSIC_NEWLINE_PATTERN : MARKDOWN_NEWLINE_PATTERN;
-    if (input.tryConsumeRegex(newlinePattern)) {
+    if (input.tryConsumeRegex(NEWLINE_PATTERN)) {
       somethingSinceNewline = false;
       return preserveExistingFormatting ? ForcedNewline::new : Whitespace::new;
     }
@@ -687,13 +667,11 @@ static boolean hasMultipleNewlines(String s) {
    * We'd remove the trailing whitespace later on (in JavaCommentsHelper.rewrite), but I feel safer
    * stripping it now: It otherwise might confuse our line-length count, which we use for wrapping.
    */
-  private static final Pattern CLASSIC_NEWLINE_PATTERN = compile("[ \t]*\n[ \t]*[*]?[ \t]?");
   /*
-   * With Traditional comments, the initial space and leading `*` characters (if any) are still
-   * present in the input, but with Markdown comments, the leading `///` characters and shared
-   * initial whitespace have been removed at the point where this pattern is applied.
+   * The leading `///` or `*` characters and shared initial whitespace have been removed at the
+   * point where this pattern is applied.
    */
-  private static final Pattern MARKDOWN_NEWLINE_PATTERN = compile("[ \t]*\n");
+  private static final Pattern NEWLINE_PATTERN = compile("[ \t]*\n");
   private static final Pattern BLOCKQUOTE_MARKER_PATTERN = compile("> ?");
 
   // We ensure elsewhere that we match this only at the beginning of a line.
diff --git a/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocWriter.java b/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocWriter.java
index 061fdcc44..31759b7c3 100644
--- a/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocWriter.java
+++ b/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocWriter.java
@@ -343,7 +343,13 @@ void writeMoeEndStripComment(MoeEndStripComment token) {
   void writeHtmlComment(HtmlComment token) {
     requestNewline();
 
-    writeToken(token);
+    List lines = token.value().lines().toList();
+    writeToken(new HtmlComment(lines.get(0)));
+    for (String line : lines.subList(1, lines.size())) {
+      writeNewline(AutoIndent.NO_AUTO_INDENT);
+      output.append(line);
+      remainingOnLine -= line.length();
+    }
 
     requestNewline();
   }
diff --git a/core/src/test/java/com/google/googlejavaformat/java/JavadocFormattingTest.java b/core/src/test/java/com/google/googlejavaformat/java/JavadocFormattingTest.java
index 2ab647c9f..84387e61d 100644
--- a/core/src/test/java/com/google/googlejavaformat/java/JavadocFormattingTest.java
+++ b/core/src/test/java/com/google/googlejavaformat/java/JavadocFormattingTest.java
@@ -109,8 +109,6 @@ class Test {}
 
   @Test
   public void commentMostlyUntouched() {
-    // This test isn't necessarily what we'd want to do, but it's what we do now, and it's OK-ish.
-    @SuppressWarnings("MisleadingEscapedSpace") // TODO(b/496180372): remove
     String input =
         """
         /**
@@ -129,10 +127,10 @@ class Test {}\
         /**
          * Foo.
          * 
+         * -->
          * bar
          */
         class Test {}
@@ -140,6 +138,20 @@ class Test {}
     doFormatTest(input, expected);
   }
 
+  @Test
+  public void markdownHtmlComment() {
+    assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();
+    String input =
+        """
+        /// 
+        class Test {}
+        """;
+    String expected = input;
+    doFormatTest(input, expected);
+  }
+
   @Test
   public void moeComments() {
     // We replace moe by MOE to avoid triggering actual MOE rewriting.
@@ -2207,7 +2219,6 @@ public void markdownBlockQuoteInBlockTag() {
 /// > To marry two wives at one time.
 class Test {}
 """;
-    // TODO(emcmanus): the blank lines here should not be present.
     String expected =
 """
 /// A test class.