Fix autolink extension to handle source spans correctly (fixes #209)

It didn't set any source spans on the Link nodes before, and even worse,
dropped source spans of the existing Text node (even if there were no
links).

Author: robinst

CHANGELOG.md

@@ -14,6 +14,7 @@ with the exception that 0.x versions can break between minor versions.
 - GitHub strikethrough: A single tilde now also works, and more than two
   tildes are not accepted anymore. This brings us in line with what
   GitHub actually does, which is a bit underspecified (#267)
+- The autolink extension now handles source spans correctly (#209)
 
 ## [0.19.0] - 2022-06-02
 ### Added

commonmark-ext-autolink/src/main/java/org/commonmark/ext/autolink/internal/AutolinkPostProcessor.java

@@ -1,16 +1,13 @@
 package org.commonmark.ext.autolink.internal;
 
-import org.commonmark.node.AbstractVisitor;
-import org.commonmark.node.Link;
-import org.commonmark.node.Node;
-import org.commonmark.node.Text;
+import org.commonmark.node.*;
 import org.commonmark.parser.PostProcessor;
 import org.nibor.autolink.LinkExtractor;
 import org.nibor.autolink.LinkSpan;
 import org.nibor.autolink.LinkType;
 import org.nibor.autolink.Span;
 
-import java.util.EnumSet;
+import java.util.*;
 
 public class AutolinkPostProcessor implements PostProcessor {
 
@@ -25,26 +22,49 @@ public Node process(Node node) {
         return node;
     }
 
-    private void linkify(Text textNode) {
-        String literal = textNode.getLiteral();
+    private void linkify(Text originalTextNode) {
+        String literal = originalTextNode.getLiteral();
 
-        Node lastNode = textNode;
+        Node lastNode = originalTextNode;
+        List<SourceSpan> sourceSpans = originalTextNode.getSourceSpans();
+        SourceSpan sourceSpan = sourceSpans.size() == 1 ? sourceSpans.get(0) : null;
 
-        for (Span span : linkExtractor.extractSpans(literal)) {
-            String text = literal.substring(span.getBeginIndex(), span.getEndIndex());
+        Iterator<Span> spans = linkExtractor.extractSpans(literal).iterator();
+        while (spans.hasNext()) {
+            Span span = spans.next();
+
+            if (lastNode == originalTextNode && !spans.hasNext() && !(span instanceof LinkSpan)) {
+                // Didn't find any links, don't bother changing existing node.
+                return;
+            }
+
+            Text textNode = createTextNode(literal, span, sourceSpan);
             if (span instanceof LinkSpan) {
-                String destination = getDestination((LinkSpan) span, text);
-                Text contentNode = new Text(text);
+                String destination = getDestination((LinkSpan) span, textNode.getLiteral());
+
                 Link linkNode = new Link(destination, null);
-                linkNode.appendChild(contentNode);
+                linkNode.appendChild(textNode);
+                linkNode.setSourceSpans(textNode.getSourceSpans());
                 lastNode = insertNode(linkNode, lastNode);
             } else {
-                lastNode = insertNode(new Text(text), lastNode);
+                lastNode = insertNode(textNode, lastNode);
             }
         }
 
         // Original node no longer needed
-        textNode.unlink();
+        originalTextNode.unlink();
+    }
+
+    private static Text createTextNode(String literal, Span span, SourceSpan sourceSpan) {
+        int beginIndex = span.getBeginIndex();
+        int endIndex = span.getEndIndex();
+        String text = literal.substring(beginIndex, endIndex);
+        Text textNode = new Text(text);
+        if (sourceSpan != null) {
+            int length = endIndex - beginIndex;
+            textNode.addSourceSpan(SourceSpan.of(sourceSpan.getLineIndex(), beginIndex, length));
+        }
+        return textNode;
     }
 
     private static String getDestination(LinkSpan linkSpan, String linkText) {

commonmark-ext-autolink/src/test/java/org/commonmark/ext/autolink/AutolinkTest.java

@@ -1,14 +1,21 @@
 package org.commonmark.ext.autolink;
 
 import org.commonmark.Extension;
+import org.commonmark.node.*;
+import org.commonmark.parser.IncludeSourceSpans;
 import org.commonmark.parser.Parser;
 import org.commonmark.renderer.html.HtmlRenderer;
 import org.commonmark.testutil.RenderingTestCase;
 import org.junit.Test;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.Set;
 
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
 public class AutolinkTest extends RenderingTestCase {
 
     private static final Set<Extension> EXTENSIONS = Collections.singleton(AutolinkExtension.create());
@@ -53,6 +60,59 @@ public void dontLinkTextWithinLinks() {
                 "<p><a href=\"http://example.com\">http://example.com</a></p>\n");
     }
 
+    @Test
+    public void sourceSpans() {
+        Parser parser = Parser.builder()
+                .extensions(EXTENSIONS)
+                .includeSourceSpans(IncludeSourceSpans.BLOCKS_AND_INLINES)
+                .build();
+        Node document = parser.parse("abc\n" +
+                "http://example.com/one\n" +
+                "def http://example.com/two\n" +
+                "ghi http://example.com/three jkl");
+
+        Paragraph paragraph = (Paragraph) document.getFirstChild();
+        Text abc = (Text) paragraph.getFirstChild();
+        assertEquals(Arrays.asList(SourceSpan.of(0, 0, 3)),
+                abc.getSourceSpans());
+
+        assertTrue(abc.getNext() instanceof SoftLineBreak);
+
+        Link one = (Link) abc.getNext().getNext();
+        assertEquals("http://example.com/one", one.getDestination());
+        assertEquals(Arrays.asList(SourceSpan.of(1, 0, 22)),
+                one.getSourceSpans());
+
+        assertTrue(one.getNext() instanceof SoftLineBreak);
+
+        Text def = (Text) one.getNext().getNext();
+        assertEquals("def ", def.getLiteral());
+        assertEquals(Arrays.asList(SourceSpan.of(2, 0, 4)),
+                def.getSourceSpans());
+
+        Link two = (Link) def.getNext();
+        assertEquals("http://example.com/two", two.getDestination());
+        assertEquals(Arrays.asList(SourceSpan.of(2, 4, 22)),
+                two.getSourceSpans());
+
+        assertTrue(two.getNext() instanceof SoftLineBreak);
+
+        Text ghi = (Text) two.getNext().getNext();
+        assertEquals("ghi ", ghi.getLiteral());
+        assertEquals(Arrays.asList(SourceSpan.of(3, 0, 4)),
+                ghi.getSourceSpans());
+
+        Link three = (Link) ghi.getNext();
+        assertEquals("http://example.com/three", three.getDestination());
+        assertEquals(Arrays.asList(SourceSpan.of(3, 4, 24)),
+                three.getSourceSpans());
+
+        Text jkl = (Text) three.getNext();
+        assertEquals(" jkl", jkl.getLiteral());
+        assertEquals(Arrays.asList(SourceSpan.of(3, 28, 4)),
+                jkl.getSourceSpans());
+    }
+
     @Override
     protected String render(String source) {
         return RENDERER.render(PARSER.parse(source));