From adf925241b08885f841f1a034ca7fe3b2aac13a5 Mon Sep 17 00:00:00 2001 From: Kurt Alfred Kluever Date: Fri, 10 Jul 2026 11:56:54 -0700 Subject: [PATCH] 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: 945823851 --- .../java/javadoc/MarkdownPositions.java | 22 +++++----- .../java/JavadocFormattingTest.java | 41 ++++++++++++++----- 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/java/javadoc/MarkdownPositions.java b/core/src/main/java/com/google/googlejavaformat/java/javadoc/MarkdownPositions.java index 088f5ea58..274520761 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/javadoc/MarkdownPositions.java +++ b/core/src/main/java/com/google/googlejavaformat/java/javadoc/MarkdownPositions.java @@ -129,17 +129,15 @@ private boolean visitListItem(ListItem listItem) { verify(matcher.lookingAt()); ListItemOpenTag openToken = new ListItemOpenTag(matcher.group(1)); addSpan(listItem, openToken, LIST_ITEM_CLOSE_TOKEN); - return switch (listItem.getFirstChild()) { - case Paragraph paragraph -> { - // A ListItem typically contains a Paragraph, but we don't want to visit that Paragraph - // because that would lead us to introduce a line break after the list introduction - // (the `-` or whatever). So we visit the children and siblings of the Paragraph instead. - visitNodeList(paragraph.getFirstChild()); - visitNodeList(paragraph.getNext()); - yield true; - } - default -> false; - }; + if (listItem.getFirstChild() instanceof Paragraph paragraph) { + // A ListItem typically contains a Paragraph, but we don't want to visit that Paragraph + // because that would lead us to introduce a line break after the list introduction + // (the `-` or whatever). So we visit the children and siblings of the Paragraph instead. + visitNodeList(paragraph.getFirstChild()); + visitNodeList(paragraph.getNext()); + return true; + } + return false; } private void visitHeading(Heading heading) { @@ -258,5 +256,5 @@ public String toString() { // The leading \s here works around what appears to be a CommonMark bug. We shouldn't ever see // space at the purported start of a list item? private static final Pattern LIST_ITEM_START_PATTERN = - Pattern.compile("(?:\\s*)(([-+*]|[0-9]+[.)])\\s)"); + Pattern.compile("(?:\\s*)(([-+*]|[0-9]+[.)])(?:\\s|$))"); } diff --git a/core/src/test/java/com/google/googlejavaformat/java/JavadocFormattingTest.java b/core/src/test/java/com/google/googlejavaformat/java/JavadocFormattingTest.java index 267aacd73..eec7f43fb 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/JavadocFormattingTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/JavadocFormattingTest.java @@ -31,6 +31,21 @@ public final class JavadocFormattingTest { private final Formatter formatter = new Formatter(); + /** + * Tests that the formatter formats the given input string to the given expected string. Also + * tests that the formatter is idempotent when formatting an already formatted string. + */ + private void doFormatTest(String input, String expected) { + try { + String actual = formatter.formatSource(input); + assertThat(actual).isEqualTo(expected); + String reformatted = formatter.formatSource(actual); + assertWithMessage("When checking idempotency").that(reformatted).isEqualTo(actual); + } catch (FormatterException e) { + throw new AssertionError(e); + } + } + @Test public void notJavadoc() { String input = @@ -1739,6 +1754,21 @@ class Test {} doFormatTest(input, expected); } + @Test + public void markdownEmptyListItem() { + assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue(); + String input = +""" +/// A list with an empty item: +/// - `foo`: enabled by default +/// - `bar`: disabled by default +/// - +class Test {} +"""; + String expected = input; + doFormatTest(input, expected); + } + @Test public void markdownFencedCodeBlocks() { assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue(); @@ -2066,17 +2096,6 @@ class Test {} doFormatTest(input, expected); } - private void doFormatTest(String input, String expected) { - try { - String actual = formatter.formatSource(input); - assertThat(actual).isEqualTo(expected); - String reformatted = formatter.formatSource(actual); - assertWithMessage("When checking idempotency").that(reformatted).isEqualTo(actual); - } catch (FormatterException e) { - throw new AssertionError(e); - } - } - @Test public void markdownBlankLinesAroundSnippetAndNoMangling() { assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();