Resolve :scope in find() against the elements it starts from

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 `:scope` in `find()`, `findInPlace()`, `remove($selector)` and the selector argument of `qp($node, $selector)`, which always referred to the document element, so `qp($xml, 'ul')->find(':scope > li')` matched nothing (issue #133). `:scope` is now the element the search starts from, including inside `:not()` and `:matches()`. When the set holds more than one element and the selector uses `:scope`, the search runs from each element in turn, as jQuery's `find()` does, so positional pseudo-classes count per element, and the results are returned in document order. `filter()` and `is()` are unchanged. `QueryPath\CSS\DOMTraverser\PseudoClass::is()` and `isNot()` take a new optional `$scope` argument, so a subclass overriding either must add it
 
 # 4.2.0
 

src/CSS/DOMTraverser/PseudoClass.php

@@ -261,13 +261,13 @@ public function elementMatches($pseudoclass, $node, $scope, $value = null)
 			case 'has':
 				return $this->has($node, $value);
 			case 'matches':
-				return $this->is($node, $value);
+				return $this->is($node, $value, $scope);
 			case 'not':
 				if (empty($value)) {
 					throw new ParseException(':not() requires a value.');
 				}
 
-				return $this->isNot($node, $value);
+				return $this->isNot($node, $value, $scope);
 			// Contains == text matches.
 			// In QP 2.1, this was changed.
 			case 'contains':
@@ -545,22 +545,22 @@ protected function findsAny(DOMElement $root, DOMElement $scope, array $group, b
 	 *
 	 * @throws ParseException
 	 */
-	protected function is($node, $selector): bool
+	protected function is($node, $selector, $scope = null): bool
 	{
 		$splos = new SPLObjectStorage();
 		$splos->offsetSet($node);
 
-		return count(DOMTraverser::filterNodes($splos, $selector, null, $this)) > 0;
+		return count(DOMTraverser::filterNodes($splos, $selector, $scope, $this)) > 0;
 	}
 
 	/**
 	 * Provides :not pseudoclass.
 	 *
 	 * @throws ParseException
 	 */
-	protected function isNot($node, $selector): bool
+	protected function isNot($node, $selector, $scope = null): bool
 	{
-		return ! $this->is($node, $selector);
+		return ! $this->is($node, $selector, $scope);
 	}
 
 	/**

src/CSS/DOMTraverser/Util.php

@@ -58,17 +58,24 @@ class Util
 		'matches',
 	];
 
+	/**
+	 * Cap on each selector cache, so a long-running process cannot grow it forever.
+	 */
+	const POSITIONAL_CACHE_LIMIT = 256;
+
 	/**
 	 * Memoized results for selectorHasPositionalPseudoClass().
 	 *
 	 * @var bool[]
 	 */
+	private static $positionalSelectorCache = [];
+
 	/**
-	 * Cap on the positional-selector cache, so a long-running process cannot grow it forever.
+	 * Memoized results for selectorUsesScope().
+	 *
+	 * @var bool[]
 	 */
-	const POSITIONAL_CACHE_LIMIT = 256;
-
-	private static $positionalSelectorCache = [];
+	private static $scopeSelectorCache = [];
 
 	/**
 	 * Check whether the given pseudo-class name is a jQuery positional filter.
@@ -116,39 +123,85 @@ public static function isSetFilterPseudoClass($name, $value = null): bool
 	 * @return bool
 	 */
 	public static function selectorHasPositionalPseudoClass($selector): bool
+	{
+		return self::anySimpleSelector(self::$positionalSelectorCache, $selector, function ($simpleSelector) {
+			return $simpleSelector->hasSetFilterPseudoClasses();
+		});
+	}
+
+	/**
+	 * Check whether a selector refers to `:scope` (or its `:x-root`/`:x-reset` aliases).
+	 *
+	 * The arguments of :not() and :matches() are searched too. Those of :has() are not: in there,
+	 * `:scope` is the element :has() is tested on.
+	 *
+	 * @param string $selector
+	 *
+	 * @return bool
+	 */
+	public static function selectorUsesScope($selector): bool
+	{
+		return self::anySimpleSelector(self::$scopeSelectorCache, $selector, function ($simpleSelector) {
+			foreach ($simpleSelector->pseudoClasses as $pseudoClass) {
+				$name = strtolower($pseudoClass['name']);
+				if (in_array($name, ['scope', 'x-root', 'x-reset'], true)) {
+					return true;
+				}
+				if (in_array($name, self::SELECTOR_ARGUMENT_PSEUDO_CLASSES, true)
+					&& ! empty($pseudoClass['value'])
+					&& self::selectorUsesScope($pseudoClass['value'])
+				) {
+					return true;
+				}
+			}
+
+			return false;
+		});
+	}
+
+	/**
+	 * Parse a selector and check whether any of its simple selectors passes a test, memoizing the answer.
+	 *
+	 * A selector that fails to parse answers FALSE; the real traversal reports the error.
+	 *
+	 * @param bool[]   $cache
+	 * @param string   $selector
+	 * @param callable $test
+	 *
+	 * @return bool
+	 */
+	private static function anySimpleSelector(array &$cache, $selector, callable $test): bool
 	{
 		$key = (string) $selector;
-		if (isset(self::$positionalSelectorCache[$key])) {
-			return self::$positionalSelectorCache[$key];
+		if (isset($cache[$key])) {
+			return $cache[$key];
 		}
 
 		$found   = false;
 		$handler = new Selector();
 		try {
-			$parser = new Parser($key, $handler);
-			$parser->parse();
+			(new Parser($key, $handler))->parse();
 
 			foreach ($handler as $selectorGroup) {
 				foreach ($selectorGroup as $simpleSelector) {
-					if ($simpleSelector->hasSetFilterPseudoClasses()) {
+					if ($test($simpleSelector)) {
 						$found = true;
 						break 2;
 					}
 				}
 			}
 		} catch (ParseException $e) {
-			// Let the real traversal report the parse error.
 			$found = false;
 		}
 
 		// The cache is keyed by selector string, which in a long-running process can be
 		// unbounded user input. Selectors repeat heavily within one query but rarely across
 		// unrelated ones, so the table is dropped wholesale rather than evicted entry by entry.
-		if (count(self::$positionalSelectorCache) >= self::POSITIONAL_CACHE_LIMIT) {
-			self::$positionalSelectorCache = [];
+		if (count($cache) >= self::POSITIONAL_CACHE_LIMIT) {
+			$cache = [];
 		}
 
-		self::$positionalSelectorCache[$key] = $found;
+		$cache[$key] = $found;
 
 		return $found;
 	}

src/DOM.php

@@ -10,6 +10,8 @@
 use IteratorAggregate;
 use Masterminds\HTML5;
 use QueryPath\CSS\DOMTraverser;
+use QueryPath\CSS\DOMTraverser\PseudoClass;
+use QueryPath\CSS\DOMTraverser\Util;
 use QueryPath\Entities;
 use SimpleXMLElement;
 use SplObjectStorage;
@@ -168,9 +170,7 @@ public function __construct($document = null, $selector = null, $options = [])
 			// We don't issue a find because that creates a new DOMQuery.
 			//$this->find($string);
 
-			$query = new DOMTraverser($this->matches);
-			$query->find($selector);
-			$this->setMatches($query->matches());
+			$this->setMatches($this->findMatches($selector));
 		}
 	}
 
@@ -371,6 +371,55 @@ public function setMatches($matches)
 		$this->length = $this->matches->count();
 	}
 
+	/**
+	 * Search the descendants of the matched nodes for a selector.
+	 *
+	 * `:scope` is the node the search starts from. When there is more than one and the selector uses
+	 * `:scope`, it is run from each node in turn, as jQuery's find() is, and the results are merged in
+	 * document order.
+	 *
+	 * @param string $selector
+	 *
+	 * @return SplObjectStorage
+	 * @throws CSS\ParseException
+	 */
+	protected function findMatches($selector): SplObjectStorage
+	{
+		if ($this->matches->count() === 1) {
+			$this->matches->rewind();
+			$node = $this->matches->current();
+
+			return (new DOMTraverser($this->matches, false, $node instanceof DOMElement ? $node : null))
+				->find($selector)
+				->matches();
+		}
+
+		if (! Util::selectorUsesScope($selector)) {
+			return (new DOMTraverser($this->matches))->find($selector)->matches();
+		}
+
+		$psHandler = new PseudoClass();
+		$psHandler->beginQuery();
+		try {
+			$found = new SplObjectStorage();
+			foreach ($this->matches as $node) {
+				$start = new SplObjectStorage();
+				$start->offsetSet($node);
+				$scope = $node instanceof DOMElement ? $node : null;
+				$found->addAll((new DOMTraverser($start, false, $scope, $psHandler))->find($selector)->matches());
+			}
+		} finally {
+			$psHandler->endQuery();
+		}
+
+		$sorted = new SplObjectStorage();
+		foreach (Util::sortDocumentOrder(iterator_to_array($found, false)) as $node) {
+			$sorted->offsetSet($node);
+		}
+
+		return $sorted;
+	}
+
 	/**
 	 * A depth-checking function. Typically, it only needs to be
 	 * invoked with the first parameter. The rest are used for recursion.

src/DOMQuery.php

@@ -16,7 +16,6 @@
 use DOMDocument;
 use DOMNode;
 use DOMXPath;
-use QueryPath\CSS\DOMTraverser;
 use Masterminds\HTML5;
 use QueryPath\Entities;
 use QueryPath\Exception;
@@ -140,10 +139,7 @@ public function top($selector = null): Query
 	 */
 	public function find($selector): Query
 	{
-		$query = new DOMTraverser($this->matches);
-		$query->find($selector);
-
-		return $this->inst($query->matches(), null);
+		return $this->inst($this->findMatches($selector), null);
 	}
 
 	/**
@@ -154,9 +150,7 @@ public function find($selector): Query
 	 */
 	public function findInPlace($selector)
 	{
-		$query = new DOMTraverser($this->matches);
-		$query->find($selector);
-		$this->setMatches($query->matches());
+		$this->setMatches($this->findMatches($selector));
 
 		return $this;
 	}

src/Helpers/QueryMutators.php

@@ -796,7 +796,7 @@ public function appendTo(DOMQuery $dest): Query
 	public function remove($selector = null): Query
 	{
 		// Not find(), which deep-clones the match set via inst().
-		$matches = empty($selector) ? $this->matches : (new DOMTraverser($this->matches))->find($selector)->matches();
+		$matches = empty($selector) ? $this->matches : $this->findMatches($selector);
 
 		$found = new SplObjectStorage();
 		foreach ($matches as $item) {

tests/Issues/Issue133Test.php

@@ -0,0 +1,98 @@
+<?php
+/**
+ * @file
+ *
+ * Regression tests for https://github.com/GravityPDF/querypath/issues/133
+ *
+ * `:scope` in find() is the element the search starts from, as in jQuery's
+ * `$(el).find(':scope > li')`, and each element in turn when the set holds several.
+ */
+
+namespace QueryPathTests;
+
+use QueryPath\CSS\DOMTraverser\Util;
+
+class Issue133Test extends TestCase
+{
+	public const XML = '<?xml version="1.0"?><root><ul id="u1"><li id="a"/><li id="b"><ul id="u3"><li id="d"/></ul></li></ul><ul id="u2"><li id="c"/></ul></root>';
+
+	public function testScopeIsTheElementFindStartsFrom()
+	{
+		$this->assertSame('a,b', $this->idList(qp(self::XML, '#u1')->find(':scope > li')));
+		$this->assertSame('d', $this->idList(qp(self::XML, '#u1')->find('li:not(:scope > li)')));
+		$this->assertSame('a,b', $this->idList(qp(self::XML, '#u1')->find('li:matches(:scope > li)')));
+		$this->assertSame('', $this->idList(qp(self::XML, '#u1')->find(':scope')));
+	}
+
+	public function testScopeIsEachElementOfTheSet()
+	{
+		$this->assertSame('a,b,c', $this->idList(qp(self::XML, '#u1, #u2')->find(':scope > li')));
+		$this->assertSame('a,b,d,c', $this->idList(qp(self::XML, 'ul')->find(':scope > li')));
+		$this->assertSame('', $this->idList(qp(self::XML, '#u1, #u2')->find(':scope + ul li')));
+		$this->assertSame('u3', $this->idList(qp(self::XML, '#u1, #u2')->find(':scope > li > ul')));
+	}
+
+	public function testPositionalPseudoClassesCountPerElementWhenScopeIsUsed()
+	{
+		$this->assertSame('a,c', $this->idList(qp(self::XML, '#u1, #u2')->find(':scope > li:first')));
+		$this->assertSame('a', $this->idList(qp(self::XML, '#u1, #u2')->find('li:first')));
+	}
+
+	public function testFindInPlaceAndTheConstructor()
+	{
+		$this->assertSame('a,b', $this->idList(qp(self::XML, '#u1')->findInPlace(':scope > li')));
+
+		$u1 = qp(self::XML, '#u1')->get(0);
+		$this->assertSame('a,b', $this->idList(qp($u1, ':scope > li')));
+	}
+
+	public function testRemoveWithASelector()
+	{
+		$qp = qp(self::XML, '#u1');
+		$this->assertSame('a,b', $this->idList($qp->remove(':scope > li')));
+		$this->assertSame('', $this->idList($qp->find('li')));
+	}
+
+	public function testScopeIsStillTheDocumentElementFromTheTop()
+	{
+		$this->assertSame('u1,u2', $this->idList(qp(self::XML)->find(':scope > ul')));
+		$this->assertSame('root', qp(self::XML)->find(':scope')->tag());
+		$this->assertSame('root', qp(self::XML, 'li')->top()->find(':scope')->tag());
+	}
+
+	public function testHasKeepsItsOwnScope()
+	{
+		$this->assertSame('b', $this->idList(qp(self::XML, '#u1')->find('li:has(> ul)')));
+		$this->assertSame('u3', $this->idList(qp(self::XML, '#u1')->find('ul:has(> li)')));
+	}
+
+	public function testFilterAndIsAreUnchanged()
+	{
+		$this->assertSame('', $this->idList(qp(self::XML, 'ul')->filter(':scope')));
+		$this->assertSame('', $this->idList(qp(self::XML, 'root')->children(':scope')));
+		$this->assertTrue(qp(self::XML, 'root')->is(':scope'));
+	}
+
+	/**
+	 * @dataProvider scopeSelectors
+	 */
+	public function testSelectorUsesScope($selector, $expected)
+	{
+		$this->assertSame($expected, Util::selectorUsesScope($selector));
+	}
+
+	public function scopeSelectors(): array
+	{
+		return [
+			'plain'         => ['ul > li', false],
+			':scope'        => [':scope > li', true],
+			':x-root'       => [':x-root > li', true],
+			'second group'  => ['ol, :scope li', true],
+			'inside :not()' => ['li:not(:scope > li)', true],
+			'nested'        => ['li:matches(:not(:scope > li))', true],
+			'inside :has()' => ['li:has(:scope > ul)', false],
+			'attribute'     => ['[name=scope]', false],
+			'parse error'   => [':scope[', false],
+		];
+	}
+}

tests/QueryPath/DOMQueryTest.php

@@ -849,7 +849,7 @@ public function testFilterEvaluatesTheSelectorAgainstTheWholeSet()
 		$this->assertEquals('two', qp($file, 'li')->filter(':eq(1)')->attr('id'));
 		$this->assertCount(2, qp($file, 'li')->filter(':odd'));
 
-		// :scope refers to the document element, exactly as it does in find().
+		// :scope refers to the document element, as it does in is().
 		$this->assertEquals(0, qp($file, 'inner')->filter(':scope')->count());
 		$this->assertEquals(1, qp($file, 'root')->filter(':scope')->count());
 	}