diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ac9391..14907c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ QueryPath Changelog - Fix `QueryPath::VERSION` and `QueryPath::VERSION_MAJOR` still reporting `3.2.2` and `3`. They now report `4.2.0` and `4`, and both are deprecated in favour of `\Composer\InstalledVersions::getPrettyVersion('gravitypdf/querypath')`, which cannot drift from the installed version (issue #83) - **Behaviour change.** Fix `:has()` never matching in `find()`, `filter()` and the other methods that use `QueryPath\CSS\DOMTraverser`, so that `ul:has(li)` selected nothing and `:not(:has(li))` selected everything (issue #121). `:has()` now matches an element with a descendant matching its argument, and supports relative selectors: `ul:has(> li)` (a child), `h2:has(+ p)` (the next sibling) and `h2:has(~ p)` (a later sibling). A positional pseudo-class in the argument indexes each element's own matches, so `ul:has(li:eq(2))` selects every list with at least three items - **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 `

` 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 `

  • ` 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 # 4.2.0 diff --git a/src/CSS/DOMTraverser.php b/src/CSS/DOMTraverser.php index e6c374a..7f443c9 100644 --- a/src/CSS/DOMTraverser.php +++ b/src/CSS/DOMTraverser.php @@ -7,6 +7,7 @@ use DOMDocument; use DOMElement; +use DOMNode; use DOMNodeList; use DOMXPath; use QueryPath\CSS\DOMTraverser\Util; @@ -164,20 +165,64 @@ public function debug($msg) */ public function find($selector): DOMTraverser { - // Setup - $handler = new Selector(); - $parser = new Parser($selector, $handler); - $parser->parse(); + return $this->findGroups($this->psHandler->parse($selector)); + } + + /** + * Reduce a set of nodes to those that, taken as candidates themselves, match a selector. + * + * Unlike find(), which searches the descendants of the nodes it starts from, only the given + * nodes can match: for ``, `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. + * + * @param SplObjectStorage $nodes + * The candidates, all from one document. + * @param string $selector + * A CSS selector. + * @param DOMNode|null $scopeNode + * The element `:scope` matches. Defaults to the document element, as in find(). + * @param PseudoClass|null $psHandler + * Share a handler across calls, so they reuse its parsed selectors and, while it has a query + * open (see PseudoClass::beginQuery()), its sibling index. + * + * @return SplObjectStorage + * The matching nodes, in the order they were given. + * @throws ParseException + */ + public static function filterNodes( + SplObjectStorage $nodes, + $selector, + $scopeNode = null, + ?PseudoClass $psHandler = null + ): SplObjectStorage { + if (count($nodes) === 0) { + return new SplObjectStorage(); + } + + $traverser = new self($nodes, true, $scopeNode, $psHandler); + $groups = $traverser->psHandler->parse($selector); + $matched = $traverser->findGroups($groups)->matches(); - return $this->findGroups($handler); + // A single group without a set filter is matched in the order given. Otherwise the result + // is in document order, or a union of groups, and has to be put back in input order. + if (count($matched) < 2 || (count($groups) === 1 && ! $traverser->groupIsSetFiltered($groups->toArray()[0]))) { + return $matched; + } + + $found = new SplObjectStorage(); + foreach ($nodes as $node) { + if ($matched->offsetExists($node)) { + $found->offsetSet($node); + } + } + + return $found; } /** * Find the matches of an already-parsed selector. * - * The traverser modifies the SimpleSelector objects it is given, so they must - * not be reused. - * * @param iterable $handler * The SimpleSelector[] groups, as a Selector yields them. * @@ -289,7 +334,10 @@ protected function resolveSubject(array $selectorGroup, int $index): SplObjectSt return $node instanceof DOMElement; }); } else { - $candidates = $this->initialMatch($selectorGroup[$index], $this->matches); + // initialMatch() consumes the parts of the simple selector it matched on. Give it a copy, + // so the caller's parsed selector can be reused. + $selectorGroup[$index] = clone $selectorGroup[$index]; + $candidates = $this->initialMatch($selectorGroup[$index], $this->matches); } $setFilters = $selectorGroup[$index]->setFilterPseudoClasses(); @@ -350,17 +398,14 @@ 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. - if (empty($nodes)) { - return []; + $inner = self::filterNodes($this->toMatches($nodes), $value, $this->scopeNode, $this->psHandler); + if ($name !== 'not') { + return iterator_to_array($inner, false); } - $traverser = new self($this->toMatches($nodes), true, $this->scopeNode, $this->psHandler); - $inner = $traverser->find($value)->matches(); - $filtered = []; foreach ($nodes as $node) { - $isMatch = $inner->offsetExists($node); - if ($name === 'not' ? ! $isMatch : $isMatch) { + if (! $inner->offsetExists($node)) { $filtered[] = $node; } } @@ -651,6 +696,9 @@ public function combineAnyDescendant($node, $selectors, $index) * calling the element-only DOM API on it. Everything downstream of here is therefore elements, * which is why matchesSimpleSelector() and friends can keep their DOMElement type hints. * + * It clears the parts of $selector it matched on, so it must be given a copy: parsed selectors + * are shared through PseudoClass::parse(). + * * @param \QueryPath\CSS\SimpleSelector $selector * @param SplObjectStorage $matches * diff --git a/src/CSS/DOMTraverser/PseudoClass.php b/src/CSS/DOMTraverser/PseudoClass.php index 62fe6b7..5815aed 100644 --- a/src/CSS/DOMTraverser/PseudoClass.php +++ b/src/CSS/DOMTraverser/PseudoClass.php @@ -26,6 +26,8 @@ /** * The PseudoClass handler. * + * It also holds what queries can share: parsed selectors and, while a query is open, sibling + * positions. Nested and per-node queries are passed their caller's handler for that reason. */ class PseudoClass { @@ -56,12 +58,19 @@ class PseudoClass protected $anbRules = []; /** - * Parsed :has() arguments, keyed by the argument. See relativeSelector(). + * :has() arguments as relativeSelector() returns them, keyed by the argument. * * @var array */ protected $relativeSelectors = []; + /** + * Parsed selectors, keyed by the selector string. See parse(). + * + * @var Selector[] + */ + protected $selectors = []; + /** * Start remembering sibling positions, so the :nth-* pseudo-classes index * each sibling list once rather than walking it for every element. @@ -82,6 +91,30 @@ public function endQuery(): void } } + /** + * Parse a selector, reusing an earlier parse of the same string. + * + * DOMTraverser does not modify a parsed selector, so every query that shares this handler can + * share the parse. + * + * @param string $selector + * + * @return Selector + * @throws ParseException + */ + public function parse($selector): Selector + { + if (! isset($this->selectors[$selector])) { + $handler = new Selector(); + $parser = new Parser($selector, $handler); + $parser->parse(); + + $this->selectors[$selector] = $handler; + } + + return $this->selectors[$selector]; + } + /** * Tests whether the given element matches the given pseudoclass. * @@ -455,19 +488,19 @@ protected function relativeSelector($selector): array return $this->relativeSelectors[$selector]; } - $handler = new Selector(); - $parser = new Parser($selector, $handler); - $parser->parse(); - $sibling = SimpleSelector::ADJACENT | SimpleSelector::SIBLING; $groups = []; - foreach ($handler as $group) { - // Groups are stored right-to-left, so the last simple selector is the leftmost. + foreach ($this->parse($selector) as $group) { + // Groups are stored right-to-left, so the last simple selector is the leftmost. It is + // copied before :scope is added, as the parse is shared. $anchor = end($group); if (count($group) === 1 || ! $anchor->isBlank()) { $anchor = new SimpleSelector(); $anchor->combinator = SimpleSelector::ANY_DESCENDANT; $group[] = $anchor; + } else { + $anchor = clone $anchor; + $group[count($group) - 1] = $anchor; } $anchor->pseudoClasses[] = ['name' => 'scope', 'value' => null]; @@ -502,15 +535,9 @@ protected function findsAny(DOMElement $root, DOMElement $scope, array $group, b { $splos = new SPLObjectStorage(); $splos->offsetSet($root); - - // The traverser consumes the element and ID selectors as it matches them, so give it a copy. - $copy = []; - foreach ($group as $simpleSelector) { - $copy[] = clone $simpleSelector; - } $traverser = new DOMTraverser($splos, $self, $scope, $this); - return count($traverser->findGroups([$copy])->matches()) > 0; + return count($traverser->findGroups([$group])->matches()) > 0; } /** @@ -522,10 +549,8 @@ protected function is($node, $selector): bool { $splos = new SPLObjectStorage(); $splos->offsetSet($node); - $traverser = new DOMTraverser($splos, true, null, $this); - $results = $traverser->find($selector)->matches(); - return count($results) > 0; + return count(DOMTraverser::filterNodes($splos, $selector, null, $this)) > 0; } /** diff --git a/src/Helpers/NodeMatcher.php b/src/Helpers/NodeMatcher.php index 27094c8..ea6024d 100644 --- a/src/Helpers/NodeMatcher.php +++ b/src/Helpers/NodeMatcher.php @@ -11,10 +11,7 @@ /** * Test nodes that are already in hand against a CSS selector. * - * Unlike find(), which searches the descendants of the nodes it is given, the - * supplied nodes are themselves the only candidates: for ``, `b` matches the - * `b` element and not `a`. Combinators are still resolved against the real document, so - * full selectors (e.g. `div > p`) keep working. + * The supplied nodes are themselves the only candidates, as in DOMTraverser::filterNodes(). * * Prefer the set methods wherever there is a set. A per-node pass cannot answer a selector * that describes a position within the set (:first, :eq(n), :odd, ...), because each node @@ -22,8 +19,9 @@ * * matches() remembers each per-node answer, because the walk-and-stop traversals * (closest(), nextUntil(), ...) revisit the same nodes from every starting element, and - * every query shares one sibling index for the :nth-* pseudo-classes. An instance must not - * outlive the call that made it, or a later DOM change could leave it holding stale answers. + * every query shares one parse of the selector and one sibling index for the :nth-* + * pseudo-classes. An instance must not outlive the call that made it, or a later DOM change + * could leave it holding stale answers. */ final class NodeMatcher { @@ -78,33 +76,16 @@ public function matches($node): bool } /** - * Reduce a set of nodes to those that match the selector, preserving their order. - * - * Nodes that are not elements can never match a CSS selector, and are skipped. + * Alias of matching(). * * @param SplObjectStorage $nodes - * The candidate nodes. * * @return SplObjectStorage * @throws ParseException */ public function filter(SplObjectStorage $nodes): SplObjectStorage { - if (count($nodes) === 0) { - return new SplObjectStorage(); - } - - $matched = $this->matching($nodes); - - // The traverser returns document order; callers such as parents() need their own. - $found = new SplObjectStorage(); - foreach ($nodes as $node) { - if ($matched->offsetExists($node)) { - $found->offsetSet($node); - } - } - - return $found; + return $this->matching($nodes); } /** @@ -117,11 +98,13 @@ public function filter(SplObjectStorage $nodes): SplObjectStorage */ public function matchesAny(SplObjectStorage $nodes): bool { - return count($nodes) > 0 && count($this->matching($nodes)) > 0; + return count($this->matching($nodes)) > 0; } /** - * Find the nodes that match the selector, in document order. + * Reduce a set of nodes to those that match the selector, in the order they were given. + * + * Nodes that are not elements can never match a CSS selector, and are skipped. * * @param SplObjectStorage $nodes * @@ -130,12 +113,6 @@ public function matchesAny(SplObjectStorage $nodes): bool */ public function matching(SplObjectStorage $nodes): SplObjectStorage { - // "Initialized" mode makes the traverser take the supplied set as its candidates rather - // than seeding them with a descendant search. The scope node is left at its default (the - // document element) so that `:scope` means the same here as in find(); scoping to a - // candidate would make every candidate match `:scope`. - $traverser = new DOMTraverser($nodes, true, null, $this->psHandler); - - return $traverser->find($this->selector)->matches(); + return DOMTraverser::filterNodes($nodes, $this->selector, null, $this->psHandler); } } diff --git a/tests/QueryPath/CSS/DOMTraverserTest.php b/tests/QueryPath/CSS/DOMTraverserTest.php index f065708..94e8779 100644 --- a/tests/QueryPath/CSS/DOMTraverserTest.php +++ b/tests/QueryPath/CSS/DOMTraverserTest.php @@ -3,7 +3,9 @@ namespace QueryPathTests\CSS; use DOMDocument; +use DOMXPath; use QueryPath\CSS\DOMTraverser; +use QueryPath\CSS\DOMTraverser\PseudoClass; use QueryPathTests\TestCase; use SPLObjectStorage; @@ -37,7 +39,7 @@ public function testConstructor() $this->assertInstanceOf('\QueryPath\CSS\DOMTraverser', $traverser); } - protected function traverser($xml = null) + protected function traverser($xml = null, ?PseudoClass $ps = null) { $dom = new DOMDocument('1.0'); if ($xml === null) { @@ -49,9 +51,7 @@ protected function traverser($xml = null) $splos = new SPLObjectStorage(); $splos->offsetSet($dom->documentElement); - $traverser = new DOMTraverser($splos); - - return $traverser; + return new DOMTraverser($splos, false, null, $ps); } protected function find($selector) @@ -376,7 +376,7 @@ public function testMultipleSelectors() /** * The
      children are, by position: l1 p1 l2 l3 p2 l4 l5 p3. */ - protected $nthXml = '
        ' + protected $nthXml = '
          ' . 'text

        • ' . 'text

        • text' . '

        '; @@ -475,4 +475,93 @@ public function testNthPseudoClassesAfterMutation() $this->assertSame(['c'], $this->ids($qp->find('li:nth-child(3)')->get())); $this->assertSame(['new', 'c'], $this->ids($qp->find('li:nth-child(odd)')->get())); } + + /** + * Load $nthXml and collect the nodes with the given IDs, in the given order. + */ + protected function nthNodes(string ...$ids): SplObjectStorage + { + $dom = new DOMDocument('1.0'); + $dom->loadXML($this->nthXml); + + $xpath = new DOMXPath($dom); + $nodes = new SplObjectStorage(); + foreach ($ids as $id) { + $nodes->offsetSet($xpath->query("//*[@id='$id']")->item(0)); + } + + return $nodes; + } + + public function filterNodesProvider(): array + { + return [ + 'keeps the given order' => [['l4', 'p1', 'l2', 'l1'], 'li', ['l4', 'l2', 'l1']], + 'tests the nodes, not below' => [['ul', 'l1'], 'ul', ['ul']], + 'resolves combinators' => [['o1', 'l2', 'l1'], 'ul > li', ['l2', 'l1']], + 'selector list' => [['p2', 'o2', 'l1'], 'p, ol > li', ['p2', 'o2']], + ':first is first in the set' => [['l4', 'l2', 'l3'], 'li:first', ['l2']], + ':eq() counts within the set' => [['l4', 'l2', 'l3'], ':eq(1)', ['l3']], + ':odd counts within the set' => [['l5', 'l4', 'l2', 'l1'], 'li:odd', ['l5', 'l2']], + ':not() with a positional arg' => [['l4', 'l2', 'l3'], 'li:not(:first)', ['l4', 'l3']], + ':matches() with positional arg' => [['l4', 'l2', 'l3'], ':matches(:last)', ['l4']], + ':nth-child counts siblings' => [['l4', 'l2', 'l3'], ':nth-child(4)', ['l3']], + 'document element' => [['root', 'ul'], 'root', ['root']], + ':root' => [['ul', 'root'], ':root', ['root']], + ':scope is the document element' => [['ul', 'root'], ':scope', ['root']], + ':scope is the scope node' => [['ul', 'l1', 'l2', 'o1'], ':scope', ['ul'], 'ul'], + 'combinator from the scope node' => [['ul', 'l1', 'l2', 'o1'], ':scope > li', ['l1', 'l2'], 'ul'], + 'empty set' => [[], 'li', []], + 'empty set, positional' => [[], 'li:first', []], + ]; + } + + /** + * @dataProvider filterNodesProvider + */ + public function testFilterNodes(array $ids, $selector, array $expected, $scopeId = null) + { + $nodes = $this->nthNodes(...$ids); + $scope = null; + foreach ($nodes as $node) { + if ($node->getAttribute('id') === $scopeId) { + $scope = $node; + } + } + + $this->assertSame($expected, $this->ids(DOMTraverser::filterNodes($nodes, $selector, $scope))); + } + + public function testFilterNodesSkipsNodesThatAreNotElements() + { + $ul = $this->nthNodes('ul'); + $ul->rewind(); + + // Every child of the
          , last first: text and comments interleave the elements. + $nodes = new SplObjectStorage(); + for ($child = $ul->current()->lastChild; $child !== null; $child = $child->previousSibling) { + $nodes->offsetSet($child); + } + + $this->assertSame(['p3', 'l5', 'l4', 'p2', 'l3', 'l2', 'p1', 'l1'], $this->ids(DOMTraverser::filterNodes($nodes, '*'))); + $this->assertSame(['l5', 'l4', 'l3', 'l2', 'l1'], $this->ids(DOMTraverser::filterNodes($nodes, ':not(p)'))); + $this->assertSame(['l1'], $this->ids(DOMTraverser::filterNodes($nodes, ':first'))); + } + + /** + * A shared handler hands every query the same parsed selector, which the traverser must leave + * as it found it. + * + * @testWith ["#ul li", ["l1", "l2", "l3", "l4", "l5"]] + * [".x", ["x"]] + * ["ul:first > li", ["l1", "l2", "l3", "l4", "l5"]] + */ + public function testQueriesSharingAHandlerReuseTheParsedSelector($selector, array $expected) + { + $ps = new PseudoClass(); + $xml = str_replace('
            ', '
              ', $this->nthXml); + for ($i = 0; $i < 2; $i++) { + $this->assertSame($expected, $this->ids($this->traverser($xml, $ps)->find($selector)->matches())); + } + } }