Skip to content

Commit 747bf1b

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 5498e81 commit 747bf1b

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 =
@@ -1713,6 +1728,21 @@ class Test {}
17131728
doFormatTest(input, expected);
17141729
}
17151730

1731+
@Test
1732+
public void markdownEmptyListItem() {
1733+
assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();
1734+
String input =
1735+
"""
1736+
/// A list with an empty item:
1737+
/// - `foo`: enabled by default
1738+
/// - `bar`: disabled by default
1739+
/// -
1740+
class Test {}
1741+
""";
1742+
String expected = input;
1743+
doFormatTest(input, expected);
1744+
}
1745+
17161746
@Test
17171747
public void markdownFencedCodeBlocks() {
17181748
assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();
@@ -2040,17 +2070,6 @@ class Test {}
20402070
doFormatTest(input, expected);
20412071
}
20422072

2043-
private void doFormatTest(String input, String expected) {
2044-
try {
2045-
String actual = formatter.formatSource(input);
2046-
assertThat(actual).isEqualTo(expected);
2047-
String reformatted = formatter.formatSource(actual);
2048-
assertWithMessage("When checking idempotency").that(reformatted).isEqualTo(actual);
2049-
} catch (FormatterException e) {
2050-
throw new AssertionError(e);
2051-
}
2052-
}
2053-
20542073
@Test
20552074
public void markdownBlankLinesAroundSnippetAndNoMangling() {
20562075
assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();

0 commit comments

Comments
 (0)