From 4e65b065de6648c96051641b1f10ffdcaf19b95c Mon Sep 17 00:00:00 2001 From: Robin Stocker Date: Tue, 12 Mar 2024 16:27:19 +1100 Subject: [PATCH 1/2] Make Nodes.find throw when not found --- .../org/commonmark/test/ListBlockParserTest.java | 1 - .../src/test/java/org/commonmark/test/Nodes.java | 16 ++++++++++++++-- .../commonmark/test/ThematicBreakParserTest.java | 3 --- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/commonmark/src/test/java/org/commonmark/test/ListBlockParserTest.java b/commonmark/src/test/java/org/commonmark/test/ListBlockParserTest.java index a8a03fb7..2ba2116d 100644 --- a/commonmark/src/test/java/org/commonmark/test/ListBlockParserTest.java +++ b/commonmark/src/test/java/org/commonmark/test/ListBlockParserTest.java @@ -60,7 +60,6 @@ public void testOrderedListIndents() { private void assertListItemIndents(String input, int expectedMarkerIndent, int expectedContentIndent) { Node doc = PARSER.parse(input); ListItem listItem = Nodes.find(doc, ListItem.class); - assertNotNull(listItem); assertEquals(expectedMarkerIndent, listItem.getMarkerIndent()); assertEquals(expectedContentIndent, listItem.getContentIndent()); } diff --git a/commonmark/src/test/java/org/commonmark/test/Nodes.java b/commonmark/src/test/java/org/commonmark/test/Nodes.java index 25ce7583..8db50492 100644 --- a/commonmark/src/test/java/org/commonmark/test/Nodes.java +++ b/commonmark/src/test/java/org/commonmark/test/Nodes.java @@ -21,7 +21,7 @@ public static List getChildren(Node parent) { * @param parent The node to get children from (node itself will not be checked) * @param nodeClass The type of node to find */ - public static T find(Node parent, Class nodeClass) { + public static T tryFind(Node parent, Class nodeClass) { Node node = parent.getFirstChild(); while (node != null) { Node next = node.getNext(); @@ -29,7 +29,7 @@ public static T find(Node parent, Class nodeClass) { //noinspection unchecked return (T) node; } - T result = find(node, nodeClass); + T result = tryFind(node, nodeClass); if (result != null) { return result; } @@ -37,4 +37,16 @@ public static T find(Node parent, Class nodeClass) { } return null; } + + /** + * Recursively try to find a node with the given type within the children of the specified node. Throw if node + * could not be found. + */ + public static T find(Node parent, Class nodeClass) { + var node = tryFind(parent, nodeClass); + if (node == null) { + throw new IllegalArgumentException("Could not find a " + nodeClass.getSimpleName() + " node in " + parent); + } + return node; + } } diff --git a/commonmark/src/test/java/org/commonmark/test/ThematicBreakParserTest.java b/commonmark/src/test/java/org/commonmark/test/ThematicBreakParserTest.java index 91ac76f6..a6b297f8 100644 --- a/commonmark/src/test/java/org/commonmark/test/ThematicBreakParserTest.java +++ b/commonmark/src/test/java/org/commonmark/test/ThematicBreakParserTest.java @@ -2,8 +2,6 @@ import org.commonmark.node.ThematicBreak; import org.commonmark.parser.Parser; -import org.commonmark.renderer.html.HtmlRenderer; -import org.commonmark.testutil.RenderingTestCase; import org.junit.Test; import static org.junit.Assert.assertEquals; @@ -23,7 +21,6 @@ public void testLiteral() { private static void assertLiteral(String expected, String input) { var tb = Nodes.find(PARSER.parse(input), ThematicBreak.class); - assertNotNull(tb); assertEquals(expected, tb.getLiteral()); } } From 21fc1df7adfc6e8d3e876e56702a0adb81397a27 Mon Sep 17 00:00:00 2001 From: Robin Stocker Date: Tue, 12 Mar 2024 16:28:13 +1100 Subject: [PATCH 2/2] Fix LinkReferenceDefinition having null SourceSpan when title is present Fixes #292. --- .../LinkReferenceDefinitionParser.java | 6 ++++-- .../commonmark/parser/block/BlockParser.java | 4 ++++ .../org/commonmark/test/SourceSpansTest.java | 19 +++++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/commonmark/src/main/java/org/commonmark/internal/LinkReferenceDefinitionParser.java b/commonmark/src/main/java/org/commonmark/internal/LinkReferenceDefinitionParser.java index d11a6f22..ebe6ebb2 100644 --- a/commonmark/src/main/java/org/commonmark/internal/LinkReferenceDefinitionParser.java +++ b/commonmark/src/main/java/org/commonmark/internal/LinkReferenceDefinitionParser.java @@ -100,6 +100,10 @@ State getState() { } private boolean startDefinition(Scanner scanner) { + // Finish any outstanding references now. We don't do this earlier because we need addSourceSpan to have been + // called before we do it. + finishReference(); + scanner.whitespace(); if (!scanner.next('[')) { return false; @@ -205,7 +209,6 @@ private boolean startTitle(Scanner scanner) { title.append('\n'); } } else { - finishReference(); // There might be another reference instead, try that for the same character. state = State.START_DEFINITION; } @@ -235,7 +238,6 @@ private boolean title(Scanner scanner) { return false; } referenceValid = true; - finishReference(); paragraphLines.clear(); // See if there's another definition. diff --git a/commonmark/src/main/java/org/commonmark/parser/block/BlockParser.java b/commonmark/src/main/java/org/commonmark/parser/block/BlockParser.java index aa956a48..addd90d1 100644 --- a/commonmark/src/main/java/org/commonmark/parser/block/BlockParser.java +++ b/commonmark/src/main/java/org/commonmark/parser/block/BlockParser.java @@ -34,6 +34,10 @@ public interface BlockParser { BlockContinue tryContinue(ParserState parserState); + /** + * Add the part of a line that belongs to this block parser to parse (i.e. without any container block markers). + * Note that the line will only include a {@link SourceLine#getSourceSpan()} if source spans are enabled for inlines. + */ void addLine(SourceLine line); /** diff --git a/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java b/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java index a96d9c58..95959893 100644 --- a/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java +++ b/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java @@ -8,6 +8,7 @@ import java.util.ArrayDeque; import java.util.Arrays; import java.util.Deque; +import java.util.List; import static org.junit.Assert.assertEquals; @@ -164,6 +165,24 @@ public void linkReferenceDefinition() { assertEquals(Arrays.asList(SourceSpan.of(1, 0, 4)), paragraph.getSourceSpans()); } + @Test + public void linkReferenceDefinitionMultiple() { + var doc = PARSER.parse("[foo]: /foo\n[bar]: /bar\n"); + var def1 = (LinkReferenceDefinition) doc.getFirstChild(); + var def2 = (LinkReferenceDefinition) doc.getLastChild(); + assertEquals(List.of(SourceSpan.of(0, 0, 11)), def1.getSourceSpans()); + assertEquals(List.of(SourceSpan.of(1, 0, 11)), def2.getSourceSpans()); + } + + @Test + public void linkReferenceDefinitionWithTitle() { + var doc = PARSER.parse("[1]: #not-code \"Text\"\n[foo]: /foo\n"); + var def1 = (LinkReferenceDefinition) doc.getFirstChild(); + var def2 = (LinkReferenceDefinition) doc.getLastChild(); + assertEquals(List.of(SourceSpan.of(0, 0, 21)), def1.getSourceSpans()); + assertEquals(List.of(SourceSpan.of(1, 0, 11)), def2.getSourceSpans()); + } + @Test public void linkReferenceDefinitionHeading() { // This is probably the trickiest because we have a link reference definition at the start of a paragraph