Fix open issues #132–#146

CHANGELOG.md

@@ -58,6 +58,19 @@ 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
+- Fix `siblings()` raising an "Attempt to read property "childNodes" on null" warning and a `foreach()` warning when the match set contains an element with no parent, such as one made with `createElement()` and not yet inserted (issue #139). Such an element has no siblings, so it now adds nothing to the result, as in jQuery
+- **Behaviour change.** `eq()`, `get()` and `slice()` now count a negative index back from the end of the match set, as jQuery does (issue #142). `eq(-1)` and `get(-1)` are the last match, where they used to give an empty set and `NULL`. `slice(-2)` now returns the last two matches instead of the whole set. An index that is still out of range gives an empty set, or `NULL` from `get()`
+- **Breaking behaviour change.** `slice($start, $end)` now takes an exclusive end index as its second argument, as jQuery's `.slice()` does, instead of a length (issue #144). `slice(1, 3)` returns the second and third matches, where it used to return three matches. A negative `$end` counts back from the end, so `slice(1, -1)` drops the first and last matches. Leaving out `$end` still slices to the end of the set, but an explicit `0`, which used to mean "to the end", now gives an empty set. To keep a length, use `slice($start, $start + $length)`
+- **Breaking behaviour change.** `even()` and `odd()` now count from zero, as jQuery's `.even()` and `.odd()` and QueryPath's own `:even` and `:odd` pseudo-classes do (issue #145). `even()` keeps the 1st, 3rd, 5th, ... matches and `odd()` keeps the 2nd, 4th, ..., the reverse of before. Swap the calls to keep the previous result
+- **Behaviour change.** `even()`, `odd()`, `first()` and `last()` now count and return every node in the match set, including text, comment, CDATA and processing-instruction nodes, as jQuery does (issues #143, #146). `even()` and `odd()` used to count those nodes but leave them out of the result, so `contents()->odd()` could be empty, and `first()` and `last()` skipped them, so `first()` could disagree with `eq(0)`. They are now the same as `eq(0)` and `eq(-1)`. To work on elements only, use `children()->first()` or `filter('*')->first()`, and likewise for `last()`, `even()` and `odd()`
+- **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
+- **Behaviour change.** `is()` now counts a positional pseudo-class (`:first`, `:last`, `:eq()`, `:lt()`, `:gt()`, `:odd`, `:even`) across the whole document, as jQuery's `.is()` does, instead of within the match set (issue #140). It returns `true` if any element of the set is among the document's matches, so `qp($xml, '#c')->is(':first')` is now `false`. `filter()` still counts within the set. `is()` with a `DOMNode` or `SplObjectStorage` argument, or with a selector that has no positional pseudo-class, is unchanged
+- **Behaviour change.** A positional pseudo-class in the selector of `prevAll()`, `parents()` and `nextAll()` now counts in walk order, before the results are sorted, as jQuery does (issue #134). `qp($xml, '#e')->prevAll(':first')` returns the nearest previous sibling, `d`, instead of the first one, `a`, and `parents(':first')` returns the parent. For a set of more than one element, the walks from each element are joined in match-set order, repeats included, and counted along that list. Add `QueryPath\CSS\DOMTraverser::filterList()`, which filters a list of nodes counting positional pseudo-classes in the order given
+- **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
+- Add `:is()` and `:where()` as aliases of `:matches()` (issue #136). QueryPath doesn't use specificity, so `:where()` matches exactly as `:is()` does, and a positional argument (`:is(:first)`) is applied to the whole set as it is for `:matches()`
+- Deprecate `QueryPath\CSS\SimpleSelector::notEmpty()`, which will be removed in 5.0; use `! isBlank()` instead. It could never return `true`, and is now the inverse of `isBlank()` (issue #138)
+- Fix `replaceAll()` inserting empty copies of the replacement. It copied the replacement node without its children, so only the element and its attributes arrived. It now copies the whole subtree (issue #135)
+- Stop every method that returns a new object (`find()`, `children()`, `filter()`, `eq()`, `next()`, ...) from deep-copying the current matches first (issue #137). A `find()` from the root no longer copies the whole document: on a 50,000-paragraph document, `children()` from the root drops from about 27 ms to 12 ms and `find('#nope')` from about 54 ms to 39 ms. As a result, `end()` now returns the original document nodes instead of detached copies of them. `clone $qp` still deep-copies the matched nodes
 
 # 4.2.0
 

src/CSS/DOMTraverser.php

@@ -98,6 +98,13 @@ class DOMTraverser implements Traverser
 	 */
 	protected $resolvingIndex;
 
+	/**
+	 * The candidates in the order filterList() counts them, or NULL to count in document order.
+	 *
+	 * @var DOMNode[]|null
+	 */
+	protected $candidateList;
+
 	/**
 	 * Build a new DOMTraverser.
 	 *
@@ -173,8 +180,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.
@@ -220,6 +228,50 @@ public static function filterNodes(
 		return $found;
 	}
 
+	/**
+	 * As filterNodes(), but positional pseudo-classes on the subject count along the list in the
+	 * order given, repeats included, as jQuery.filter() counts a traversal's walk.
+	 *
+	 * @param DOMNode[]        $nodes
+	 *   The candidates, all from one document, in the order to count them.
+	 * @param string           $selector
+	 *   A CSS selector.
+	 * @param DOMNode|null     $scopeNode
+	 * @param PseudoClass|null $psHandler
+	 *
+	 * @return SplObjectStorage
+	 *   The matching nodes, once each, in the order they first appear in the list.
+	 * @throws ParseException
+	 */
+	public static function filterList(
+		array $nodes,
+		$selector,
+		$scopeNode = null,
+		?PseudoClass $psHandler = null
+	): SplObjectStorage {
+		$unique = new SplObjectStorage();
+		foreach ($nodes as $node) {
+			$unique->offsetSet($node);
+		}
+
+		if (count($unique) === 0) {
+			return $unique;
+		}
+
+		$traverser                = new self($unique, true, $scopeNode, $psHandler);
+		$traverser->candidateList = array_values($nodes);
+		$matched                  = $traverser->find($selector)->matches();
+
+		$found = new SplObjectStorage();
+		foreach ($unique as $node) {
+			if ($matched->offsetExists($node)) {
+				$found->offsetSet($node);
+			}
+		}
+
+		return $found;
+	}
+
 	/**
 	 * Find the matches of an already-parsed selector.
 	 *
@@ -330,14 +382,18 @@ protected function resolveSubject(array $selectorGroup, int $index): SplObjectSt
 		// Initialize matches if necessary. A caller-supplied set may hold non-element nodes,
 		// which can never match a selector.
 		if ($index === 0 && $this->initialized) {
-			$candidates = array_filter(iterator_to_array($this->matches, false), function ($node) {
+			$candidates = array_filter($this->candidateList ?? iterator_to_array($this->matches, false), function ($node) {
 				return $node instanceof DOMElement;
 			});
 		} 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();
@@ -369,14 +425,33 @@ protected function resolveSubject(array $selectorGroup, int $index): SplObjectSt
 
 		$this->resolvingIndex = $previous;
 
-		$matched = Util::sortDocumentOrder($matched);
+		if ($index !== 0 || $this->candidateList === null) {
+			$matched = Util::sortDocumentOrder($matched);
+		}
 		foreach ($setFilters as $pseudoClass) {
 			$matched = $this->applySetFilter($matched, $pseudoClass);
 		}
 
 		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.
 	 *
@@ -398,7 +473,7 @@ protected function applySetFilter(array $nodes, array $pseudoClass): array
 		// :not() and :matches() whose argument uses a positional filter.
 		// The argument has to see the whole result set, exactly as jQuery's
 		// :not(:first) does, so run it over this set and keep or drop the result.
-		$inner = self::filterNodes($this->toMatches($nodes), $value, $this->scopeNode, $this->psHandler);
+		$inner = self::filterList($nodes, $value, $this->scopeNode, $this->psHandler);
 		if ($name !== 'not') {
 			return iterator_to_array($inner, false);
 		}

src/CSS/DOMTraverser/PseudoClass.php

@@ -261,13 +261,15 @@ public function elementMatches($pseudoclass, $node, $scope, $value = null)
 			case 'has':
 				return $this->has($node, $value);
 			case 'matches':
-				return $this->is($node, $value);
+			case 'is':
+			case 'where':
+				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':
@@ -541,26 +543,26 @@ protected function findsAny(DOMElement $root, DOMElement $scope, array $group, b
 	}
 
 	/**
-	 * Provides :matches pseudoclass, which tests the node itself.
+	 * Provides :matches pseudoclass (and its :is and :where aliases), which tests the node itself.
 	 *
 	 * @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

@@ -56,19 +56,28 @@ class Util
 	private const SELECTOR_ARGUMENT_PSEUDO_CLASSES = [
 		'not',
 		'matches',
+		'is',
+		'where',
 	];
 
+	/**
+	 * 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.
@@ -86,7 +95,7 @@ public static function isPositionalPseudoClass($name): bool
 	 * Check whether a pseudo-class has to be applied to the whole result set.
 	 *
 	 * That is true of the positional pseudo-classes themselves, and of
-	 * :not()/:matches() when their argument contains one.
+	 * :not()/:matches()/:is()/:where() when their argument contains one.
 	 *
 	 * @param string $name
 	 * @param mixed  $value
@@ -116,39 +125,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(), :matches(), :is() and :where() 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/CSS/SimpleSelector.php

@@ -196,17 +196,17 @@ public function isBlank(): bool
 	}
 
 	/**
+	 * Whether this restricts the elements it matches. The inverse of isBlank().
+	 *
+	 * As with isBlank(), the combinator is not counted: it relates this simple selector to the next,
+	 * and does not narrow what this one matches.
+	 *
 	 * @return bool
+	 * @deprecated Use ! isBlank(). This will be removed in 5.0.
 	 */
 	public function notEmpty(): bool
 	{
-		return ! empty($this->element)
-			   && ! empty($this->id)
-			   && ! empty($this->classes)
-			   && ! empty($this->combinator)
-			   && ! empty($this->attributes)
-			   && ! empty($this->pseudoClasses)
-			   && ! empty($this->pseudoElements);
+		return ! $this->isBlank();
 	}
 
 	public function __toString()