Skip to content

Commit ce80f77

Browse files
eamonnmcmanusgoogle-java-format Team
authored andcommitted
Abandon formatting of a Markdown Javadoc comment if it contains unsupported constructs.
This is much better than mangling the text of the comment. Of course, better still would be supporting the constructs, as hopefully we will eventually. PiperOrigin-RevId: 933718210
1 parent d964ff1 commit ce80f77

4 files changed

Lines changed: 73 additions & 46 deletions

File tree

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,10 @@ public static String formatJavadoc(String input, int blockIndent) {
8787
default ->
8888
throw new IllegalArgumentException("Input does not start with /** or ///: " + input);
8989
};
90-
if (!classicJavadoc) {
91-
input = "///" + markdownCommentText(input);
92-
}
90+
String inputForLexer = classicJavadoc ? input : ("///" + markdownCommentText(input));
9391
ImmutableList<Token> tokens;
9492
try {
95-
tokens = lex(input, classicJavadoc);
93+
tokens = lex(inputForLexer, classicJavadoc);
9694
} catch (LexException e) {
9795
return input;
9896
}

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,11 @@ static ImmutableList<Token> lex(String input, boolean classicJavadoc) throws Lex
8383
} else {
8484
checkArgument(input.startsWith("///"));
8585
input = input.substring("///".length());
86-
markdownPositions = MarkdownPositions.parse(input);
86+
try {
87+
markdownPositions = MarkdownPositions.parse(input);
88+
} catch (UnsupportedOperationException e) {
89+
throw new LexException(e);
90+
}
8791
}
8892
return new JavadocLexer(new CharStream(input), markdownPositions, classicJavadoc)
8993
.generateTokens();
@@ -709,5 +713,11 @@ private static Pattern closeTagPattern(String namePattern) {
709713
return compile(format("</(?:%s)\\b[^>]*>", namePattern), CASE_INSENSITIVE);
710714
}
711715

712-
static class LexException extends Exception {}
716+
static class LexException extends Exception {
717+
LexException() {}
718+
719+
LexException(Throwable cause) {
720+
super(cause);
721+
}
722+
}
713723
}

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

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,18 @@
3636
import java.util.regex.Pattern;
3737
import org.commonmark.ext.gfm.tables.TableBlock;
3838
import org.commonmark.ext.gfm.tables.TablesExtension;
39+
import org.commonmark.node.BlockQuote;
3940
import org.commonmark.node.BulletList;
4041
import org.commonmark.node.Code;
4142
import org.commonmark.node.FencedCodeBlock;
4243
import org.commonmark.node.Heading;
44+
import org.commonmark.node.IndentedCodeBlock;
45+
import org.commonmark.node.LinkReferenceDefinition;
4346
import org.commonmark.node.ListItem;
4447
import org.commonmark.node.Node;
4548
import org.commonmark.node.OrderedList;
4649
import org.commonmark.node.Paragraph;
50+
import org.commonmark.node.ThematicBreak;
4751
import org.commonmark.parser.IncludeSourceSpans;
4852
import org.commonmark.parser.Parser;
4953

@@ -90,7 +94,7 @@ private static class TokenVisitor {
9094
void visit(Node node) {
9195
boolean alreadyVisitedChildren = false;
9296
switch (node) {
93-
case Heading heading -> addSpan(heading, HEADER_OPEN_TOKEN, HEADER_CLOSE_TOKEN);
97+
case Heading heading -> visitHeading(heading);
9498
case Paragraph paragraph -> addSpan(paragraph, PARAGRAPH_OPEN_TOKEN, PARAGRAPH_CLOSE_TOKEN);
9599
case BulletList bulletList -> addSpan(bulletList, LIST_OPEN_TOKEN, LIST_CLOSE_TOKEN);
96100
case OrderedList orderedList -> addSpan(orderedList, LIST_OPEN_TOKEN, LIST_CLOSE_TOKEN);
@@ -101,6 +105,14 @@ void visit(Node node) {
101105
alreadyVisitedChildren = true;
102106
}
103107
case Code code -> visitCodeSpan(code);
108+
case BlockQuote blockQuote ->
109+
throw new UnsupportedOperationException("Block quotes not supported");
110+
case IndentedCodeBlock indentedCodeBlock ->
111+
throw new UnsupportedOperationException("Indented code blocks not supported");
112+
case ThematicBreak thematicBreak ->
113+
throw new UnsupportedOperationException("Thematic breaks not supported");
114+
case LinkReferenceDefinition linkReferenceDefinition ->
115+
throw new UnsupportedOperationException("Link reference definitions not supported");
104116
// TODO: others
105117
default -> {}
106118
}
@@ -130,6 +142,19 @@ private boolean visitListItem(ListItem listItem) {
130142
};
131143
}
132144

145+
private void visitHeading(Heading heading) {
146+
// There are two ways to spell a heading in Markdown:
147+
// 1. With hashes: `# Heading` `## Subheading`
148+
// 2. With dashes or underlines: `Heading\n=======` `Subheading\n-------`
149+
// We currently only support the first kind. We can distinguish the two cases by checking if
150+
// the heading text contains a newline.
151+
String s = nodeString(heading);
152+
if (s.contains("\n")) {
153+
throw new UnsupportedOperationException("Unsupported heading: " + s);
154+
}
155+
addSpan(heading, HEADER_OPEN_TOKEN, HEADER_CLOSE_TOKEN);
156+
}
157+
133158
private void visitFencedCodeBlock(FencedCodeBlock fencedCodeBlock) {
134159
// Any indentation before the code block is part of FencedCodeBlock. This makes sense
135160
// because the lines inside the code block must also be indented by that amount. That
@@ -205,6 +230,10 @@ private int endPosition(Node node) {
205230
var last = node.getSourceSpans().getLast();
206231
return last.getInputIndex() + last.getLength();
207232
}
233+
234+
private String nodeString(Node node) {
235+
return input.substring(startPosition(node), endPosition(node));
236+
}
208237
}
209238

210239
@Override

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

Lines changed: 29 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1860,17 +1860,14 @@ public void markdownThematicBreaks() {
18601860
/// foo
18611861
/// ***
18621862
/// bar
1863-
class Test {}
1864-
""";
1865-
// TODO: the line break before `***` should be preserved.
1866-
// It's OK to introduce a blank line before `bar` since it is a new paragraph.
1867-
String expected =
1868-
"""
1869-
/// foo ***
1863+
/// baz
1864+
///
18701865
///
1871-
/// bar
18721866
class Test {}
18731867
""";
1868+
// TODO: thematic breaks are not supported. That means the input is unchanged. We can see this
1869+
// because the blank lines at the end are preserved.
1870+
String expected = input;
18741871
doFormatTest(input, expected);
18751872
}
18761873

@@ -1883,21 +1880,14 @@ public void markdownSetextHeadings() {
18831880
/// =======
18841881
/// Phoebe B. Peabody-Beebe
18851882
///
1883+
///
18861884
/// Subheading
18871885
/// ----------
18881886
class Test {}
18891887
""";
1890-
// TODO: the line breaks before the lines of repeated characters should be preserved.
1891-
// Or, we could rewrite this style of heading as `# Heading`.
1892-
String expected =
1893-
"""
1894-
/// Heading =======
1895-
///
1896-
/// Phoebe B. Peabody-Beebe
1897-
///
1898-
/// Subheading ----------
1899-
class Test {}
1900-
""";
1888+
// TODO: This kind of heading is not supported. That means the input is unchanged.
1889+
// We can see this because the extra blank line is preserved.
1890+
String expected = input;
19011891
doFormatTest(input, expected);
19021892
}
19031893

@@ -1908,14 +1898,14 @@ public void markdownIndentedCodeBlocks() {
19081898
"""
19091899
/// code block
19101900
/// is indented
1901+
/// text after code block
1902+
///
1903+
///
19111904
class Test {}
19121905
""";
1913-
// TODO: the evil indented code block should be preserved.
1914-
String expected =
1915-
"""
1916-
/// code block is indented
1917-
class Test {}
1918-
""";
1906+
// TODO: evil indented code blocks like this are not supported. That means the input is
1907+
// unchanged. We can see this because the blank lines at the end are preserved.
1908+
String expected = input;
19191909
doFormatTest(input, expected);
19201910
}
19211911

@@ -1924,15 +1914,17 @@ public void markdownLinkReferenceDefinitions() {
19241914
assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();
19251915
String input =
19261916
"""
1927-
/// [foo]
1917+
/// [foo] [bar]
1918+
///
19281919
/// [foo]: /url "title"
1920+
/// [bar]: /url2 "title2"
1921+
///
1922+
///
19291923
class Test {}
19301924
""";
1931-
String expected =
1932-
"""
1933-
/// [foo] [foo]: /url "title"
1934-
class Test {}
1935-
""";
1925+
// TODO: link reference definitions are not supported. That means the input is unchanged. We can
1926+
// see this because the blank lines at the end are preserved.
1927+
String expected = input;
19361928
doFormatTest(input, expected);
19371929
}
19381930

@@ -1965,16 +1957,13 @@ public void markdownBlockQuotes() {
19651957
"""
19661958
/// > foo
19671959
/// > bar
1968-
class Test {}
1969-
""";
1970-
// TODO: the block quote should be preserved, and ideally bar would be joined to foo.
1971-
String expected =
1972-
"""
1973-
/// >
19741960
///
1975-
/// foo > bar
1961+
///
19761962
class Test {}
19771963
""";
1964+
// TODO: block quotes are not supported. That means the input is unchanged. We can see this
1965+
// because the extra blank lines at the end are preserved.
1966+
String expected = input;
19781967
doFormatTest(input, expected);
19791968
}
19801969

@@ -2014,10 +2003,11 @@ public void markdownAutolinks() {
20142003
assume().that(MARKDOWN_JAVADOC_SUPPORTED).isTrue();
20152004
String input =
20162005
"""
2017-
/// <http://{@code example.com> <br> is not inside @code}.
2006+
/// <http://{@code:example.com> <br> is not inside @code}.
20182007
class Test {}
20192008
""";
20202009
// TODO: the <br> should trigger a line break since it is not in fact inside {@code ...}.
2010+
// This is something of a corner case, though.
20212011
String expected = input;
20222012
doFormatTest(input, expected);
20232013
}

0 commit comments

Comments
 (0)