Skip to content

Commit eb495c7

Browse files
eamonnmcmanusgoogle-java-format Team
authored andcommitted
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: 953966721
1 parent cf859e3 commit eb495c7

4 files changed

Lines changed: 73 additions & 33 deletions

File tree

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

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.googlejavaformat.java.javadoc;
1616

17+
import static com.google.common.base.Preconditions.checkArgument;
1718
import static com.google.common.base.Preconditions.checkState;
1819
import static com.google.googlejavaformat.java.javadoc.JavadocLexer.lex;
1920
import static java.util.regex.Pattern.CASE_INSENSITIVE;
@@ -60,6 +61,7 @@
6061
import com.google.googlejavaformat.java.javadoc.Token.TableCloseTag;
6162
import com.google.googlejavaformat.java.javadoc.Token.TableOpenTag;
6263
import com.google.googlejavaformat.java.javadoc.Token.Whitespace;
64+
import java.util.ArrayList;
6365
import java.util.List;
6466
import java.util.regex.Matcher;
6567
import java.util.regex.Pattern;
@@ -90,7 +92,7 @@ public static String formatJavadoc(String input, int blockIndent) {
9092
default ->
9193
throw new IllegalArgumentException("Input does not start with /** or ///: " + input);
9294
};
93-
String inputForLexer = classicJavadoc ? input : ("///" + markdownCommentText(input));
95+
String inputForLexer = classicJavadoc ? classicCommentText(input) : markdownCommentText(input);
9496
ImmutableList<Token> tokens;
9597
try {
9698
tokens = lex(inputForLexer, classicJavadoc);
@@ -208,6 +210,45 @@ private static boolean oneLineJavadoc(String line, int blockIndent) {
208210

209211
private static final CharMatcher NOT_SPACE_OR_TAB = CharMatcher.noneOf(" \t");
210212

213+
private static final Pattern CLASSIC_PREFIX_PATTERN = Pattern.compile("^[ \\t]*[*][ \\t]?");
214+
215+
private static String stripJavadocBeginAndEnd(String input) {
216+
checkArgument(input.startsWith("/**"), "Missing /**: %s", input);
217+
checkArgument(input.endsWith("*/") && input.length() > 4, "Missing */: %s", input);
218+
return input.substring("/**".length(), input.length() - "*/".length());
219+
}
220+
221+
/**
222+
* Returns the given classic Javadoc comment after removing the leading ∕✱✱, trailing ✱∕, and any
223+
* leading asterisks and common leading whitespace on each line.
224+
*/
225+
private static String classicCommentText(String input) {
226+
String stripped = stripJavadocBeginAndEnd(input);
227+
List<String> lines = stripped.lines().toList();
228+
if (lines.isEmpty()) {
229+
// Can't happen: it would only happen for `/***/`, but we filter out comments starting `/***`.
230+
return "";
231+
}
232+
// The first line is handled specially in case we have something like `/** * foo\n * bar\n */`.
233+
// The end result should not strip the `*` from `* foo`.
234+
List<String> processedLines = new ArrayList<>();
235+
processedLines.add(lines.get(0));
236+
for (String line : lines.subList(1, lines.size())) {
237+
Matcher m = CLASSIC_PREFIX_PATTERN.matcher(line);
238+
if (m.find()) {
239+
processedLines.add(m.replaceFirst(""));
240+
} else {
241+
// Input line did not have leading `*`. In that case, it's hard to know what is supposed to
242+
// be indentation of the comment as a whole and what is supposed to be indentation of the
243+
// content. We just strip all leading whitespace.
244+
processedLines.add(line.stripLeading());
245+
}
246+
}
247+
// Unlike Markdown comments, stripping common leading whitespace is not mandated by any
248+
// specification. But it's not forbidden either.
249+
return stripCommonLeadingWhitespace(processedLines);
250+
}
251+
211252
/**
212253
* Returns the given string with the leading /// and any common leading whitespace removed from
213254
* each line. The resultant string can then be fed to a standard Markdown parser.
@@ -219,6 +260,10 @@ private static String markdownCommentText(String input) {
219260
.peek(line -> checkState(line.contains("///"), "Line does not contain ///: %s", line))
220261
.map(line -> line.substring(line.indexOf("///") + 3))
221262
.toList();
263+
return stripCommonLeadingWhitespace(lines);
264+
}
265+
266+
private static String stripCommonLeadingWhitespace(List<String> lines) {
222267
int leadingSpace =
223268
lines.stream()
224269
.filter(line -> NOT_SPACE_OR_TAB.matchesAnyOf(line))

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

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
package com.google.googlejavaformat.java.javadoc;
1616

17-
import static com.google.common.base.Preconditions.checkArgument;
1817
import static com.google.common.base.Preconditions.checkNotNull;
1918
import static com.google.common.base.Verify.verify;
2019
import static com.google.common.collect.Iterators.peekingIterator;
@@ -76,16 +75,8 @@ static ImmutableList<Token> lex(String input, boolean classicJavadoc) throws Lex
7675
input = normalizeLineEndings(input);
7776
MarkdownPositions markdownPositions;
7877
if (classicJavadoc) {
79-
/*
80-
* TODO(cpovirk): In theory, we should interpret Unicode escapes (yet output them in their
81-
* original form). This would mean mean everything from an encoded ∕✱✱ to an encoded <pre>
82-
* tag, so we'll probably never bother.
83-
*/
84-
input = stripJavadocBeginAndEnd(input);
8578
markdownPositions = MarkdownPositions.EMPTY;
8679
} else {
87-
checkArgument(input.startsWith("///"));
88-
input = input.substring("///".length());
8980
try {
9081
markdownPositions = MarkdownPositions.parse(input);
9182
} catch (UnsupportedOperationException e) {
@@ -104,16 +95,6 @@ private static String normalizeLineEndings(String input) {
10495

10596
private static final Pattern NON_UNIX_LINE_ENDING = Pattern.compile("\r\n?");
10697

107-
private static String stripJavadocBeginAndEnd(String input) {
108-
/*
109-
* We do this ahead of time so that the main part of the lexer need not say things like
110-
* "(?![*]/)" to avoid accidentally swallowing ✱∕ when consuming a newline.
111-
*/
112-
checkArgument(input.startsWith("/**"), "Missing /**: %s", input);
113-
checkArgument(input.endsWith("*/") && input.length() > 4, "Missing */: %s", input);
114-
return input.substring("/**".length(), input.length() - "*/".length());
115-
}
116-
11798
/**
11899
* An element of the nested contexts we might be in. For example, if we are inside {@code
119100
* <pre>{@code ...}</pre>} then the stack of nested contexts would be {@code PRE} plus {@code
@@ -248,8 +229,7 @@ private Token readToken() throws LexException {
248229
private Function<String, Token> consumeToken() throws LexException {
249230
boolean preserveExistingFormatting = preserveExistingFormatting();
250231

251-
Pattern newlinePattern = classicJavadoc ? CLASSIC_NEWLINE_PATTERN : MARKDOWN_NEWLINE_PATTERN;
252-
if (input.tryConsumeRegex(newlinePattern)) {
232+
if (input.tryConsumeRegex(NEWLINE_PATTERN)) {
253233
somethingSinceNewline = false;
254234
return preserveExistingFormatting ? ForcedNewline::new : Whitespace::new;
255235
}
@@ -687,13 +667,11 @@ static boolean hasMultipleNewlines(String s) {
687667
* We'd remove the trailing whitespace later on (in JavaCommentsHelper.rewrite), but I feel safer
688668
* stripping it now: It otherwise might confuse our line-length count, which we use for wrapping.
689669
*/
690-
private static final Pattern CLASSIC_NEWLINE_PATTERN = compile("[ \t]*\n[ \t]*[*]?[ \t]?");
691670
/*
692-
* With Traditional comments, the initial space and leading `*` characters (if any) are still
693-
* present in the input, but with Markdown comments, the leading `///` characters and shared
694-
* initial whitespace have been removed at the point where this pattern is applied.
671+
* The leading `///` or `*` characters and shared initial whitespace have been removed at the
672+
* point where this pattern is applied.
695673
*/
696-
private static final Pattern MARKDOWN_NEWLINE_PATTERN = compile("[ \t]*\n");
674+
private static final Pattern NEWLINE_PATTERN = compile("[ \t]*\n");
697675
private static final Pattern BLOCKQUOTE_MARKER_PATTERN = compile("> ?");
698676

699677
// We ensure elsewhere that we match this only at the beginning of a line.

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,13 @@ void writeMoeEndStripComment(MoeEndStripComment token) {
343343
void writeHtmlComment(HtmlComment token) {
344344
requestNewline();
345345

346-
writeToken(token);
346+
List<String> lines = token.value().lines().toList();
347+
writeToken(new HtmlComment(lines.get(0)));
348+
for (String line : lines.subList(1, lines.size())) {
349+
writeNewline(AutoIndent.NO_AUTO_INDENT);
350+
output.append(line);
351+
remainingOnLine -= line.length();
352+
}
347353

348354
requestNewline();
349355
}

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,6 @@ class Test {}
109109

110110
@Test
111111
public void commentMostlyUntouched() {
112-
// This test isn't necessarily what we'd want to do, but it's what we do now, and it's OK-ish.
113-
@SuppressWarnings("MisleadingEscapedSpace") // TODO(b/496180372): remove
114112
String input =
115113
"""
116114
/**
@@ -129,17 +127,31 @@ class Test {}\
129127
/**
130128
* Foo.
131129
* <!--
132-
*abc
130+
* abc
133131
* def
134132
* </tr>
135-
*-->
133+
* -->
136134
* bar
137135
*/
138136
class Test {}
139137
""";
140138
doFormatTest(input, expected);
141139
}
142140

141+
@Test
142+
public void markdownHtmlComment() {
143+
assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();
144+
String input =
145+
"""
146+
/// <!--
147+
/// abc
148+
/// -->
149+
class Test {}
150+
""";
151+
String expected = input;
152+
doFormatTest(input, expected);
153+
}
154+
143155
@Test
144156
public void moeComments() {
145157
// We replace moe by MOE to avoid triggering actual MOE rewriting.
@@ -2207,7 +2219,6 @@ public void markdownBlockQuoteInBlockTag() {
22072219
/// > To marry two wives at one time.
22082220
class Test {}
22092221
""";
2210-
// TODO(emcmanus): the blank lines here should not be present.
22112222
String expected =
22122223
"""
22132224
/// A test class.

0 commit comments

Comments
 (0)