Skip to content

Commit 4586194

Browse files
kluevergoogle-java-format Team
authored andcommitted
Fix NullPointerException when visitListItem encounters a non-paragraph first child.
`visitListItem` in `MarkdownPositions` previously used a pattern-matching `switch` on `listItem.getFirstChild()`. If a Markdown `ListItem` starts with a non-paragraph child (such as when an empty or unfinished bullet item `/// -` has `null` as its first child), the `switch` statement throws a `NullPointerException` because it does not explicitly handle `null`. This change replaces the `switch` with `if (listItem.getFirstChild() instanceof Paragraph paragraph)` to safely return `false` on `null` or non-`Paragraph` children without throwing an exception. It also updates `LIST_ITEM_START_PATTERN` to allow list markers without trailing whitespace at the end of the input (`$`). Tested: Added `markdownEmptyListItem` in `JavadocFormattingTest.java` and ran `blaze test //third_party/java_src/google_java_format/...`. PiperOrigin-RevId: 945788396
1 parent 211ff6f commit 4586194

2 files changed

Lines changed: 40 additions & 23 deletions

File tree

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

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -129,17 +129,15 @@ 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+
if (listItem.getFirstChild() instanceof Paragraph paragraph) {
133+
// A ListItem typically contains a Paragraph, but we don't want to visit that Paragraph
134+
// because that would lead us to introduce a line break after the list introduction
135+
// (the `-` or whatever). So we visit the children and siblings of the Paragraph instead.
136+
visitNodeList(paragraph.getFirstChild());
137+
visitNodeList(paragraph.getNext());
138+
return true;
139+
}
140+
return false;
143141
}
144142

145143
private void visitHeading(Heading heading) {
@@ -258,5 +256,5 @@ public String toString() {
258256
// The leading \s here works around what appears to be a CommonMark bug. We shouldn't ever see
259257
// space at the purported start of a list item?
260258
private static final Pattern LIST_ITEM_START_PATTERN =
261-
Pattern.compile("(?:\\s*)(([-+*]|[0-9]+[.)])\\s)");
259+
Pattern.compile("(?:\\s*)(([-+*]|[0-9]+[.)])(?:\\s|$))");
262260
}

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

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,21 @@ public final class JavadocFormattingTest {
3131

3232
private final Formatter formatter = new Formatter();
3333

34+
/**
35+
* Tests that the formatter formats the given input string to the given expected string. Also
36+
* tests that the formatter is idempotent when formatting an already formatted string.
37+
*/
38+
private void doFormatTest(String input, String expected) {
39+
try {
40+
String actual = formatter.formatSource(input);
41+
assertThat(actual).isEqualTo(expected);
42+
String reformatted = formatter.formatSource(actual);
43+
assertWithMessage("When checking idempotency").that(reformatted).isEqualTo(actual);
44+
} catch (FormatterException e) {
45+
throw new AssertionError(e);
46+
}
47+
}
48+
3449
@Test
3550
public void notJavadoc() {
3651
String input =
@@ -1739,6 +1754,21 @@ class Test {}
17391754
doFormatTest(input, expected);
17401755
}
17411756

1757+
@Test
1758+
public void markdownEmptyListItem() {
1759+
assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();
1760+
String input =
1761+
"""
1762+
/// A list with an empty item:
1763+
/// - `foo`: enabled by default
1764+
/// - `bar`: disabled by default
1765+
/// -
1766+
class Test {}
1767+
""";
1768+
String expected = input;
1769+
doFormatTest(input, expected);
1770+
}
1771+
17421772
@Test
17431773
public void markdownFencedCodeBlocks() {
17441774
assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();
@@ -2066,17 +2096,6 @@ class Test {}
20662096
doFormatTest(input, expected);
20672097
}
20682098

2069-
private void doFormatTest(String input, String expected) {
2070-
try {
2071-
String actual = formatter.formatSource(input);
2072-
assertThat(actual).isEqualTo(expected);
2073-
String reformatted = formatter.formatSource(actual);
2074-
assertWithMessage("When checking idempotency").that(reformatted).isEqualTo(actual);
2075-
} catch (FormatterException e) {
2076-
throw new AssertionError(e);
2077-
}
2078-
}
2079-
20802099
@Test
20812100
public void markdownBlankLinesAroundSnippetAndNoMangling() {
20822101
assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();

0 commit comments

Comments
 (0)