Skip to content

Commit ff77a3c

Browse files
kluevergoogle-java-format Team
authored andcommitted
Fix VerifyException in MarkdownPositions when formatting ordered list items with right parenthesis or plus bullet markers.
Update `LIST_ITEM_START_PATTERN` from `(?:\s*)((-|\*|[0-9]+\.)\s)` to `(?:\s*)(([-+*]|[0-9]+[.)])\s)`. This allows `TokenVisitor.visitListItem` to correctly recognize ordered list markers that use a right parenthesis (`)`) as permitted by Section 5.2 of the CommonMark spec (e.g., `1) foo`), as well as bullet lists using plus (`+ foo`). PiperOrigin-RevId: 943602624
1 parent 3bfe3d9 commit ff77a3c

3 files changed

Lines changed: 57 additions & 13 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,5 +258,5 @@ public String toString() {
258258
// The leading \s here works around what appears to be a CommonMark bug. We shouldn't ever see
259259
// space at the purported start of a list item?
260260
private static final Pattern LIST_ITEM_START_PATTERN =
261-
Pattern.compile("(?:\\s*)((-|\\*|[0-9]+\\.)\\s)");
261+
Pattern.compile("(?:\\s*)(([-+*]|[0-9]+[.)])\\s)");
262262
}

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

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@
1818
import static com.google.common.truth.Truth.assertWithMessage;
1919
import static com.google.common.truth.TruthJUnit.assume;
2020
import static java.nio.charset.StandardCharsets.UTF_8;
21-
import static org.junit.Assert.assertThrows;
2221

23-
import com.google.common.base.VerifyException;
2422
import com.google.common.io.ByteStreams;
2523
import org.junit.Test;
2624
import org.junit.runner.RunWith;
@@ -2124,19 +2122,32 @@ public void markdownLongCommentOnPackageDeclaration() {
21242122
doFormatTest(input, expected);
21252123
}
21262124

2127-
// TODO: b/346668798 - This should not throw an exception.
2128-
// CommonMark allows ')' as an ordered list marker (e.g. '1) foo'), but MarkdownPositions
2129-
// only expects '.' in LIST_ITEM_START_PATTERN, throwing a VerifyException.
21302125
@Test
21312126
public void markdownOrderedListWithParenthesis() {
21322127
assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();
21332128
String input =
21342129
"""
2130+
/// Summary paragraph.
21352131
/// 1) first item
21362132
/// 2) second item
21372133
class Test {}
21382134
""";
2139-
assertThrows(VerifyException.class, () -> formatter.formatSource(input));
2135+
String expected = input;
2136+
doFormatTest(input, expected);
2137+
}
2138+
2139+
@Test
2140+
public void markdownBulletListWithPlus() {
2141+
assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();
2142+
String input =
2143+
"""
2144+
/// Summary paragraph.
2145+
/// + first item
2146+
/// + second item
2147+
class Test {}
2148+
""";
2149+
String expected = input;
2150+
doFormatTest(input, expected);
21402151
}
21412152

21422153
// TODO: b/346668798 - Test the following Markdown constructs, and make the tests work as needed.

core/src/test/java/com/google/googlejavaformat/java/javadoc/MarkdownPositionsTest.java

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,7 @@
1515

1616
import static com.google.common.collect.ImmutableListMultimap.flatteningToImmutableListMultimap;
1717
import static com.google.common.truth.Truth.assertThat;
18-
import static org.junit.Assert.assertThrows;
1918

20-
import com.google.common.base.VerifyException;
2119
import com.google.common.collect.ImmutableListMultimap;
2220
import com.google.googlejavaformat.java.javadoc.Token.HeaderCloseTag;
2321
import com.google.googlejavaformat.java.javadoc.Token.HeaderOpenTag;
@@ -67,17 +65,52 @@ public void list() {
6765
assertThat(map).isEqualTo(expected);
6866
}
6967

70-
// TODO: b/346668798 - This should not throw an exception.
71-
// CommonMark allows ')' as an ordered list marker (e.g. '1) foo'), but MarkdownPositions
72-
// only expects '.' in LIST_ITEM_START_PATTERN, throwing a VerifyException.
7368
@Test
7469
public void listWithParenthesis() {
7570
String text =
7671
"""
7772
1) foo
7873
2) bar
7974
""";
80-
assertThrows(VerifyException.class, () -> MarkdownPositions.parse(text));
75+
var positions = MarkdownPositions.parse(text);
76+
ImmutableListMultimap<Integer, Token> map = positionToToken(positions, text);
77+
int firstItem = text.indexOf('1');
78+
int secondItem = text.indexOf('2');
79+
int end = text.length() - 1;
80+
ImmutableListMultimap<Integer, Token> expected =
81+
ImmutableListMultimap.<Integer, Token>builder()
82+
.put(firstItem, new ListOpenTag(""))
83+
.put(firstItem, new ListItemOpenTag("1) "))
84+
.put(secondItem - 1, new ListItemCloseTag(""))
85+
.put(secondItem, new ListItemOpenTag("2) "))
86+
.put(end, new ListItemCloseTag(""))
87+
.put(end, new ListCloseTag(""))
88+
.build();
89+
assertThat(map).isEqualTo(expected);
90+
}
91+
92+
@Test
93+
public void listWithPlus() {
94+
String text =
95+
"""
96+
+ foo
97+
+ bar
98+
""";
99+
var positions = MarkdownPositions.parse(text);
100+
ImmutableListMultimap<Integer, Token> map = positionToToken(positions, text);
101+
int firstItem = text.indexOf('+');
102+
int secondItem = text.lastIndexOf('+');
103+
int end = text.length() - 1;
104+
ImmutableListMultimap<Integer, Token> expected =
105+
ImmutableListMultimap.<Integer, Token>builder()
106+
.put(firstItem, new ListOpenTag(""))
107+
.put(firstItem, new ListItemOpenTag("+ "))
108+
.put(secondItem - 1, new ListItemCloseTag(""))
109+
.put(secondItem, new ListItemOpenTag("+ "))
110+
.put(end, new ListItemCloseTag(""))
111+
.put(end, new ListCloseTag(""))
112+
.build();
113+
assertThat(map).isEqualTo(expected);
81114
}
82115

83116
@Test

0 commit comments

Comments
 (0)