Return traversal results in jQuery's order

CHANGELOG.md

@@ -56,6 +56,7 @@ QueryPath Changelog
 - **Behaviour change.** `remove($selector)` and `replaceAll($selector)` now select with `QueryPath\CSS\DOMTraverser`, the engine behind `find()`, instead of the legacy `QueryPath\CSS\QueryPathEventHandler` (issues #126, #123, #124). `remove($selector)` searches the descendants of the match set, as `find()` and `detach($selector)` do, so `qp($xml, 'p')->remove('p')` no longer removes the `<p>` elements themselves. `replaceAll($selector, $document)` still searches the whole of `$document`, document element included. Both now accept `:matches()`, which threw "Unknown Pseudo-Class: matches", and apply a positional `:not()` argument to the whole set, so `remove('li:not(:first)')` keeps only the first `<li>` instead of removing nothing. `QueryPathEventHandler` is no longer used and is deprecated
 - Add `QueryPath\CSS\DOMTraverser::filterNodes($nodes, $selector)`, which returns the given nodes that themselves match a selector, in the order given (issue #116). `filter()`, `is()`, the selector-filtered traversal methods, and `:not()`/`:matches()` all use it. `DOMTraverser` no longer modifies a parsed selector, so queries sharing a `PseudoClass` handler parse each selector once: `nextUntil()` over 2000 siblings drops from about 16 ms to 6 ms, and `tr:not(:nth-child(2n))` on a 2000-row table from 15 ms to 6 ms
 - Consolidate the sibling and ancestor walks behind `next()`, `prev()`, `nextAll()`, `prevAll()`, `nextUntil()`, `prevUntil()`, `parent()`, `parents()`, `parentsUntil()` and `closest()` into one helper, so a fix to one no longer has to be repeated in its mirror (issue #115). As a side effect, `parentsUntil()` no longer loops forever on an element that is not attached to the document, and `nextUntil('')` and `prevUntil('')` treat an empty selector as no selector, as the other methods already did, instead of throwing `QueryPath\CSS\ParseException`. `closest('')` returns an empty set, as in jQuery. `nextUntil()`, `prevUntil()` and `parentsUntil()` also stop at an element already collected from an earlier starting element, as `nextAll()` and `prevAll()` do, so `nextUntil()` on each of 2000 siblings falls from about 100 ms to 1 ms
+- **Behaviour change.** `prevAll()`, `prevUntil()`, `nextAll()`, `nextUntil()`, `siblings()`, `parent()` and `closest()` now return the results for a multi-element set in jQuery's order, instead of grouped by starting element (issue #117). `prevAll()` and `prevUntil()` return reverse document order, as `parents()` and `parentsUntil()` already did, and the others return document order. `qp($xml, 'c')->add('d')->prevAll()` now returns `c, b, a` rather than `b, a, c`. `next()` and `prev()` keep the order of the starting elements, as in jQuery, and the results for a single starting element are unchanged
 
 # 4.2.0
 

src/Helpers/QueryFilters.php

@@ -557,7 +557,7 @@ public function lastChild(): Query
 	 */
 	public function nextUntil($selector = null): Query
 	{
-		return $this->inst($this->walk('nextSibling', $selector, false), null);
+		return $this->inDocumentOrder($this->walk('nextSibling', $selector, false));
 	}
 
 	/**
@@ -583,7 +583,7 @@ public function nextUntil($selector = null): Query
 	 */
 	public function prevUntil($selector = null): Query
 	{
-		return $this->inst($this->walk('previousSibling', $selector, false), null);
+		return $this->inReverseDocumentOrder($this->walk('previousSibling', $selector, false));
 	}
 
 	/**
@@ -605,7 +605,7 @@ public function prevUntil($selector = null): Query
 	 */
 	public function parentsUntil($selector = null): Query
 	{
-		return $this->inst($this->sortReverseDocumentOrder($this->walk('parentNode', $selector, false)), null);
+		return $this->inReverseDocumentOrder($this->walk('parentNode', $selector, false));
 	}
 
 	/**
@@ -694,7 +694,7 @@ public function closest($selector): Query
 			return $this->inst(new SplObjectStorage(), null);
 		}
 
-		return $this->inst($this->walk('parentNode', $selector, true, true), null);
+		return $this->inDocumentOrder($this->walk('parentNode', $selector, true, true));
 	}
 
 	/**
@@ -715,7 +715,7 @@ public function closest($selector): Query
 	 */
 	public function parent($selector = null): Query
 	{
-		return $this->inst($this->walk('parentNode', $selector, true), null);
+		return $this->inDocumentOrder($this->walk('parentNode', $selector, true));
 	}
 
 	/**
@@ -736,11 +736,7 @@ public function parent($selector = null): Query
 	 */
 	public function parents($selector = null): Query
 	{
-		// jQuery returns the ancestors of a multi-element set in reverse document order, with
-		// duplicates removed. The selector is applied after that ordering.
-		$ancestors = $this->sortReverseDocumentOrder($this->walk('parentNode', null, false));
-
-		return $this->inst($this->filterCandidates($ancestors, $selector), null);
+		return $this->inReverseDocumentOrder($this->walk('parentNode', null, false), $selector);
 	}
 
 	/**
@@ -788,7 +784,7 @@ public function next($selector = null): Query
 	 */
 	public function nextAll($selector = null): Query
 	{
-		return $this->inst($this->filterCandidates($this->walk('nextSibling', null, false), $selector), null);
+		return $this->inDocumentOrder($this->walk('nextSibling', null, false), $selector);
 	}
 
 	/**
@@ -837,7 +833,7 @@ public function prev($selector = null): Query
 	 */
 	public function prevAll($selector = null): Query
 	{
-		return $this->inst($this->filterCandidates($this->walk('previousSibling', null, false), $selector), null);
+		return $this->inReverseDocumentOrder($this->walk('previousSibling', null, false), $selector);
 	}
 
 	/**
@@ -943,7 +939,7 @@ public function siblings($selector = null): Query
 			}
 		}
 
-		return $this->inst($this->filterCandidates($found, $selector), null);
+		return $this->inDocumentOrder($found, $selector);
 	}
 
 	/**
@@ -994,26 +990,54 @@ private function walk(string $axis, $selector, bool $first, bool $inclusive = fa
 	}
 
 	/**
-	 * Sort a set of nodes into reverse document order.
+	 * Wrap a traversal's results, optionally filtered by a selector, in document order.
+	 *
+	 * The results of a traversal are accumulated per starting element, so a set built from
+	 * more than one would otherwise be grouped by starting element, as jQuery's never are.
 	 *
-	 * jQuery's ancestor traversal methods 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.
+	 * @param SplObjectStorage $nodes
+	 * @param string|null      $selector
+	 *
+	 * @return Query
+	 * @throws ParseException
+	 */
+	private function inDocumentOrder(SplObjectStorage $nodes, $selector = null): Query
+	{
+		return $this->inst($this->sortDocumentOrder($this->filterCandidates($nodes, $selector), false), null);
+	}
+
+	/**
+	 * As inDocumentOrder(), in reverse document order, as jQuery returns the walks that head
+	 * backwards: parents(), parentsUntil(), prevAll() and prevUntil().
 	 *
 	 * @param SplObjectStorage $nodes
-	 *   The nodes to sort. Duplicates are already removed by SplObjectStorage.
+	 * @param string|null      $selector
+	 *
+	 * @return Query
+	 * @throws ParseException
+	 */
+	private function inReverseDocumentOrder(SplObjectStorage $nodes, $selector = null): Query
+	{
+		return $this->inst($this->sortDocumentOrder($this->filterCandidates($nodes, $selector), true), null);
+	}
+
+	/**
+	 * @param SplObjectStorage $nodes
+	 * @param bool             $reverse
 	 *
 	 * @return SplObjectStorage
-	 *   The same nodes, in reverse document order.
 	 */
-	private function sortReverseDocumentOrder(SplObjectStorage $nodes): SplObjectStorage
+	private function sortDocumentOrder(SplObjectStorage $nodes, bool $reverse): SplObjectStorage
 	{
-		// A single starting element's walk up the tree already yields reverse document order.
+		// A single starting element's walk is already in order.
 		if (count($nodes) < 2 || count($this->matches) < 2) {
 			return $nodes;
 		}
 
-		$ordered = array_reverse(Util::sortDocumentOrder(iterator_to_array($nodes, false)));
+		$ordered = Util::sortDocumentOrder(iterator_to_array($nodes, false));
+		if ($reverse) {
+			$ordered = array_reverse($ordered);
+		}
 
 		$sorted = new SplObjectStorage();
 		foreach ($ordered as $node) {

tests/Issues/Issue117Test.php

@@ -0,0 +1,54 @@
+<?php
+/**
+ * @file
+ *
+ * Regression tests for https://github.com/GravityPDF/querypath/issues/117
+ *
+ * The traversal methods returned the results of a multi-element set grouped by
+ * starting element. jQuery returns `parents()`, `parentsUntil()`, `prevAll()` and
+ * `prevUntil()` in reverse document order, `next()` and `prev()` in the order of
+ * the starting elements, and the rest in document order. The expected values
+ * below are what jQuery 3.7.1 returns for the same calls.
+ */
+
+namespace QueryPathTests;
+
+class Issue117Test extends TestCase
+{
+	public const SIBLINGS = '<?xml version="1.0"?><r><i id="a"/><i id="b"/><i id="c"/><i id="d"/><i id="e"/></r>';
+
+	public const ANCESTORS = '<?xml version="1.0"?><r id="r"><s id="s1"><t id="t1"><u id="u1"/></t></s><s id="s2"><t id="t2"><u id="u2"/></t></s></r>';
+
+	public function testBackwardWalksReturnReverseDocumentOrder()
+	{
+		$this->assertSame('c,b,a', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'd')->prevAll()));
+		$this->assertSame('d,c,b', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'e')->prevUntil('#a')));
+		$this->assertSame('t2,s2,t1,s1,r', $this->idList($this->selectIds(self::ANCESTORS, 'u1', 'u2')->parents()));
+		$this->assertSame('t2,s2,t1,s1', $this->idList($this->selectIds(self::ANCESTORS, 'u1', 'u2')->parentsUntil('#r')));
+	}
+
+	public function testForwardWalksReturnDocumentOrder()
+	{
+		$this->assertSame('b,c,d,e', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'a')->nextAll()));
+		$this->assertSame('b,c,d', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'a')->nextUntil('#e')));
+		$this->assertSame('a,b,c,d,e', $this->idList($this->selectIds(self::SIBLINGS, 'd', 'b')->siblings()));
+	}
+
+	public function testParentAndClosestReturnDocumentOrder()
+	{
+		$this->assertSame('t1,t2', $this->idList($this->selectIds(self::ANCESTORS, 'u2', 'u1')->parent()));
+		$this->assertSame('s1,s2', $this->idList($this->selectIds(self::ANCESTORS, 'u2', 'u1')->closest('s')));
+	}
+
+	public function testNextAndPrevKeepTheOrderOfTheStartingElements()
+	{
+		$this->assertSame('d,b', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'a')->next()));
+		$this->assertSame('d,b', $this->idList($this->selectIds(self::SIBLINGS, 'e', 'c')->prev()));
+	}
+
+	public function testSingleStartingElementKeepsItsWalkOrder()
+	{
+		$this->assertSame('d,c,b,a', $this->idList($this->selectIds(self::SIBLINGS, 'e')->prevAll()));
+		$this->assertSame('t1,s1,r', $this->idList($this->selectIds(self::ANCESTORS, 'u1')->parents()));
+	}
+}

tests/QueryPath/DOMQueryTest.php

@@ -2139,8 +2139,8 @@ public function testNextAllAndPrevAllFromOverlappingStarts()
 		};
 
 		$this->assertSame('c,d,e', $names(qp($xml, 'b')->add('d')->nextAll()));
-		$this->assertSame('e,c,d', $names(qp($xml, 'd')->add('b')->nextAll()));
-		$this->assertSame('a,c,b', $names(qp($xml, 'b')->add('d')->prevAll()));
+		$this->assertSame('c,d,e', $names(qp($xml, 'd')->add('b')->nextAll()));
+		$this->assertSame('c,b,a', $names(qp($xml, 'b')->add('d')->prevAll()));
 		$this->assertSame('c,b,a', $names(qp($xml, 'd')->add('b')->prevAll()));
 	}
 

tests/QueryPath/TestCase.php

@@ -34,6 +34,38 @@ protected function capture(callable $callback): string
 		}
 	}
 
+	/**
+	 * Build a match set from element IDs, in the order given rather than document order.
+	 *
+	 * @param string $xml
+	 * @param string ...$ids
+	 *
+	 * @return \QueryPath\DOMQuery
+	 */
+	protected function selectIds(string $xml, string ...$ids)
+	{
+		$qp = qp($xml, '#' . array_shift($ids));
+		foreach ($ids as $id) {
+			$qp = $qp->add('#' . $id);
+		}
+
+		return $qp;
+	}
+
+	/**
+	 * The IDs of a match set's elements, comma-separated, in match-set order.
+	 *
+	 * @param \QueryPath\DOMQuery $query
+	 *
+	 * @return string
+	 */
+	protected function idList($query): string
+	{
+		return implode(',', array_map(function ($node) {
+			return $node->getAttribute('id');
+		}, $query->get()));
+	}
+
 	/**
 	 * Whether libxml's HTML parser follows HTML5, as it does from 2.14.
 	 *