Only identify a /// comment as Markdown Javadoc if it is in a position where that makes sense.

The formatter currently recognizes and formats `/**` as Javadoc regardless of where it occurs. We could easily make it reformat only true Javadoc in that case too, by generalizing the logic here slightly. But the main motivation for the change here is that there may be a lot of "accidental Javadoc", since `///` comments only acquired their new meaning in Java 24.

PiperOrigin-RevId: 891072159

Author: eamonnmcmanus

core/src/main/java/com/google/googlejavaformat/java/Formatter.java

@@ -16,13 +16,15 @@
 
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterators;
 import com.google.common.collect.Range;
 import com.google.common.collect.RangeSet;
 import com.google.common.collect.TreeRangeSet;
 import com.google.common.io.CharSink;
 import com.google.common.io.CharSource;
 import com.google.errorprone.annotations.Immutable;
+import com.google.googlejavaformat.CommentsHelper;
 import com.google.googlejavaformat.Doc;
 import com.google.googlejavaformat.DocBuilder;
 import com.google.googlejavaformat.FormattingError;
@@ -109,13 +111,20 @@ static void format(final JavaInput javaInput, JavaOutput javaOutput, JavaFormatt
       throw FormatterException.fromJavacDiagnostics(errorDiagnostics);
     }
     OpsBuilder builder = new OpsBuilder(javaInput, javaOutput);
+    ImmutableSet.Builder<Integer> markdownJavadocPositions = ImmutableSet.builder();
     // Output the compilation unit.
-    JavaInputAstVisitor visitor = new JavaInputAstVisitor(builder, options.indentationMultiplier());
+    JavaInputAstVisitor visitor =
+        new JavaInputAstVisitor(builder, options.indentationMultiplier(), markdownJavadocPositions);
     visitor.scan(unit, null);
     builder.sync(javaInput.getText().length());
     builder.drain();
     Doc doc = new DocBuilder().withOps(builder.build()).build();
-    doc.computeBreaks(javaOutput.getCommentsHelper(), MAX_LINE_LENGTH, new Doc.State(+0, 0));
+    CommentsHelper commentsHelper =
+        new JavaCommentsHelper(
+            Newlines.guessLineSeparator(javaInput.getText()),
+            options,
+            markdownJavadocPositions.build());
+    doc.computeBreaks(commentsHelper, MAX_LINE_LENGTH, new Doc.State(+0, 0));
     doc.write(javaOutput);
     javaOutput.flush();
   }
@@ -209,7 +218,10 @@ public ImmutableList<Replacement> getFormatReplacements(
 
     String lineSeparator = Newlines.guessLineSeparator(input);
     JavaOutput javaOutput =
-        new JavaOutput(lineSeparator, javaInput, new JavaCommentsHelper(lineSeparator, options));
+        new JavaOutput(
+            lineSeparator,
+            javaInput,
+            new JavaCommentsHelper(lineSeparator, options, ImmutableSet.of()));
     try {
       format(javaInput, javaOutput, options);
     } catch (FormattingError e) {

core/src/main/java/com/google/googlejavaformat/java/JavaCommentsHelper.java

@@ -16,6 +16,7 @@
 
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableSet;
 import com.google.googlejavaformat.CommentsHelper;
 import com.google.googlejavaformat.Input.Tok;
 import com.google.googlejavaformat.Newlines;
@@ -31,10 +32,15 @@ final class JavaCommentsHelper implements CommentsHelper {
 
   private final String lineSeparator;
   private final JavaFormatterOptions options;
+  private final ImmutableSet<Integer> markdownJavadocPositions;
 
-  JavaCommentsHelper(String lineSeparator, JavaFormatterOptions options) {
+  JavaCommentsHelper(
+      String lineSeparator,
+      JavaFormatterOptions options,
+      ImmutableSet<Integer> markdownJavadocPositions) {
     this.lineSeparator = lineSeparator;
     this.options = options;
+    this.markdownJavadocPositions = markdownJavadocPositions;
   }
 
   @Override
@@ -56,7 +62,7 @@ public String rewrite(Tok tok, int maxWidth, int column0) {
       }
     }
     if (tok.isSlashSlashComment()) {
-      return indentLineComments(lines, column0);
+      return indentLineComments(tok, lines, column0);
     }
     return CommentsHelper.reformatParameterComment(tok)
         .orElseGet(
@@ -97,8 +103,8 @@ private String preserveIndentation(List<String> lines, int column0) {
   }
 
   // Wraps and re-indents line comments.
-  private String indentLineComments(List<String> lines, int column0) {
-    lines = wrapLineComments(lines, column0);
+  private String indentLineComments(Tok tok, List<String> lines, int column0) {
+    lines = wrapLineComments(tok, lines, column0);
     StringBuilder builder = new StringBuilder();
     builder.append(lines.get(0).trim());
     String indentString = Strings.repeat(" ", column0);
@@ -108,21 +114,17 @@ private String indentLineComments(List<String> lines, int column0) {
     return builder.toString();
   }
 
-  /** Probably a markdown comment, so don't try to wrap it. */
-  private static final Pattern MARKDOWN_JAVADOC_PREFIX = Pattern.compile("^///(\\s|$)");
-
   // Preserve special `//noinspection` and `//$NON-NLS-x$` comments used by IDEs, which cannot
   // contain leading spaces.
   private static final Pattern LINE_COMMENT_MISSING_SPACE_PREFIX =
       Pattern.compile("^(//+)(?!noinspection|\\$NON-NLS-\\d+\\$)[^\\s/]");
 
-  private List<String> wrapLineComments(List<String> lines, int column0) {
+  private List<String> wrapLineComments(Tok tok, List<String> lines, int column0) {
     List<String> result = new ArrayList<>();
     for (String line : lines) {
-      if (MARKDOWN_JAVADOC_PREFIX.matcher(line).find()) {
-        // Don't try to wrap comments that might be markdown javadoc.
-        // This is fairly approximate: a /// comment is only javadoc if it precedes a javadocable
-        // program element. But even if this isn't javadoc, it's not a disaster if we don't wrap it.
+      if (markdownJavadocPositions.contains(tok.getPosition())) {
+        // Don't wrap markdown comments. Eventually we will format them properly, but for now at
+        // least don't mangle them by wrapping with `// ` on the continuation lines.
         result.add(line);
         continue;
       }

core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java

@@ -170,6 +170,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.OptionalInt;
 import java.util.Set;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
@@ -270,6 +271,7 @@ private static ImmutableSetMultimap<String, String> typeAnnotations() {
   }
 
   private final OpsBuilder builder;
+  private final ImmutableSet.Builder<Integer> markdownJavadocPositions;
 
   private static final Indent.Const ZERO = Indent.Const.ZERO;
   private final int indentMultiplier;
@@ -306,9 +308,13 @@ private static final ImmutableList<Op> forceBreakList(Optional<BreakTag> breakTa
    *
    * @param builder the {@link OpsBuilder}
    */
-  JavaInputAstVisitor(OpsBuilder builder, int indentMultiplier) {
+  JavaInputAstVisitor(
+      OpsBuilder builder,
+      int indentMultiplier,
+      ImmutableSet.Builder<Integer> markdownJavadocPositions) {
     this.builder = builder;
     this.indentMultiplier = indentMultiplier;
+    this.markdownJavadocPositions = markdownJavadocPositions;
     minusTwo = Indent.Const.make(-2, indentMultiplier);
     minusFour = Indent.Const.make(-4, indentMultiplier);
     plusTwo = Indent.Const.make(+2, indentMultiplier);
@@ -396,7 +402,7 @@ private void handleModule(boolean afterFirstToken, CompilationUnitTree node) {
     }
   }
 
-  /** Skips over extra semi-colons at the top-level, or in a class member declaration lists. */
+  /** Skips over extra semicolons at the top-level, or in a class member declaration lists. */
   private void dropEmptyDeclarations() {
     if (builder.peekToken().equals(Optional.of(";"))) {
       while (builder.peekToken().equals(Optional.of(";"))) {
@@ -407,6 +413,16 @@ private void dropEmptyDeclarations() {
     }
   }
 
+  private void recordMarkdownJavadocPosition(Tree tree) {
+    OptionalInt javadocPosition = javadocPosition(tree);
+    if (javadocPosition.isPresent()) {
+      int pos = javadocPosition.getAsInt();
+      if (builder.getInput().getText().startsWith("///", pos)) {
+        markdownJavadocPositions.add(pos);
+      }
+    }
+  }
+
   // Replace with Flags.IMPLICIT_CLASS once JDK 25 is the minimum supported version
   private static final int IMPLICIT_CLASS = 1 << 19;
 
@@ -416,6 +432,7 @@ public Void visitClass(ClassTree tree, Void unused) {
       visitImplicitClass(tree);
       return null;
     }
+    recordMarkdownJavadocPosition(tree);
     switch (tree.getKind()) {
       case ANNOTATION_TYPE -> visitAnnotationType(tree);
       case CLASS, INTERFACE -> visitClassDeclaration(tree);
@@ -1020,6 +1037,9 @@ private void visitVariables(
       Direction annotationDirection) {
     if (fragments.size() == 1) {
       VariableTree fragment = fragments.get(0);
+      if (declarationKind == DeclarationKind.FIELD) {
+        recordMarkdownJavadocPosition(fragment);
+      }
       declareOne(
           declarationKind,
           annotationDirection,
@@ -1455,6 +1475,7 @@ public Void visitAnnotatedType(AnnotatedTypeTree node, Void unused) {
 
   @Override
   public Void visitMethod(MethodTree node, Void unused) {
+    recordMarkdownJavadocPosition(node);
     sync(node);
     List<? extends AnnotationTree> annotations = node.getModifiers().getAnnotations();
     List<? extends AnnotationTree> returnTypeAnnotations = ImmutableList.of();
@@ -2781,6 +2802,7 @@ public Void visitIdentifier(IdentifierTree node, Void unused) {
 
   @Override
   public Void visitModule(ModuleTree node, Void unused) {
+    recordMarkdownJavadocPosition(node);
     for (AnnotationTree annotation : node.getAnnotations()) {
       scan(annotation, null);
       builder.forcedBreak();
@@ -3941,18 +3963,34 @@ && getStartPosition(it.peek()) == start) {
     return fragments;
   }
 
-  /** Does this declaration have javadoc preceding it? */
-  private boolean hasJavaDoc(Tree bodyDeclaration) {
+  private OptionalInt javadocPosition(Tree bodyDeclaration) {
     int position = ((JCTree) bodyDeclaration).getStartPosition();
     Input.Token token = builder.getInput().getPositionTokenMap().get(position);
-    if (token != null) {
-      for (Input.Tok tok : token.getToksBefore()) {
-        if (tok.getText().startsWith("/**")) {
-          return true;
-        }
+    if (token == null) {
+      return OptionalInt.empty();
+    }
+    var toksBefore = token.getToksBefore();
+    // toksBefore is in source order. If there are several comments preceding bodyDeclaration, we
+    // want the last one that is a javadoc comment.
+    for (int i = toksBefore.size() - 1; i >= 0; i--) {
+      Input.Tok tok = toksBefore.get(i);
+      String text = tok.getText();
+      // TODO: consider making earlier versions behave compatibly. Prior to Java 23, there are no
+      // markdown javadoc comments, and /// is just a regular // comment that happens to start with
+      // an additional slash. As of Java 23, /// is markdown javadoc, and all consecutive /// lines
+      // are part of the same javac token. To ensure consistent formatting before and after this
+      // change, we would have to merge /// lines in a compatible way, including when we are
+      // extracting their text to make a comment that might be reformatted.
+      if (text.startsWith("/**") || (Runtime.version().feature() >= 23 && text.startsWith("///"))) {
+        return OptionalInt.of(tok.getPosition());
       }
     }
-    return false;
+    return OptionalInt.empty();
+  }
+
+  /** Does this declaration have javadoc preceding it? */
+  private boolean hasJavaDoc(Tree bodyDeclaration) {
+    return javadocPosition(bodyDeclaration).isPresent();
   }
 
   private static Optional<? extends Input.Token> getNextToken(Input input, int position) {

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

@@ -15,6 +15,7 @@
 package com.google.googlejavaformat.java;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.TruthJUnit.assume;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.io.ByteStreams;
@@ -25,6 +26,7 @@
 /** Tests formatting javadoc. */
 @RunWith(JUnit4.class)
 public final class JavadocFormattingTest {
+  private static final boolean MARKDOWN_JAVADOC_SUPPORTED = Runtime.version().feature() >= 23;
 
   private final Formatter formatter = new Formatter();
 
@@ -1599,38 +1601,55 @@ public void setSomething() {}
 
   @Test
   public void simpleMarkdown() {
+    assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();
     String input =
-        """
-        package com.example;
+"""
+package com.example;
 
-        /// # Heading
-        ///
-        /// A very long line of text, long enough that it will need to be wrapped to fit within the maximum line length.
-        class Test {
-          /// Another very long line of text, also long enough that it will need to be wrapped to fit within the maximum line length.
-          /// @param <T> a generic type
-          <T> T method() {
-            return null;
-          }
-        }\
-        """;
+/// # Heading
+///
+/// A very long line of text, long enough that it will need to be wrapped to fit within the maximum line length.
+class Test {
+  /// Another very long line of text, also long enough that it will need to be wrapped to fit within the maximum line length.
+  /// @param <T> a generic type
+  <T> T method() {
+    return null;
+  }
+
+  /// This long line of text looks like a javadoc comment, but is not, because it is separated from the actual javadoc comment by a plain comment.
+  // This is the plain comment.
+  /// A third very long line of text, this time a javadoc comment on a field, which again exceeds the maximum line length.
+  String field;
+
+  /// A fourth very long line of text, which however is not a javadoc comment so will be wrapped like a regular // comment.
+}\
+""";
     // TODO(emcmanus): Actually format the javadoc. For now, we just leave `///` lines alone, unlike
     // `//` lines which get wrapped.
     String expected =
-        """
-        package com.example;
-
-        /// # Heading
-        ///
-        /// A very long line of text, long enough that it will need to be wrapped to fit within the maximum line length.
-        class Test {
-          /// Another very long line of text, also long enough that it will need to be wrapped to fit within the maximum line length.
-          /// @param <T> a generic type
-          <T> T method() {
-            return null;
-          }
-        }
-        """;
+"""
+package com.example;
+
+/// # Heading
+///
+/// A very long line of text, long enough that it will need to be wrapped to fit within the maximum line length.
+class Test {
+  /// Another very long line of text, also long enough that it will need to be wrapped to fit within the maximum line length.
+  /// @param <T> a generic type
+  <T> T method() {
+    return null;
+  }
+
+  /// This long line of text looks like a javadoc comment, but is not, because it is separated from
+  // the actual javadoc comment by a plain comment.
+  // This is the plain comment.
+  /// A third very long line of text, this time a javadoc comment on a field, which again exceeds the maximum line length.
+  String field;
+
+  /// A fourth very long line of text, which however is not a javadoc comment so will be wrapped
+  // like a regular // comment.
+}
+""";
     doFormatTest(input, expected);
   }
 }