Count a positional compound left of a combinator across the document in filter()

CHANGELOG.md

@@ -58,6 +58,7 @@ QueryPath Changelog
 - 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
 - **Behaviour change.** A positional pseudo-class (`:first`, `:eq()`, `:last`, ...) in the stop selector of `nextUntil()`, `prevUntil()` and `parentsUntil()` now counts across the whole document, as jQuery's does, instead of seeing each sibling or ancestor as a one-element set (issue #118). `nextUntil(':first')` used to stop at once and now walks to the end, and `nextUntil(':eq(3)')` stops at the fourth element in the document. `closest()` no longer matches a positional selector at all, as in jQuery, where `closest(':first')` used to return the element itself. `next()`, `prev()` and `parent()` still test each candidate on its own, which agrees with jQuery for a single starting element
+- **Behaviour change.** Fix `filter()`, `not()` and `is()` matching nothing when a positional pseudo-class sits on a compound left of a combinator, as in `ul:first > li` (issue #132). That compound now counts across the whole document, as in jQuery's `.filter()` and in `find()`, so `qp($xml, 'li')->filter('ul:first > li')` returns the items of the document's first `<ul>`. A positional pseudo-class on the subject still counts within the set. `QueryPath\CSS\DOMTraverser::filterNodes()` follows suit
 
 # 4.2.0
 

src/CSS/DOMTraverser.php

@@ -173,8 +173,9 @@ public function find($selector): DOMTraverser
 	 *
 	 * Unlike find(), which searches the descendants of the nodes it starts from, only the given
 	 * nodes can match: for `<a><b/></a>`, `b` matches the `b` element and not `a`. Combinators are
-	 * still resolved against the document, so `div > p` works, and positional pseudo-classes
-	 * (`:first`, `:eq(n)`, ...) count within the given set. Nodes that are not elements never match.
+	 * still resolved against the document, so `div > p` works. As in jQuery's .filter(), a positional
+	 * pseudo-class (`:first`, `:eq(n)`, ...) counts within the given set when it is on the subject, and
+	 * across the document when it is left of a combinator. Nodes that are not elements never match.
 	 *
 	 * @param SplObjectStorage $nodes
 	 *   The candidates, all from one document.
@@ -335,9 +336,13 @@ protected function resolveSubject(array $selectorGroup, int $index): SplObjectSt
 			});
 		} else {
 			// initialMatch() consumes the parts of the simple selector it matched on. Give it a copy,
-			// so the caller's parsed selector can be reused.
+			// so the caller's parsed selector can be reused. In filterNodes(), a compound left of a
+			// combinator is looked up across the document, so its positional filters count there.
 			$selectorGroup[$index] = clone $selectorGroup[$index];
-			$candidates            = $this->initialMatch($selectorGroup[$index], $this->matches);
+			$candidates            = $this->initialMatch(
+				$selectorGroup[$index],
+				$this->initialized ? self::documentScope($this->dom) : $this->matches
+			);
 		}
 
 		$setFilters           = $selectorGroup[$index]->setFilterPseudoClasses();
@@ -377,6 +382,23 @@ protected function resolveSubject(array $selectorGroup, int $index): SplObjectSt
 		return $this->toMatches($matched);
 	}
 
+	/**
+	 * The document element, standing in for the document as it does in qp($document).
+	 *
+	 * @param DOMDocument|null $document
+	 *
+	 * @return SplObjectStorage
+	 */
+	public static function documentScope($document): SplObjectStorage
+	{
+		$scope = new SplObjectStorage();
+		if ($document instanceof DOMDocument && $document->documentElement) {
+			$scope->offsetSet($document->documentElement);
+		}
+
+		return $scope;
+	}
+
 	/**
 	 * Apply a single set-level pseudo-class to an ordered list of nodes.
 	 *

src/Helpers/NodeMatcher.php

@@ -115,13 +115,9 @@ public function matchesInDocument($node): bool
 	 */
 	private function documentMatches($document): SplObjectStorage
 	{
-		// As in qp($document), the document element stands in for the document, so it can match.
-		$scope = new SplObjectStorage();
-		if ($document->documentElement) {
-			$scope->offsetSet($document->documentElement);
-		}
-
-		return (new DOMTraverser($scope, false, null, $this->psHandler))->find($this->selector)->matches();
+		return (new DOMTraverser(DOMTraverser::documentScope($document), false, null, $this->psHandler))
+			->find($this->selector)
+			->matches();
 	}
 
 	/**

tests/Issues/Issue132Test.php

@@ -0,0 +1,59 @@
+<?php
+/**
+ * @file
+ *
+ * Regression tests for https://github.com/GravityPDF/querypath/issues/132
+ *
+ * A positional pseudo-class on a compound left of a combinator, tested against
+ * the nodes in hand. As in jQuery's .filter(), that compound counts across the
+ * document.
+ */
+
+namespace QueryPathTests;
+
+use QueryPath\CSS\DOMTraverser;
+use SplObjectStorage;
+
+class Issue132Test extends TestCase
+{
+	private const XML = '<?xml version="1.0"?><root><ul id="u1"><li id="a"/><li id="b"/></ul><ul id="u2"><li id="c"/></ul></root>';
+
+	/**
+	 * @testWith ["ul:first > li", "a,b"]
+	 *           ["ul:eq(1) > li", "c"]
+	 *           ["ul:last li", "c"]
+	 *           ["ul:eq(0) + ul > li", "c"]
+	 *           ["ul:gt(0) > li:first", "c"]
+	 *           ["ul:first > li:last", "b"]
+	 *           ["ul:first > li, ul:last > li", "a,b,c"]
+	 */
+	public function testFilterCountsALeftHandPositionalCompoundAcrossTheDocument($selector, $expected)
+	{
+		$this->assertSame($expected, $this->idList(qp(self::XML, 'li')->filter($selector)));
+	}
+
+	public function testIsMatchesWhatFindSelects()
+	{
+		$this->assertSame('a,b', $this->idList(qp(self::XML)->find('ul:first > li')));
+		$this->assertTrue(qp(self::XML, 'li')->is('ul:first > li'));
+	}
+
+	public function testTheCompoundIsNotLimitedToTheNodesInHand()
+	{
+		// u1 is not in the set, but is still the document's first <ul>.
+		$this->assertSame('b', $this->idList(qp(self::XML, '#b')->filter('ul:first > li')));
+		$this->assertSame('', $this->idList(qp(self::XML, '#c')->filter('ul:first > li')));
+		$this->assertSame('c', $this->idList(qp(self::XML, 'li')->not('ul:first > li')));
+	}
+
+	public function testFilterNodes()
+	{
+		$nodes = new SplObjectStorage();
+		foreach (qp(self::XML, 'li')->get() as $node) {
+			$nodes->offsetSet($node);
+		}
+
+		$this->assertCount(2, DOMTraverser::filterNodes($nodes, 'ul:first > li'));
+		$this->assertCount(1, DOMTraverser::filterNodes($nodes, 'ul:eq(1) > li'));
+	}
+}