Repository: jhy/jsoup
Pull request #2095 (2023-12-30 07:29:35)

Improve equals and hashCode implementations using Objects


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/Attribute.java b/src/main/java/org/jsoup/nodes/Attribute.java
index 0d7c6b6b..6dcc37e9 100644
--- a/src/main/java/org/jsoup/nodes/Attribute.java
+++ b/src/main/java/org/jsoup/nodes/Attribute.java
@@ -10,6 +10,7 @@ import org.jspecify.annotations.Nullable;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Map;
+import java.util.Objects;
 import java.util.regex.Pattern;
 
 /**
@@ -253,15 +254,12 @@ public class Attribute implements Map.Entry<String, String>, Cloneable  {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         Attribute attribute = (Attribute) o;
-        if (key != null ? !key.equals(attribute.key) : attribute.key != null) return false;
-        return val != null ? val.equals(attribute.val) : attribute.val == null;
+        return Objects.equals(key, attribute.key) && Objects.equals(val, attribute.val);
     }
 
     @Override
     public int hashCode() { // note parent not considered
-        int result = key != null ? key.hashCode() : 0;
-        result = 31 * result + (val != null ? val.hashCode() : 0);
-        return result;
+        return Objects.hash(key, val);
     }
 
     @Override
diff --git a/src/main/java/org/jsoup/nodes/Attributes.java b/src/main/java/org/jsoup/nodes/Attributes.java
index ff2f0b99..0de6a9b6 100644
--- a/src/main/java/org/jsoup/nodes/Attributes.java
+++ b/src/main/java/org/jsoup/nodes/Attributes.java
@@ -19,6 +19,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
+import java.util.Objects;
 import java.util.Set;
 
 import static org.jsoup.internal.Normalizer.lowerCase;
@@ -496,14 +497,7 @@ public class Attributes implements Iterable<Attribute>, Cloneable {
         for (int i = 0; i < size; i++) {
             String key = keys[i];
             int thatI = that.indexOfKey(key);
-            if (thatI == NotFound)
-                return false;
-            Object val = vals[i];
-            Object thatVal = that.vals[thatI];
-            if (val == null) {
-                if (thatVal != null)
-                    return false;
-            } else if (!val.equals(thatVal))
+            if (thatI == NotFound || !Objects.equals(vals[i], that.vals[thatI]))
                 return false;
         }
         return true;
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/nodes/Range.java b/src/main/java/org/jsoup/nodes/Range.java
index ccc1db5f..1a665f88 100644
--- a/src/main/java/org/jsoup/nodes/Range.java
+++ b/src/main/java/org/jsoup/nodes/Range.java
@@ -1,5 +1,7 @@
 package org.jsoup.nodes;
 
+import java.util.Objects;
+
 import static org.jsoup.internal.SharedConstants.*;
 
 /**
@@ -108,9 +110,7 @@ public class Range {
 
     @Override
     public int hashCode() {
-        int result = start.hashCode();
-        result = 31 * result + end.hashCode();
-        return result;
+        return Objects.hash(start, end);
     }
 
     /**
@@ -198,10 +198,7 @@ public class Range {
 
         @Override
         public int hashCode() {
-            int result = pos;
-            result = 31 * result + lineNumber;
-            result = 31 * result + columnNumber;
-            return result;
+            return Objects.hash(pos, lineNumber, columnNumber);
         }
     }
 
@@ -245,9 +242,7 @@ public class Range {
         }
 
         @Override public int hashCode() {
-            int result = nameRange.hashCode();
-            result = 31 * result + valueRange.hashCode();
-            return result;
+            return Objects.hash(nameRange, valueRange);
         }
     }
 }
diff --git a/src/main/java/org/jsoup/parser/Tag.java b/src/main/java/org/jsoup/parser/Tag.java
index 20efd3db..5d8dbdbc 100644
--- a/src/main/java/org/jsoup/parser/Tag.java
+++ b/src/main/java/org/jsoup/parser/Tag.java
@@ -6,6 +6,7 @@ import org.jsoup.internal.SharedConstants;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 import java.util.function.Consumer;
 
 /**
@@ -233,15 +234,8 @@ public class Tag implements Cloneable {
 
     @Override
     public int hashCode() {
-        int result = tagName.hashCode();
-        result = 31 * result + (isBlock ? 1 : 0);
-        result = 31 * result + (formatAsBlock ? 1 : 0);
-        result = 31 * result + (empty ? 1 : 0);
-        result = 31 * result + (selfClosing ? 1 : 0);
-        result = 31 * result + (preserveWhitespace ? 1 : 0);
-        result = 31 * result + (formList ? 1 : 0);
-        result = 31 * result + (formSubmit ? 1 : 0);
-        return result;
+        return Objects.hash(tagName, isBlock, formatAsBlock, empty, selfClosing, preserveWhitespace,
+            formList, formSubmit);
     }
 
     @Override
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/safety/Safelist.java b/src/main/java/org/jsoup/safety/Safelist.java
index 629522fc..cb5038d0 100644
--- a/src/main/java/org/jsoup/safety/Safelist.java
+++ b/src/main/java/org/jsoup/safety/Safelist.java
@@ -16,6 +16,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 import static org.jsoup.internal.Normalizer.lowerCase;
@@ -646,21 +647,15 @@ public class Safelist {
 
         @Override
         public int hashCode() {
-            final int prime = 31;
-            int result = 1;
-            result = prime * result + ((value == null) ? 0 : value.hashCode());
-            return result;
+            return value.hashCode();
         }
 
         @Override
         public boolean equals(Object obj) {
             if (this == obj) return true;
-            if (obj == null) return false;
-            if (getClass() != obj.getClass()) return false;
+            if (obj == null || getClass() != obj.getClass()) return false;
             TypedValue other = (TypedValue) obj;
-            if (value == null) {
-                return other.value == null;
-            } else return value.equals(other.value);
+            return Objects.equals(value, other.value);
         }
 
         @Override
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"));
     }