Repository: jhy/jsoup
Pull request #2119 (2024-02-15 03:00:56)

Add uses of StringUtil.joining()


Patch:
diff --git a/CHANGES.md b/CHANGES.md
index 2995fe15..9db0887a 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -3,13 +3,19 @@
 ## 1.18.2 (Pending)
 
 ### Improvements
-* The form associated elements returned by `FormElement.elements()` now reflect changes made to the DOM, 
+
+* The form associated elements returned by `FormElement.elements()` now reflect changes made to the DOM,
   subsequently to the original parse. [2140](https://github.com/jhy/jsoup/issues/2140)
+* In the `TreeBuilder`, the `onNodeInserted()` and `onNodeClosed()` events are now also fired for the outermost /
+  root `Document` node. This enables source position tracking on the Document node (which was previously unset). And
+  it also enables the node traversor to see the outer Document node. [2182](https://github.com/jhy/jsoup/pull/2182)
 
 ### Bug Fixes
 
 * `Element.cssSelector()` would fail if the element's class contained a `*`
   character. [2169](https://github.com/jhy/jsoup/issues/2169)
+* When tracking source ranges, a text node following an invalid self-closing element may be left
+  untracked.[2175](https://github.com/jhy/jsoup/issues/2175)
 
 ## 1.18.1 (2024-Jul-10)
 
diff --git a/pom.xml b/pom.xml
index 7d88aac8..0639fc18 100644
--- a/pom.xml
+++ b/pom.xml
@@ -117,7 +117,7 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-javadoc-plugin</artifactId>
-        <version>3.7.0</version>
+        <version>3.8.0</version>
         <configuration>
           <doclint>none</doclint>
           <source>8</source>
@@ -228,7 +228,7 @@
         <!-- API version compat check - https://siom79.github.io/japicmp/ -->
         <groupId>com.github.siom79.japicmp</groupId>
         <artifactId>japicmp-maven-plugin</artifactId>
-        <version>0.21.2</version>
+        <version>0.22.0</version>
         <configuration>
           <!-- hard code previous version; can't detect when running stateless on build server -->
           <oldVersion>
@@ -432,7 +432,7 @@
       <!-- org.jspecify.annotations.nonnull, with Apache 2 license. Build time only. -->
       <groupId>org.jspecify</groupId>
       <artifactId>jspecify</artifactId>
-      <version>0.3.0</version>
+      <version>1.0.0</version>
       <scope>provided</scope>
     </dependency>
   </dependencies>
diff --git a/src/main/java/org/jsoup/nodes/Element.java b/src/main/java/org/jsoup/nodes/Element.java
index a947b2d2..d923bb1c 100644
--- a/src/main/java/org/jsoup/nodes/Element.java
+++ b/src/main/java/org/jsoup/nodes/Element.java
@@ -1430,17 +1430,15 @@ public class Element extends Node {
      @see #wholeOwnText()
      */
     public String wholeText() {
-        final StringBuilder accum = StringUtil.borrowBuilder();
-        nodeStream().forEach(node -> appendWholeText(node, accum));
-        return StringUtil.releaseBuilder(accum);
+        return wholeTextOf(nodeStream());
     }
 
-    private static void appendWholeText(Node node, StringBuilder accum) {
-        if (node instanceof TextNode) {
-            accum.append(((TextNode) node).getWholeText());
-        } else if (node.nameIs("br")) {
-            accum.append("\n");
-        }
+    private static String wholeTextOf(Stream<Node> stream) {
+        return stream.map(node -> {
+            if (node instanceof TextNode) return ((TextNode) node).getWholeText();
+            if (node.nameIs("br")) return "\n";
+            return "";
+        }).collect(StringUtil.joining(""));
     }
 
     /**
@@ -1453,14 +1451,7 @@ public class Element extends Node {
      @since 1.15.1
      */
     public String wholeOwnText() {
-        final StringBuilder accum = StringUtil.borrowBuilder();
-        final int size = childNodeSize();
-        for (int i = 0; i < size; i++) {
-            Node node = childNodes.get(i);
-            appendWholeText(node, accum);
-        }
-
-        return StringUtil.releaseBuilder(accum);
+        return wholeTextOf(childNodes.stream());
     }
 
     /**
diff --git a/src/main/java/org/jsoup/parser/Tokeniser.java b/src/main/java/org/jsoup/parser/Tokeniser.java
index 2416a66d..ff3bfa4f 100644
--- a/src/main/java/org/jsoup/parser/Tokeniser.java
+++ b/src/main/java/org/jsoup/parser/Tokeniser.java
@@ -49,8 +49,7 @@ final class Tokeniser {
     @Nullable private String lastStartTag; // the last start tag emitted, to test appropriate end tag
     @Nullable private String lastStartCloseSeq; // "</" + lastStartTag, so we can quickly check for that in RCData
 
-    private static final int Unset = -1;
-    private int markupStartPos, charStartPos = 0; // reader pos at the start of markup / characters. updated on state transition. Initialized to start (0), but set to Unset after emissions.
+    private int markupStartPos, charStartPos = 0; // reader pos at the start of markup / characters. markup updated on state transition, char on token emit.
 
     Tokeniser(TreeBuilder treeBuilder) {
         tagPending = startPending  = new Token.StartTag(treeBuilder);
@@ -90,7 +89,7 @@ final class Tokeniser {
         isEmitPending = true;
         token.startPos(markupStartPos);
         token.endPos(reader.pos());
-        charStartPos = Unset;
+        charStartPos = reader.pos(); // update char start when we complete a token emit
 
         if (token.type == Token.TokenType.StartTag) {
             Token.StartTag startTag = (Token.StartTag) token;
@@ -158,15 +157,9 @@ final class Tokeniser {
     }
 
     void transition(TokeniserState newState) {
-        // track markup / data position on state transitions
-        switch (newState) {
-            case TagOpen:
-                markupStartPos = reader.pos();
-                break;
-            case Data:
-                if (charStartPos == Unset) // don't reset when we are jumping between e.g data -> char ref -> data
-                    charStartPos = reader.pos();
-        }
+        // track markup position on state transitions
+        if (newState == TokeniserState.TagOpen)
+            markupStartPos = reader.pos();
 
         this.state = newState;
     }
diff --git a/src/main/java/org/jsoup/parser/TreeBuilder.java b/src/main/java/org/jsoup/parser/TreeBuilder.java
index fb5c0708..89ba8270 100644
--- a/src/main/java/org/jsoup/parser/TreeBuilder.java
+++ b/src/main/java/org/jsoup/parser/TreeBuilder.java
@@ -58,6 +58,7 @@ abstract class TreeBuilder {
         start = new Token.StartTag(this);
         currentToken = start; // init current token to the virtual start token.
         this.baseUri = baseUri;
+        onNodeInserted(doc);
     }
 
     void completeParse() {
@@ -108,7 +109,13 @@ abstract class TreeBuilder {
     boolean stepParser() {
         // if we have reached the end already, step by popping off the stack, to hit nodeRemoved callbacks:
         if (currentToken.type == Token.TokenType.EOF) {
-            if (stack == null || stack.isEmpty()) return false; // stack will be null if TB was closed, as in case of runParser() + completeFragment()
+            if (stack == null) {
+                return false;
+            } if (stack.isEmpty()) {
+                onNodeClosed(doc); // the root doc is not on the stack, so let this final step close it
+                stack = null;
+                return true;
+            }
             pop();
             return true;
         }
diff --git a/src/main/java/org/jsoup/select/Elements.java b/src/main/java/org/jsoup/select/Elements.java
index c344f2e0..1986c6e4 100644
--- a/src/main/java/org/jsoup/select/Elements.java
+++ b/src/main/java/org/jsoup/select/Elements.java
@@ -214,13 +214,9 @@ public class Elements extends ArrayList<Element> {
      * @see #eachText()
      */
     public String text() {
-        StringBuilder sb = StringUtil.borrowBuilder();
-        for (Element element : this) {
-            if (sb.length() != 0)
-                sb.append(" ");
-            sb.append(element.text());
-        }
-        return StringUtil.releaseBuilder(sb);
+        return stream()
+            .map(Element::text)
+            .collect(StringUtil.joining(" "));
     }
 
     /**
@@ -260,13 +256,9 @@ public class Elements extends ArrayList<Element> {
      * @see #outerHtml()
      */
     public String html() {
-        StringBuilder sb = StringUtil.borrowBuilder();
-        for (Element element : this) {
-            if (sb.length() != 0)
-                sb.append("\n");
-            sb.append(element.html());
-        }
-        return StringUtil.releaseBuilder(sb);
+        return stream()
+            .map(Element::html)
+            .collect(StringUtil.joining("\n"));
     }
     
     /**
@@ -276,13 +268,9 @@ public class Elements extends ArrayList<Element> {
      * @see #html()
      */
     public String outerHtml() {
-        StringBuilder sb = StringUtil.borrowBuilder();
-        for (Element element : this) {
-            if (sb.length() != 0)
-                sb.append("\n");
-            sb.append(element.outerHtml());
-        }
-        return StringUtil.releaseBuilder(sb);
+        return stream()
+            .map(Element::outerHtml)
+            .collect(StringUtil.joining("\n"));
     }
 
     /**


Test patch:
diff --git a/src/test/java/org/jsoup/parser/PositionTest.java b/src/test/java/org/jsoup/parser/PositionTest.java
index 49d544c5..9dd42971 100644
--- a/src/test/java/org/jsoup/parser/PositionTest.java
+++ b/src/test/java/org/jsoup/parser/PositionTest.java
@@ -487,6 +487,15 @@ class PositionTest {
         assertEquals("h1:0-9~12-17; id:4-6=7-8; #text:9-12; #text:17-18; h2:18-27~30-35; id:22-24=25-26; #text:27-30; h10:35-40~43-49; #text:40-43; ", track.toString());
     }
 
+    @Test void tracksAfterPSelfClose() {
+        // https://github.com/jhy/jsoup/issues/2175
+        String html = "foo<p/>bar &amp; 2";
+        Document doc = Jsoup.parse(html, TrackingHtmlParser);
+        StringBuilder track = new StringBuilder();
+        doc.body().forEachNode(node -> accumulatePositions(node, track));
+        assertEquals("body:0-0~18-18; #text:0-3; p:3-7~3-7; #text:7-18; ", track.toString());
+    }
+
     @Test void tracksFirstTextnode() {
         // https://github.com/jhy/jsoup/issues/2106
         String html = "foo<p></p>bar<p></p><div><b>baz</b></div>";
@@ -510,6 +519,22 @@ class PositionTest {
         assertEquals("class=\"On\"", attr.html());
     }
 
+    @Test void tracksDocument() {
+        String html = "<!doctype html><title>Foo</title><p>Bar.";
+        Document doc = Jsoup.parse(html, TrackingHtmlParser);
+        StringBuilder track = new StringBuilder();
+        doc.forEachNode(node -> accumulatePositions(node, track));
+        assertEquals("#document:0-0~40-40; #doctype:0-15; html:15-15~40-40; head:15-15~33-33; title:15-22~15-33; #text:22-25; body:33-33~40-40; p:33-36~40-40; #text:36-40; ", track.toString());
+    }
+
+    @Test void tracksDocumentXml() {
+        String html = "<!doctype html><title>Foo</title><p>Bar.";
+        Document doc = Jsoup.parse(html, TrackingXmlParser);
+        StringBuilder track = new StringBuilder();
+        doc.forEachNode(node -> accumulatePositions(node, track));
+        assertEquals("#document:0-0~40-40; #doctype:0-15; title:15-22~25-33; #text:22-25; p:33-36~40-40; #text:36-40; ", track.toString());
+    }
+
     @Test void updateKeyMaintainsRangeUc() {
         String html = "<p xsi:CLASS=On>One</p>";
         Document doc = Jsoup.parse(html, TrackingXmlParser);
diff --git a/src/test/java/org/jsoup/parser/StreamParserTest.java b/src/test/java/org/jsoup/parser/StreamParserTest.java
index bbbd620c..cebab0c4 100644
--- a/src/test/java/org/jsoup/parser/StreamParserTest.java
+++ b/src/test/java/org/jsoup/parser/StreamParserTest.java
@@ -36,7 +36,7 @@ class StreamParserTest {
             StringBuilder seen;
             seen = new StringBuilder();
             parser.stream().forEachOrdered(el -> trackSeen(el, seen));
-            assertEquals("title[Test];head+;div#1[D1]+;span[P One];p#3+;p#4[P Two];div#2[D2]+;p#6[P three];div#5[D3];body;html;", seen.toString());
+            assertEquals("title[Test];head+;div#1[D1]+;span[P One];p#3+;p#4[P Two];div#2[D2]+;p#6[P three];div#5[D3];body;html;#root;", seen.toString());
             // checks expected order, and the + indicates that element had a next sibling at time of emission
         }
     }
@@ -48,7 +48,7 @@ class StreamParserTest {
             StringBuilder seen;
             seen = new StringBuilder();
             parser.stream().forEachOrdered(el -> trackSeen(el, seen));
-            assertEquals("DIV#1[D1]+;span[P One];p#3+;p#4[P Two];div#2[D2]+;p#6[P three];div#5[D3];outmost;", seen.toString());
+            assertEquals("DIV#1[D1]+;span[P One];p#3+;p#4[P Two];div#2[D2]+;p#6[P three];div#5[D3];outmost;#root;", seen.toString());
             // checks expected order, and the + indicates that element had a next sibling at time of emission
         }
     }
@@ -64,7 +64,7 @@ class StreamParserTest {
             trackSeen(it.next(), seen);
         }
 
-        assertEquals("title[Test];head+;div#1[D1]+;span[P One];p#3+;p#4[P Two];div#2[D2]+;p#6[P three];div#5[D3];body;html;", seen.toString());
+        assertEquals("title[Test];head+;div#1[D1]+;span[P One];p#3+;p#4[P Two];div#2[D2]+;p#6[P three];div#5[D3];body;html;#root;", seen.toString());
         // checks expected order, and the + indicates that element had a next sibling at time of emission
     }
 
@@ -75,13 +75,13 @@ class StreamParserTest {
 
         StringBuilder seen = new StringBuilder();
         parser.stream().forEach(el -> trackSeen(el, seen));
-        assertEquals("head+;p[One]+;p[Two];body;html;", seen.toString());
+        assertEquals("head+;p[One]+;p[Two];body;html;#root;", seen.toString());
 
         String html2 = "<div>Three<div>Four</div></div>";
         StringBuilder seen2 = new StringBuilder();
         parser.parse(html2, "");
         parser.stream().forEach(el -> trackSeen(el, seen2));
-        assertEquals("head+;div[Four];div[Three];body;html;", seen2.toString());
+        assertEquals("head+;div[Four];div[Three];body;html;#root;", seen2.toString());
 
         // re-run without a new parse should be empty
         StringBuilder seen3 = new StringBuilder();
@@ -247,7 +247,7 @@ class StreamParserTest {
         StreamParser streamer = basic();
         assertFalse(isClosed(streamer));
         long count = streamer.stream().count();
-        assertEquals(6, count);
+        assertEquals(7, count);
 
         assertTrue(isClosed(streamer));
     }
@@ -261,7 +261,7 @@ class StreamParserTest {
             it.next();
             count++;
         }
-        assertEquals(6, count);
+        assertEquals(7, count);
         assertTrue(isClosed(streamer));
     }
 
@@ -384,7 +384,7 @@ class StreamParserTest {
         try (StreamParser parser = new StreamParser(Parser.htmlParser()).parseFragment(html, context, "")) {
             StringBuilder seen = new StringBuilder();
             parser.stream().forEachOrdered(el -> trackSeen(el, seen));
-            assertEquals("td[One];tr#1+;td[Two];tr#2+;td[Three];tr#3;tbody;table;", seen.toString());
+            assertEquals("td[One];tr#1+;td[Two];tr#2+;td[Three];tr#3;tbody;table;#root;", seen.toString());
             // checks expected order, and the + indicates that element had a next sibling at time of emission
             // note that we don't get a full doc, just the fragment (and the context at the end of the stack)
 
@@ -405,7 +405,7 @@ class StreamParserTest {
                 trackSeen(it.next(), seen);
             }
 
-            assertEquals("td[One];tr#1+;td[Two];tr#2+;td[Three];tr#3;tbody;table;", seen.toString());
+            assertEquals("td[One];tr#1+;td[Two];tr#2+;td[Three];tr#3;tbody;table;#root;", seen.toString());
             // checks expected order, and the + indicates that element had a next sibling at time of emission
             // note that we don't get a full doc, just the fragment (and the context at the end of the stack)
 
@@ -451,7 +451,7 @@ class StreamParserTest {
         try (StreamParser parser = new StreamParser(Parser.xmlParser()).parseFragment(html, context, "")) {
             StringBuilder seen = new StringBuilder();
             parser.stream().forEachOrdered(el -> trackSeen(el, seen));
-            assertEquals("td[One];tr#1+;td[Two];tr#2+;td[Three];tr#3;", seen.toString());
+            assertEquals("td[One];tr#1+;td[Two];tr#2+;td[Three];tr#3;#root;", seen.toString());
             // checks expected order, and the + indicates that element had a next sibling at time of emission
             // note that we don't get a full doc, just the fragment
 
diff --git a/src/test/java/org/jsoup/safety/CleanerTest.java b/src/test/java/org/jsoup/safety/CleanerTest.java
index 961a7636..078d80fa 100644
--- a/src/test/java/org/jsoup/safety/CleanerTest.java
+++ b/src/test/java/org/jsoup/safety/CleanerTest.java
@@ -395,6 +395,8 @@ public class CleanerTest {
         Element p = orig.expectFirst("p");
         Range origRange = p.sourceRange();
         assertEquals("2,2:22-2,10:30", origRange.toString());
+        assertEquals("1,1:0-1,1:0", orig.sourceRange().toString());
+        assertEquals("2,19:39-2,19:39", orig.endSourceRange().toString());
 
         Range.AttributeRange attributeRange = p.attributes().sourceRange("id");
         assertEquals("2,5:25-2,7:27=2,8:28-2,9:29", attributeRange.toString());
@@ -404,7 +406,7 @@ public class CleanerTest {
         assertEquals("1", cleanP.id());
         Range cleanRange = cleanP.sourceRange();
         assertEquals(origRange, cleanRange);
-        assertEquals(orig.endSourceRange(), clean.endSourceRange());
+        assertEquals(p.endSourceRange(), cleanP.endSourceRange());
         assertEquals(attributeRange, cleanP.attributes().sourceRange("id"));
     }