Skip to content

Commit b7de448

Browse files
kluevergoogle-java-format Team
authored andcommitted
Prevent leading blank lines in list items when formatting block tags.
* Update `writeTableOpen`, `writePreOpen`, `writeBlockquoteOpenOrClose`, `writeMarkdownTable`, and `writeMarkdownFencedCodeBlock` in `JavadocWriter` to uniformly call `requestBlankLineForBlockTag()` (`if (wroteAnythingSignificant && !atStartOfLine)`), ensuring that block elements opening immediately at the start of a list item (`<li>` or `- `) do not get preceded by an unnecessary blank line. * Update `writeListOpen` and `writeListItemOpen` in `JavadocWriter` to check `if (wroteAnythingSignificant)` before requesting a newline or blank line, and set `atStartOfLine = true` in `writeBeginJavadoc` for `///` comments. This prevents `google-java-format` from prepending a leading blank line (`///\n`) when a list is the very first element in a Javadoc comment. * Fix a `NullPointerException` in `MarkdownPositions.visitListItem` when a Markdown list item has a non-`Paragraph` first child (such as `- <table>...</table>`), safely falling through when `firstChild` is not an instance of `Paragraph`. * Update unit tests in `JavadocFormattingTest` (`tableInHtmlListItem`, `tableInMarkdownListItem`, `tableInsideMarkdownListItemAfterText`, and `markdownLooseLists`) to verify correct formatting without unwanted blank lines or `NullPointerException` crashes. PiperOrigin-RevId: 945755137
1 parent 77bee2d commit b7de448

3 files changed

Lines changed: 49 additions & 41 deletions

File tree

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

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ void writeBeginJavadoc() {
114114
} else {
115115
output.append("/// ");
116116
remainingOnLine = JavadocFormatter.MAX_LINE_LENGTH - blockIndent - 4;
117+
atStartOfLine = true;
117118
}
118119
}
119120

@@ -190,7 +191,7 @@ void writeSnippetEnd(SnippetEnd token) {
190191
}
191192

192193
void writeListOpen(ListOpenTag token) {
193-
if (classicJavadoc) {
194+
if (classicJavadoc && wroteAnythingSignificant) {
194195
requestBlankLine();
195196
}
196197

@@ -200,7 +201,9 @@ void writeListOpen(ListOpenTag token) {
200201
continuingListStack.push(indent);
201202
postWriteModifiedContinuingListStack.push();
202203

203-
requestNewline();
204+
if (wroteAnythingSignificant) {
205+
requestNewline();
206+
}
204207
}
205208

206209
void writeListClose(ListCloseTag token) {
@@ -219,7 +222,9 @@ void writeListClose(ListCloseTag token) {
219222
}
220223

221224
void writeListItemOpen(ListItemOpenTag token) {
222-
requestNewline();
225+
if (wroteAnythingSignificant) {
226+
requestNewline();
227+
}
223228

224229
if (continuingListItemOfInnermostList) {
225230
continuingListItemOfInnermostList = false;
@@ -260,19 +265,15 @@ void writeParagraphOpen(Token token) {
260265
}
261266

262267
void writeBlockquoteOpenOrClose(Token token) {
263-
if (wroteAnythingSignificant) {
264-
requestBlankLine();
265-
}
268+
requestBlankLineForBlockTag();
266269

267270
writeToken(token);
268271

269272
requestBlankLine();
270273
}
271274

272275
void writePreOpen(PreOpenTag token) {
273-
if (wroteAnythingSignificant) {
274-
requestBlankLine();
275-
}
276+
requestBlankLineForBlockTag();
276277

277278
writeToken(token);
278279
}
@@ -292,9 +293,7 @@ void writeCodeClose(CodeCloseTag token) {
292293
}
293294

294295
void writeTableOpen(TableOpenTag token) {
295-
if (wroteAnythingSignificant) {
296-
requestBlankLine();
297-
}
296+
requestBlankLineForBlockTag();
298297

299298
writeToken(token);
300299
}
@@ -343,10 +342,8 @@ void writeLiteral(Literal token) {
343342
}
344343

345344
void writeMarkdownFencedCodeBlock(MarkdownFencedCodeBlock token) {
346-
if (wroteAnythingSignificant && !atStartOfLine) {
347-
// A reminder that atStartOfLine is still true after `-␣` because it is a StartOfLineToken.
348-
requestBlankLine();
349-
}
345+
// A reminder that atStartOfLine is still true after `-␣` because it is a StartOfLineToken.
346+
requestBlankLineForBlockTag();
350347
flushWhitespace();
351348
output.append(token.start());
352349
token
@@ -363,9 +360,7 @@ void writeMarkdownFencedCodeBlock(MarkdownFencedCodeBlock token) {
363360
}
364361

365362
void writeMarkdownTable(MarkdownTable token) {
366-
if (wroteAnythingSignificant && !atStartOfLine) {
367-
requestBlankLine();
368-
}
363+
requestBlankLineForBlockTag();
369364
flushWhitespace();
370365
List<String> lines = token.value().lines().toList();
371366
output.append(lines.get(0));
@@ -385,6 +380,12 @@ private void requestBlankLine() {
385380
requestWhitespace(BLANK_LINE);
386381
}
387382

383+
private void requestBlankLineForBlockTag() {
384+
if (wroteAnythingSignificant && !atStartOfLine) {
385+
requestBlankLine();
386+
}
387+
}
388+
388389
private void requestNewline() {
389390
requestWhitespace(NEWLINE);
390391
}

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -129,17 +129,17 @@ private boolean visitListItem(ListItem listItem) {
129129
verify(matcher.lookingAt());
130130
ListItemOpenTag openToken = new ListItemOpenTag(matcher.group(1));
131131
addSpan(listItem, openToken, LIST_ITEM_CLOSE_TOKEN);
132-
return switch (listItem.getFirstChild()) {
133-
case Paragraph paragraph -> {
134-
// A ListItem typically contains a Paragraph, but we don't want to visit that Paragraph
135-
// because that would lead us to introduce a line break after the list introduction
136-
// (the `-` or whatever). So we visit the children and siblings of the Paragraph instead.
137-
visitNodeList(paragraph.getFirstChild());
138-
visitNodeList(paragraph.getNext());
139-
yield true;
140-
}
141-
default -> false;
142-
};
132+
Node firstChild = listItem.getFirstChild();
133+
// Safely handles null and non-Paragraph children.
134+
if (firstChild instanceof Paragraph paragraph) {
135+
// A ListItem typically contains a Paragraph, but we don't want to visit that Paragraph
136+
// because that would lead us to introduce a line break after the list introduction
137+
// (the `-` or whatever). So we visit the children and siblings of the Paragraph instead.
138+
visitNodeList(paragraph.getFirstChild());
139+
visitNodeList(paragraph.getNext());
140+
return true;
141+
}
142+
return false;
143143
}
144144

145145
private void visitHeading(Heading heading) {

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

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1940,11 +1940,9 @@ public void markdownLooseLists() {
19401940
/// - item 2
19411941
class Test {}
19421942
""";
1943-
// TODO: the line break between items should be preserved, and there should not be a blank line
1944-
// before the list.
1943+
// TODO: the line break between items should be preserved.
19451944
String expected =
19461945
"""
1947-
///
19481946
/// - item 1
19491947
/// - item 2
19501948
class Test {}
@@ -2216,14 +2214,12 @@ public void tableInHtmlListItem() {
22162214
/// </ul>
22172215
class Test {}
22182216
""";
2219-
// requestBlankLine() in writeTableOpen() inserts a blank line after <li> before <table> when
2220-
// inside a list item.
2217+
// TODO(kak): The indentation here is still wrong (`<tr>` and `</table>` should be indented
2218+
// inside `<li>`). Also the blank line before </ul> should probably not be there?
22212219
String expected =
22222220
"""
22232221
/// <ul>
2224-
/// <li>
2225-
///
2226-
/// <table>
2222+
/// <li><table>
22272223
/// <tr><td>Foo</td></tr>
22282224
/// </table>
22292225
///
@@ -2233,6 +2229,20 @@ class Test {}
22332229
doFormatTest(input, expected);
22342230
}
22352231

2232+
@Test
2233+
public void tableInMarkdownListItem() {
2234+
assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();
2235+
String input =
2236+
"""
2237+
/// - <table>
2238+
/// <tr><td>Foo</td></tr>
2239+
/// </table>
2240+
class Test {}
2241+
""";
2242+
String expected = input;
2243+
doFormatTest(input, expected);
2244+
}
2245+
22362246
@Test
22372247
public void tableInsideMarkdownListItemAfterText() {
22382248
assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();
@@ -2244,11 +2254,8 @@ public void tableInsideMarkdownListItemAfterText() {
22442254
/// </table>
22452255
class Test {}
22462256
""";
2247-
// requestBlankLine() in writeTableOpen() inserts a blank line after item text before <table>
2248-
// inside the list item.
22492257
String expected =
22502258
"""
2251-
///
22522259
/// - item text
22532260
///
22542261
/// <table>

0 commit comments

Comments
 (0)