Adding test cases to improve test coverage and refactoring to clean up design and implementation code smell

src/main/java/org/jsoup/internal/SharedConstants.java

@@ -11,6 +11,7 @@ public final class SharedConstants {
  public static final String EndRangeKey = "jsoup.end";
 
  public static final int DefaultBufferSize = 1024 * 32;
+ public static final int HashPrime = 31;
 
  private SharedConstants() {}
 }

src/main/java/org/jsoup/nodes/Attributes.java

@@ -555,20 +555,34 @@ public int deduplicate(ParseSettings settings) {
  return 0;
  boolean preserve = settings.preserveAttributeCase();
  int dupes = 0;
- OUTER: for (int i = 0; i < keys.length; i++) {
- for (int j = i + 1; j < keys.length; j++) {
- if (keys[j] == null)
- continue OUTER; // keys.length doesn't shrink when removing, so re-test
- if ((preserve && keys[i].equals(keys[j])) || (!preserve && keys[i].equalsIgnoreCase(keys[j]))) {
- dupes++;
- remove(j);
- j--;
- }
+
+ for (int i = 0; i < keys.length; i++) {
+ dupes += removeDuplicatesFrom(i, preserve);
+ }
+
+ return dupes;
+ }
+
+ private int removeDuplicatesFrom(int index, boolean preserve) {
+ int dupes = 0;
+
+ for (int j = index + 1; j < keys.length; j++) {
+ if (keys[j] == null)
+ break; // keys.length doesn't shrink when removing, so re-test
+
+ if (isDuplicate(keys[index], keys[j], preserve)) {
+ dupes++;
+ remove(j);
+ j--;
  }
  }
  return dupes;
  }
 
+ private boolean isDuplicate(String key1, String key2, boolean preserve) {
+ return preserve ? key1.equals(key2) : key1.equalsIgnoreCase(key2);
+ }
+
  private static class Dataset extends AbstractMap<String, String> {
  private final Attributes attributes;
 

src/main/java/org/jsoup/nodes/Element.java

@@ -185,7 +185,7 @@ public String normalName() {
  * @since 1.17.2
  */
  public boolean elementIs(String normalName, String namespace) {
- return tag.normalName().equals(normalName) && tag.namespace().equals(namespace);
+ return tag.matchesTagAttributes(normalName, namespace);
  }
 
  /**

src/main/java/org/jsoup/nodes/Range.java

@@ -109,7 +109,7 @@ public boolean equals(Object o) {
  @Override
  public int hashCode() {
  int result = start.hashCode();
- result = 31 * result + end.hashCode();
+ result = HashPrime * result + end.hashCode();
  return result;
  }
 
@@ -199,8 +199,8 @@ public boolean equals(Object o) {
  @Override
  public int hashCode() {
  int result = pos;
- result = 31 * result + lineNumber;
- result = 31 * result + columnNumber;
+ result = HashPrime * result + lineNumber;
+ result = HashPrime * result + columnNumber;
  return result;
  }
  }
@@ -246,7 +246,7 @@ public Range valueRange() {
 
  @Override public int hashCode() {
  int result = nameRange.hashCode();
- result = 31 * result + valueRange.hashCode();
+ result = HashPrime * result + valueRange.hashCode();
  return result;
  }
  }

src/main/java/org/jsoup/parser/Tag.java

@@ -53,6 +53,17 @@ public String namespace() {
  return namespace;
  }
 
+ /**
+ * Checks if the tag's normal name and namespace match the provided parameters.
+ *
+ * @param normalName The normalized name to check.
+ * @param namespace The namespace to check.
+ * @return true if the tag's normal name and namespace match the provided parameters, false otherwise.
+ */
+ public boolean matchesTagAttributes(String normalName, String namespace) {
+ return this.normalName().equals(normalName) && this.namespace().equals(namespace);
+ }
+
  /**
  * Get a Tag by name. If not previously defined (unknown), returns a new generic tag, that can do anything.
  * <p>

src/main/java/org/jsoup/select/QueryParser.java

@@ -5,7 +5,9 @@
 import org.jsoup.parser.TokenQueue;
 
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -106,35 +108,18 @@ private void combinator(char combinator) {
  evals.clear();
 
  // for most combinators: change the current eval into an AND of the current eval and the new eval
- switch (combinator) {
- case '>':
- ImmediateParentRun run = currentEval instanceof ImmediateParentRun ?
- (ImmediateParentRun) currentEval : new ImmediateParentRun(currentEval);
- run.add(newEval);
- currentEval = run;
- break;
- case ' ':
- currentEval = new CombiningEvaluator.And(new StructuralEvaluator.Parent(currentEval), newEval);
- break;
- case '+':
- currentEval = new CombiningEvaluator.And(new StructuralEvaluator.ImmediatePreviousSibling(currentEval), newEval);
- break;
- case '~':
- currentEval = new CombiningEvaluator.And(new StructuralEvaluator.PreviousSibling(currentEval), newEval);
- break;
- case ',':
- CombiningEvaluator.Or or;
- if (currentEval instanceof CombiningEvaluator.Or) {
- or = (CombiningEvaluator.Or) currentEval;
- } else {
- or = new CombiningEvaluator.Or();
- or.add(currentEval);
- }
- or.add(newEval);
- currentEval = or;
- break;
- default:
- throw new Selector.SelectorParseException("Unknown combinator '%s'", combinator);
+ Map<Character, CombinatorHandler> handlers = new HashMap<>();
+ handlers.put('>', new GreaterThanHandler());
+ handlers.put(' ', new SpaceHandler());
+ handlers.put('+', new PlusHandler());
+ handlers.put('~', new TildeHandler());
+ handlers.put(',', new CommaHandler());
+
+ CombinatorHandler handler = handlers.get(combinator);
+ if (handler != null) {
+ currentEval = handler.handle(currentEval, newEval);
+ } else {
+ throw new Selector.SelectorParseException("Unknown combinator '%s'", combinator);
  }
 
  if (replaceRightMost)
@@ -440,3 +425,53 @@ public String toString() {
  return query;
  }
 }
+
+interface CombinatorHandler {
+ Evaluator handle(Evaluator currentEval, Evaluator newEval);
+}
+
+class GreaterThanHandler implements CombinatorHandler {
+ @Override
+ public Evaluator handle(Evaluator currentEval, Evaluator newEval) {
+ ImmediateParentRun run = currentEval instanceof ImmediateParentRun ?
+ (ImmediateParentRun) currentEval : new ImmediateParentRun(currentEval);
+ run.add(newEval);
+ return run;
+ }
+}
+
+class SpaceHandler implements CombinatorHandler {
+ @Override
+ public Evaluator handle(Evaluator currentEval, Evaluator newEval) {
+ return new CombiningEvaluator.And(new StructuralEvaluator.Parent(currentEval), newEval);
+ }
+}
+
+class PlusHandler implements CombinatorHandler {
+ @Override
+ public Evaluator handle(Evaluator currentEval, Evaluator newEval) {
+ return new CombiningEvaluator.And(new StructuralEvaluator.ImmediatePreviousSibling(currentEval), newEval);
+ }
+}
+
+class TildeHandler implements CombinatorHandler {
+ @Override
+ public Evaluator handle(Evaluator currentEval, Evaluator newEval) {
+ return new CombiningEvaluator.And(new StructuralEvaluator.PreviousSibling(currentEval), newEval);
+ }
+}
+
+class CommaHandler implements CombinatorHandler {
+ @Override
+ public Evaluator handle(Evaluator currentEval, Evaluator newEval) {
+ CombiningEvaluator.Or or;
+ if (currentEval instanceof CombiningEvaluator.Or) {
+ or = (CombiningEvaluator.Or) currentEval;
+ } else {
+ or = new CombiningEvaluator.Or();
+ or.add(currentEval);
+ }
+ or.add(newEval);
+ return or;
+ }
+}

src/test/java/org/jsoup/helper/ValidateTest.java

@@ -43,4 +43,36 @@ public void testNotNull() {
  }
  assertTrue(threw);
  }
+ @Test
+ void testEnsureNotNullWithNonNullObject() {
+ Object nonNullObject = new Object();
+ assertEquals(nonNullObject, Validate.ensureNotNull(nonNullObject));
+ }
+
+ @Test
+ void testEnsureNotNullWithNullObject() {
+ Object nullObject = null;
+ assertThrows(ValidationException.class,() -> Validate.ensureNotNull(nullObject));
+ }
+
+ @Test
+ void testIsTrueWithFalseValue() {
+ assertThrows(ValidationException.class,() -> Validate.isTrue(false));
+ }
+
+ @Test
+ void testIsFalseWithTrueValue() {
+ assertThrows(ValidationException.class,() -> Validate.isFalse(true));
+ }
+
+ @Test
+ void testNoNullElementsWithNullElement() {
+ Object[] objects = { "notNull", null, "anotherNotNull" };
+ assertThrows(ValidationException.class, () -> Validate.noNullElements(objects, "Array must not contain null elements"));
+ }
+
+ @Test
+ void testSomeConditionThatShouldNotBeMet() {
+ assertThrows(ValidationException.class, () -> Validate.assertFail("This condition should not be met"));
+ }
 }

src/test/java/org/jsoup/nodes/AttributeTest.java

@@ -89,4 +89,14 @@ public void html() {
  Document doc2 = Jsoup.parse(html, Parser.htmlParser().settings(ParseSettings.preserveCase));
  assertEquals("<a href=\"autofocus\" REQUIRED>One</a>", doc2.selectFirst("a").outerHtml());
  }
+
+ @Test
+ void createFromEncoded() {
+ String unencodedKey = "src";
+ String encodedValue = "&lt;img src=&quot;/image.jpg&quot;&gt;";
+ Attribute attribute = Attribute.createFromEncoded(unencodedKey, encodedValue);
+ assertNotNull(attribute);
+ assertEquals(unencodedKey, attribute.getKey());
+ assertEquals("<img src=\"/image.jpg\">", attribute.getValue());
+ }
 }

src/test/java/org/jsoup/parser/PositionTest.java

@@ -1,19 +1,9 @@
 package org.jsoup.parser;
 
+import org.jsoup.Connection;
 import org.jsoup.Jsoup;
 import org.jsoup.integration.servlets.FileServlet;
-import org.jsoup.nodes.Attribute;
-import org.jsoup.nodes.CDataNode;
-import org.jsoup.nodes.Comment;
-import org.jsoup.nodes.DataNode;
-import org.jsoup.nodes.Document;
-import org.jsoup.nodes.DocumentType;
-import org.jsoup.nodes.Element;
-import org.jsoup.nodes.LeafNode;
-import org.jsoup.nodes.Node;
-import org.jsoup.nodes.Range;
-import org.jsoup.nodes.TextNode;
-import org.jsoup.nodes.XmlDeclaration;
+import org.jsoup.nodes.*;
 import org.jsoup.select.Elements;
 import org.junit.jupiter.api.Test;
 
@@ -533,6 +523,35 @@ private void printRange(Node node) {
  assertEquals(expectedRange, attr2.sourceRange().toString());
  }
 
+ @Test void hashCodeShouldBeConsistentWithEquals() {
+ String html = "<div one=\"Hello\nthere\" \nid=1 \nclass=\nfoo\nattr5 data-custom=\"123\">Text";
+ Document doc = Jsoup.parse(html, TrackingHtmlParser);
+
+ Element div = doc.expectFirst("div");
+
+ Range.AttributeRange classAttributeRange = div.attributes().sourceRange("class");
+ int classAttributeNameRangeHash = classAttributeRange.nameRange().hashCode();
+ int classAttributeValueRangeHash = classAttributeRange.valueRange().hashCode();
+
+ Range.AttributeRange customAttributeRange = div.attributes().sourceRange("data-custom");
+ int customAttributeRangeNameRangeHash = customAttributeRange.nameRange().hashCode();
+ int customAttributeRangeValueRangeHash = customAttributeRange.valueRange().hashCode();
+
+
+ assertEquals(31 * classAttributeNameRangeHash + classAttributeValueRangeHash, classAttributeRange.hashCode());
+ assertEquals(31 * customAttributeRangeNameRangeHash + customAttributeRangeValueRangeHash, customAttributeRange.hashCode());
+ }
+
+ @Test public void formDataWithNoSelectedOption() {
+ String html = "<form><select name='foo'><option value='one'><option value='two'><option value='three'></form>";
+ Document doc = Jsoup.parse(html);
+ FormElement form = (FormElement) doc.select("form").first();
+ List<Connection.KeyVal> data = form.formData();
+
+ assertEquals(1, data.size());
+ assertEquals("foo=one", data.get(0).toString());
+ }
+
  static void accumulateAttributePositions(Node node, StringBuilder sb) {
  if (node instanceof LeafNode) return; // leafnode pseudo attributes are not tracked
  for (Attribute attribute : node.attributes()) {