Fix parents($selector) so the selector filters the ancestors themselves

CHANGELOG.md

@@ -4,7 +4,10 @@ QueryPath Changelog
 # Unreleased changes
 
 - **Breaking behaviour change:** `DOMQuery::is()` now behaves like jQuery's `.is()`. It tests the elements held in the current match set and returns `true` when at least one of them matches the selector. Previously it ran a descendant search, so `html5qp('<p><span>foo</span></p>', 'p')->is('span')` returned `true`. Use `has()` if you need the old "does the set contain something matching this selector" behaviour (#51)
-- `DOMQuery::is()` no longer raises a fatal error when the match set contains non-element nodes (text nodes, comments, processing instructions). Those nodes cannot match a CSS selector, so they are skipped
+- `DOMQuery::is()` no longer raises a fatal error when the match set contains non-element nodes (text nodes, comments, processing instructions). Those nodes cannot match a CSS selector, so they are skipped (#51)
+- Fix `parents($selector)` so the selector filters the ancestors themselves, instead of matching any ancestor that merely *contains* an element matching the selector. `qp($xml, 'Demographics > Age > Name')->parents('Demographics')` now returns only `<Demographics>`, matching jQuery (#62)
+- Apply the same fix to the other selector-filtered traversal methods in `QueryPath\Helpers\QueryFilters`: `parent()`, `parents()`, `parentsUntil()`, `closest()`, `next()`, `nextAll()`, `nextUntil()`, `prev()`, `prevAll()`, `prevUntil()`, `siblings()`, and `not()` (#62)
+- **Behaviour change:** `parents()` and `parentsUntil()` now return their results in reverse document order with duplicates removed, as jQuery does. Previously a set built from more than one starting element was grouped by starting element (#62)
 - Reorganise, modernise, and repair the `examples/` directory. Each example now lives in its own subdirectory with an `index.php`, and the full set is indexed in `examples/quickstart-guide.md`
 - Convert the remaining legacy examples: `simple_example.php`, `techniques.php`, `svg.php`, `rss.php`, `odt.php`, `parse_php.php`, and `sparql.php`
 - Fix examples that no longer ran: send a `User-Agent` where remote hosts now require one, resolve paths relative to the example rather than the working directory, and stop relying on the removed `qp.php` autoloader and the PHP 8 incompatible `eachLambda()`

src/Helpers/QueryFilters.php

@@ -3,12 +3,12 @@
 namespace QueryPath\Helpers;
 
 use DOMElement;
+use DOMNode;
 use QueryPath\CSS\DOMTraverser;
 use QueryPath\CSS\ParseException;
 use QueryPath\DOMQuery;
 use QueryPath\Exception;
 use QueryPath\Query;
-use QueryPath\QueryPath;
 use SplObjectStorage;
 use stdClass;
 
@@ -577,7 +577,7 @@ public function nextUntil($selector = null): Query
 			while (isset($m->nextSibling)) {
 				$m = $m->nextSibling;
 				if ($m->nodeType === XML_ELEMENT_NODE) {
-					if (null !== $selector && QueryPath::with($m, null, $this->options)->is($selector) > 0) {
+					if (null !== $selector && $this->matchesNodeSelector($m, $selector)) {
 						break;
 					}
 					$found->offsetSet($m);
@@ -616,7 +616,7 @@ public function prevUntil($selector = null): Query
 			while (isset($m->previousSibling)) {
 				$m = $m->previousSibling;
 				if ($m->nodeType === XML_ELEMENT_NODE) {
-					if (null !== $selector && QueryPath::with($m, null, $this->options)->is($selector)) {
+					if (null !== $selector && $this->matchesNodeSelector($m, $selector)) {
 						break;
 					}
 
@@ -654,7 +654,7 @@ public function parentsUntil($selector = null): Query
 				// Is there any case where parent node is not an element?
 				if ($m->nodeType === XML_ELEMENT_NODE) {
 					if (! empty($selector)) {
-						if (QueryPath::with($m, null, $this->options)->is($selector) > 0) {
+						if ($this->matchesNodeSelector($m, $selector)) {
 							break;
 						}
 						$found->offsetSet($m);
@@ -665,7 +665,7 @@ public function parentsUntil($selector = null): Query
 			}
 		}
 
-		return $this->inst($found, null);
+		return $this->inst($this->sortReverseDocumentOrder($found), null);
 	}
 
 	/**
@@ -726,7 +726,7 @@ public function not($selector): Query
 			}
 		} else {
 			foreach ($this->matches as $m) {
-				if (! QueryPath::with($m, null, $this->options)->is($selector)) {
+				if (! $this->matchesNodeSelector($m, $selector)) {
 					$found->offsetSet($m);
 				}
 			}
@@ -756,17 +756,13 @@ public function closest($selector): Query
 	{
 		$found = new SplObjectStorage();
 		foreach ($this->matches as $m) {
-			if (QueryPath::with($m, null, $this->options)->is($selector) > 0) {
+			if ($this->matchesNodeSelector($m, $selector)) {
 				$found->offsetSet($m);
 			} else {
 				while ($m->parentNode->nodeType !== XML_DOCUMENT_NODE) {
 					$m = $m->parentNode;
 					// Is there any case where parent node is not an element?
-					if ($m->nodeType === XML_ELEMENT_NODE && QueryPath::with(
-						$m,
-						null,
-						$this->options
-					)->is($selector) > 0) {
+					if ($this->matchesNodeSelector($m, $selector)) {
 						$found->offsetSet($m);
 						break;
 					}
@@ -844,7 +840,7 @@ private function getParentElements(?string $selector, bool $immediate): Query
 				// Is there any case where parent node is not an element?
 				if ($m->nodeType === XML_ELEMENT_NODE) {
 					if (! empty($selector)) {
-						if (QueryPath::with($m, null, $this->options)->is($selector) > 0) {
+						if ($this->matchesNodeSelector($m, $selector)) {
 							$found->offsetSet($m);
 							if ($immediate) {
 								break;
@@ -860,6 +856,13 @@ private function getParentElements(?string $selector, bool $immediate): Query
 			}
 		}
 
+		// jQuery returns the ancestors of a multi-element set in reverse
+		// document order, with duplicates removed. parent() keeps the
+		// legacy per-element ordering.
+		if (! $immediate) {
+			$found = $this->sortReverseDocumentOrder($found);
+		}
+
 		return $this->inst($found, null);
 	}
 
@@ -890,7 +893,7 @@ public function next($selector = null): Query
 				$m = $m->nextSibling;
 				if ($m->nodeType === XML_ELEMENT_NODE) {
 					if (! empty($selector)) {
-						if (QueryPath::with($m, null, $this->options)->is($selector) > 0) {
+						if ($this->matchesNodeSelector($m, $selector)) {
 							$found->offsetSet($m);
 							break;
 						}
@@ -932,7 +935,7 @@ public function nextAll($selector = null): Query
 				$m = $m->nextSibling;
 				if ($m->nodeType === XML_ELEMENT_NODE) {
 					if (! empty($selector)) {
-						if (QueryPath::with($m, null, $this->options)->is($selector) > 0) {
+						if ($this->matchesNodeSelector($m, $selector)) {
 							$found->offsetSet($m);
 						}
 					} else {
@@ -973,7 +976,7 @@ public function prev($selector = null): Query
 				$m = $m->previousSibling;
 				if ($m->nodeType === XML_ELEMENT_NODE) {
 					if (! empty($selector)) {
-						if (QueryPath::with($m, null, $this->options)->is($selector)) {
+						if ($this->matchesNodeSelector($m, $selector)) {
 							$found->offsetSet($m);
 							break;
 						}
@@ -1015,7 +1018,7 @@ public function prevAll($selector = null): Query
 				$m = $m->previousSibling;
 				if ($m->nodeType === XML_ELEMENT_NODE) {
 					if (! empty($selector)) {
-						if (QueryPath::with($m, null, $this->options)->is($selector)) {
+						if ($this->matchesNodeSelector($m, $selector)) {
 							$found->offsetSet($m);
 						}
 					} else {
@@ -1143,14 +1146,124 @@ public function siblings($selector = null): Query
 			$parent = $m->parentNode;
 			foreach ($parent->childNodes as $n) {
 				if ($n->nodeType === XML_ELEMENT_NODE && $n !== $m) {
+					if (! empty($selector) && ! $this->matchesNodeSelector($n, $selector)) {
+						continue;
+					}
+
 					$found->offsetSet($n);
 				}
 			}
 		}
-		if (empty($selector)) {
-			return $this->inst($found, null);
+
+		return $this->inst($found, null);
+	}
+
+	/**
+	 * Test whether a single node, taken as an element, matches a CSS selector.
+	 *
+	 * The node itself is the only candidate, so this asks "is this node a match?"
+	 * rather than "does this node contain a match?" — which is what running a
+	 * find() against the node would ask.
+	 *
+	 * @param DOMNode $node
+	 *   The node to test.
+	 * @param string  $selector
+	 *   A valid CSS selector.
+	 *
+	 * @return bool
+	 *   TRUE if the node is an element and matches the selector.
+	 * @throws ParseException
+	 * @see NodeMatcher
+	 */
+	private function matchesNodeSelector($node, $selector): bool
+	{
+		return NodeMatcher::matchesNode($node, $selector);
+	}
+
+	/**
+	 * Sort a set of nodes into reverse document order.
+	 *
+	 * jQuery's ancestor traversal methods (parents(), parentsUntil()) return
+	 * their results in reverse document order with duplicates removed. Because
+	 * the results are accumulated per source element, a set built from more than
+	 * one starting element would otherwise be grouped by source element instead.
+	 *
+	 * @param SplObjectStorage $nodes
+	 *   The nodes to sort. Duplicates are already removed by SplObjectStorage.
+	 *
+	 * @return SplObjectStorage
+	 *   The same nodes, in reverse document order.
+	 */
+	private function sortReverseDocumentOrder(SplObjectStorage $nodes): SplObjectStorage
+	{
+		if (count($nodes) < 2) {
+			return $nodes;
+		}
+
+		$indexed = [];
+		foreach ($nodes as $node) {
+			$indexed[] = [$this->documentOrderPath($node), $node];
+		}
+
+		usort($indexed, function ($a, $b) {
+			// Reverse document order, so the comparison operands are swapped.
+			return $this->compareDocumentOrderPaths($b[0], $a[0]);
+		});
+
+		$sorted = new SplObjectStorage();
+		foreach ($indexed as $entry) {
+			$sorted->offsetSet($entry[1]);
+		}
+
+		return $sorted;
+	}
+
+	/**
+	 * Build a comparable representation of a node's position in its document.
+	 *
+	 * The path is the list of child offsets from the document down to the node,
+	 * which can be compared element by element to determine document order.
+	 *
+	 * @param DOMNode $node
+	 *
+	 * @return array
+	 */
+	private function documentOrderPath($node): array
+	{
+		$path = [];
+		while ($node instanceof DOMNode && $node->parentNode !== null) {
+			$offset  = 0;
+			$sibling = $node->previousSibling;
+			while ($sibling !== null) {
+				++$offset;
+				$sibling = $sibling->previousSibling;
+			}
+			array_unshift($path, $offset);
+			$node = $node->parentNode;
+		}
+
+		return $path;
+	}
+
+	/**
+	 * Compare two paths produced by documentOrderPath().
+	 *
+	 * @param array $a
+	 * @param array $b
+	 *
+	 * @return int
+	 *   A negative number if $a precedes $b in the document, positive if it
+	 *   follows it, and zero if they are the same node.
+	 */
+	private function compareDocumentOrderPaths(array $a, array $b): int
+	{
+		$shared = min(count($a), count($b));
+		for ($i = 0; $i < $shared; ++$i) {
+			if ($a[$i] !== $b[$i]) {
+				return $a[$i] < $b[$i] ? -1 : 1;
+			}
 		}
 
-		return $this->inst($found, null)->filter($selector);
+		return count($a) - count($b);
 	}
 }