diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e390a2..2e3f0fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,14 @@ QueryPath Changelog # Unreleased changes +- **Breaking behaviour change:** `DOMQuery::is()` now behaves like jQuery's `.is()`. It tests the elements held in the current match set and returns `true` when at least one of them matches the selector. Previously it ran a descendant search, so `html5qp('

foo

', 'p')->is('span')` returned `true`. Use `has()` if you need the old "does the set contain something matching this selector" behaviour (#51) +- `DOMQuery::is()` no longer raises a fatal error when the match set contains non-element nodes (text nodes, comments, processing instructions). Those nodes cannot match a CSS selector, so they are skipped (#51) +- Fix `parents($selector)` so the selector filters the ancestors themselves, instead of matching any ancestor that merely *contains* an element matching the selector. `qp($xml, 'Demographics > Age > Name')->parents('Demographics')` now returns only ``, matching jQuery (#62) +- Apply the same fix to the other selector-filtered traversal methods in `QueryPath\Helpers\QueryFilters`: `parent()`, `parents()`, `parentsUntil()`, `closest()`, `next()`, `nextAll()`, `nextUntil()`, `prev()`, `prevAll()`, `prevUntil()`, `siblings()`, and `not()` (#62) +- **Behaviour change:** `parents()` and `parentsUntil()` now return their results in reverse document order with duplicates removed, as jQuery does. Previously a set built from more than one starting element was grouped by starting element (#62) +- **Breaking behaviour change:** `DOMQuery::filter()` now narrows the match set to the members that match the selector, as jQuery does, instead of keeping any element that merely *contains* a match. `qp($file, 'inner')->filter('li')` returned both `` elements and now returns none. Use `has()` for the old behaviour +- Fix `children($selector)`, which resolved `:scope` against each child rather than the document element, so `children(':scope')` matched every child +- `filter()` and `children($selector)` now evaluate the selector against the whole candidate set in a single pass, rather than one node at a time. A per-node pass cannot evaluate a selector describing a position within the set, since each node is the only member of its own one-element set - Reorganise, modernise, and repair the `examples/` directory. Each example now lives in its own subdirectory with an `index.php`, and the full set is indexed in `examples/quickstart-guide.md` - Convert the remaining legacy examples: `simple_example.php`, `techniques.php`, `svg.php`, `rss.php`, `odt.php`, `parse_php.php`, and `sparql.php` - Fix examples that no longer ran: send a `User-Agent` where remote hosts now require one, resolve paths relative to the example rather than the working directory, and stop relying on the removed `qp.php` autoloader and the PHP 8 incompatible `eachLambda()` diff --git a/src/CSS/DOMTraverser/Util.php b/src/CSS/DOMTraverser/Util.php index a7206b2..95e997f 100644 --- a/src/CSS/DOMTraverser/Util.php +++ b/src/CSS/DOMTraverser/Util.php @@ -7,7 +7,9 @@ namespace QueryPath\CSS\DOMTraverser; +use DOMNode; use QueryPath\CSS\EventHandler; +use SplObjectStorage; /** * Utilities for DOM Traversal. @@ -165,4 +167,120 @@ public static function parseAnB($rule): array return [$aVal, $bVal]; } + + /** + * Sort nodes into document order. + * + * PHP's DOM has no compareDocumentPosition(), so each node is described by the list of + * child offsets from the document down to it, and those lists are compared element by + * element. Sibling offsets are indexed a whole child list at a time and memoized for the + * duration of the sort — computing them by walking previousSibling per node is quadratic + * on the width of the parent. + * + * The memo lives only as long as the call, so a document mutated between calls cannot + * produce a stale answer. + * + * @param array $nodes + * + * @return array + */ + public static function sortDocumentOrder(array $nodes): array + { + if (count($nodes) < 2) { + return array_values($nodes); + } + + $offsets = new SplObjectStorage(); + $indexed = []; + foreach ($nodes as $node) { + $indexed[] = [self::documentOrderPath($node, $offsets), $node]; + } + + usort($indexed, function ($a, $b) { + return self::comparePaths($a[0], $b[0]); + }); + + $sorted = []; + foreach ($indexed as $entry) { + $sorted[] = $entry[1]; + } + + return $sorted; + } + + /** + * Describe a node's position as the child offsets from the document down to it. + * + * @param DOMNode $node + * @param SplObjectStorage $offsets + * + * @return array + */ + private static function documentOrderPath($node, SplObjectStorage $offsets): array + { + $path = []; + while ($node instanceof DOMNode && $node->parentNode !== null) { + $path[] = self::siblingOffset($node, $offsets); + $node = $node->parentNode; + } + + // Built leaf-first; array_unshift() per step would be quadratic on depth. + return array_reverse($path); + } + + /** + * A node's offset among its parent's children. + * + * The parent's whole child list is indexed on the first request, so sorting a wide set of + * siblings costs one pass over the list rather than one pass per node. + * + * @param DOMNode $node + * @param SplObjectStorage $offsets + * + * @return int + */ + private static function siblingOffset($node, SplObjectStorage $offsets): int + { + if ($offsets->offsetExists($node)) { + return $offsets[$node]; + } + + $offset = 0; + foreach ($node->parentNode->childNodes as $sibling) { + $offsets[$sibling] = $offset++; + } + + if ($offsets->offsetExists($node)) { + return $offsets[$node]; + } + + // The node has to be one of its parent's children, but do not assume the DOM handed + // back the same PHP object we were given. + $offset = 0; + for ($sibling = $node->previousSibling; $sibling !== null; $sibling = $sibling->previousSibling) { + ++$offset; + } + + return $offset; + } + + /** + * Compare two paths from documentOrderPath(). + * + * @param array $a + * @param array $b + * + * @return int + */ + private static function comparePaths(array $a, array $b): int + { + $shared = min(count($a), count($b)); + for ($i = 0; $i < $shared; ++$i) { + if ($a[$i] !== $b[$i]) { + return $a[$i] < $b[$i] ? -1 : 1; + } + } + + return count($a) - count($b); + } } diff --git a/src/Helpers/NodeMatcher.php b/src/Helpers/NodeMatcher.php new file mode 100644 index 0000000..4a91112 --- /dev/null +++ b/src/Helpers/NodeMatcher.php @@ -0,0 +1,104 @@ +descendants of the nodes it is given, so for `` it would report + * that the `a` element "matches" the selector `b`. Here the supplied nodes are + * themselves the only candidates, so `b` matches the `b` element and nothing else. + * + * The DOMTraverser is built in "initialized" mode, which tells it to treat the + * supplied set as the candidate set rather than seeding it with a descendant + * search. Combinators are still resolved against the real document by walking up + * from each candidate, so full selectors (e.g. `div > p`) keep working. + * + * The scope node is deliberately left at its default (the document element) so that + * `:scope` means the same thing here as it does in find(). Passing a candidate as the + * scope node would make every candidate match `:scope`. + */ +final class NodeMatcher +{ + + /** + * Reduce a set of nodes to those that match a selector. + * + * Nodes that are not elements can never match a CSS selector, and are skipped + * rather than raising an error. + * + * @param SplObjectStorage $nodes + * The candidate nodes. + * @param string $selector + * A valid CSS selector. + * + * @return SplObjectStorage + * The subset of $nodes that match the selector. + * @throws ParseException + */ + public static function filter(SplObjectStorage $nodes, $selector): SplObjectStorage + { + $candidates = new SplObjectStorage(); + foreach ($nodes as $node) { + if ($node instanceof DOMElement) { + $candidates->offsetSet($node); + } + } + + if (count($candidates) === 0) { + return $candidates; + } + + $traverser = new DOMTraverser($candidates, true); + $traverser->find($selector); + $matched = $traverser->matches(); + + // Returned in the caller's order rather than the traverser's, so callers do not each + // have to re-walk their own input to restore it. + $found = new SplObjectStorage(); + foreach ($nodes as $node) { + if ($matched->offsetExists($node)) { + $found->offsetSet($node); + } + } + + return $found; + } + + /** + * Test whether at least one of the given nodes matches a selector. + * + * @param SplObjectStorage $nodes + * @param string $selector + * + * @return bool + * @throws ParseException + */ + /** + * Test whether a single node, taken as an element, matches a selector. + * + * @param mixed $node + * The node to test. Anything that is not an element returns FALSE. + * @param string $selector + * + * @return bool + * @throws ParseException + */ + public static function matchesNode($node, $selector): bool + { + if (! $node instanceof DOMElement) { + return false; + } + + $nodes = new SplObjectStorage(); + $nodes->offsetSet($node); + + return count(self::filter($nodes, $selector)) > 0; + } +} diff --git a/src/Helpers/QueryChecks.php b/src/Helpers/QueryChecks.php index cdb4b6c..d55f884 100644 --- a/src/Helpers/QueryChecks.php +++ b/src/Helpers/QueryChecks.php @@ -21,20 +21,33 @@ trait QueryChecks { /** - * Given a selector, this checks to see if the current set has one or more matches. + * Check the current set of elements against a selector, and return TRUE if at least + * one of them matches. + * + * This behaves like jQuery's is(): only the elements in the current match set are + * tested. Neither the descendants nor the ancestors of those elements are considered. + * Use has() if you need to know whether the current set contains something that + * matches a selector. * * Unlike jQuery's version, this supports full selectors (not just simple ones). * - * @param string|DOMNode $selector - * The selector to search for. As of QueryPath 2.1.1, this also supports passing a - * DOMNode object. + * Non-element nodes in the match set (text nodes, comments, processing instructions, and + * so on) can never match a CSS selector, and are simply skipped. + * + * @param string|DOMNode|Traversable $selector + * The selector to test the current match set against. As of QueryPath 2.1.1, this also + * supports passing a DOMNode object, in which case the current match set must consist of + * exactly that one node, or a Traversable (e.g. another DOMQuery's match set), in which + * case the two sets must contain exactly the same nodes. * * @return boolean * TRUE if one or more elements match. FALSE if no match is found. * @throws Exception - * @throws Exception + * @throws ParseException * @see get() * @see eq() + * @see has() + * @see filter() */ public function is($selector): bool { @@ -63,7 +76,9 @@ public function is($selector): bool throw new Exception('Cannot compare an object to a DOMQuery.'); } - return $this->branch($selector)->count() > 0; + // Only the elements in the match set are candidates: a descendant that matches + // the selector must not make is() true. See NodeMatcher for the details. + return count(NodeMatcher::filter($this->matches, $selector)) > 0; } /** diff --git a/src/Helpers/QueryFilters.php b/src/Helpers/QueryFilters.php index 6b39a07..e6809c2 100644 --- a/src/Helpers/QueryFilters.php +++ b/src/Helpers/QueryFilters.php @@ -3,12 +3,12 @@ namespace QueryPath\Helpers; use DOMElement; -use QueryPath\CSS\DOMTraverser; +use DOMNode; +use QueryPath\CSS\DOMTraverser\Util; use QueryPath\CSS\ParseException; use QueryPath\DOMQuery; use QueryPath\Exception; use QueryPath\Query; -use QueryPath\QueryPath; use SplObjectStorage; use stdClass; @@ -41,23 +41,11 @@ trait QueryFilters */ public function filter($selector): Query { - $found = new SplObjectStorage(); - $tmp = new SplObjectStorage(); - - foreach ($this->matches as $m) { - $tmp->offsetSet($m); - // Seems like this should be right... but it fails unit - // tests. Need to compare to jQuery. - // $query = new \QueryPath\CSS\DOMTraverser($tmp, TRUE, $m); - $query = new DOMTraverser($tmp); - $query->find($selector); - if (count($query->matches())) { - $found->offsetSet($m); - } - $tmp->offsetUnset($m); - } - - return $this->inst($found, null); + // The whole match set is filtered in one pass rather than one node at a time. + // A per-node pass cannot evaluate a selector that describes a position within the + // set (:first, :eq(n), :odd, ...), because each node would be the only member of + // its own one-element set. + return $this->inst(NodeMatcher::filter($this->matches, $selector), null); } /** @@ -577,7 +565,7 @@ public function nextUntil($selector = null): Query while (isset($m->nextSibling)) { $m = $m->nextSibling; if ($m->nodeType === XML_ELEMENT_NODE) { - if (null !== $selector && QueryPath::with($m, null, $this->options)->is($selector) > 0) { + if (null !== $selector && $this->matchesNodeSelector($m, $selector)) { break; } $found->offsetSet($m); @@ -616,7 +604,7 @@ public function prevUntil($selector = null): Query while (isset($m->previousSibling)) { $m = $m->previousSibling; if ($m->nodeType === XML_ELEMENT_NODE) { - if (null !== $selector && QueryPath::with($m, null, $this->options)->is($selector)) { + if (null !== $selector && $this->matchesNodeSelector($m, $selector)) { break; } @@ -654,7 +642,7 @@ public function parentsUntil($selector = null): Query // Is there any case where parent node is not an element? if ($m->nodeType === XML_ELEMENT_NODE) { if (! empty($selector)) { - if (QueryPath::with($m, null, $this->options)->is($selector) > 0) { + if ($this->matchesNodeSelector($m, $selector)) { break; } $found->offsetSet($m); @@ -665,7 +653,7 @@ public function parentsUntil($selector = null): Query } } - return $this->inst($found, null); + return $this->inst($this->sortReverseDocumentOrder($found), null); } /** @@ -725,8 +713,9 @@ public function not($selector): Query } } } else { + $matched = $this->filterCandidates($this->matches, $selector); foreach ($this->matches as $m) { - if (! QueryPath::with($m, null, $this->options)->is($selector)) { + if (! $matched->offsetExists($m)) { $found->offsetSet($m); } } @@ -756,17 +745,13 @@ public function closest($selector): Query { $found = new SplObjectStorage(); foreach ($this->matches as $m) { - if (QueryPath::with($m, null, $this->options)->is($selector) > 0) { + if ($this->matchesNodeSelector($m, $selector)) { $found->offsetSet($m); } else { while ($m->parentNode->nodeType !== XML_DOCUMENT_NODE) { $m = $m->parentNode; // Is there any case where parent node is not an element? - if ($m->nodeType === XML_ELEMENT_NODE && QueryPath::with( - $m, - null, - $this->options - )->is($selector) > 0) { + if ($this->matchesNodeSelector($m, $selector)) { $found->offsetSet($m); break; } @@ -838,29 +823,39 @@ public function parents($selector = null): Query private function getParentElements(?string $selector, bool $immediate): Query { $found = new SplObjectStorage(); + + // parent() stops at the nearest matching ancestor of each element, so its candidates + // have to be tested one at a time as the walk reaches them. + if ($immediate) { + foreach ($this->matches as $m) { + while ($m->parentNode && $m->parentNode->nodeType !== XML_DOCUMENT_NODE) { + $m = $m->parentNode; + // Is there any case where parent node is not an element? + if ($m->nodeType === XML_ELEMENT_NODE + && (empty($selector) || $this->matchesNodeSelector($m, $selector))) { + $found->offsetSet($m); + break; + } + } + } + + return $this->inst($found, null); + } + foreach ($this->matches as $m) { while ($m->parentNode && $m->parentNode->nodeType !== XML_DOCUMENT_NODE) { $m = $m->parentNode; // Is there any case where parent node is not an element? if ($m->nodeType === XML_ELEMENT_NODE) { - if (! empty($selector)) { - if (QueryPath::with($m, null, $this->options)->is($selector) > 0) { - $found->offsetSet($m); - if ($immediate) { - break; - } - } - } else { - $found->offsetSet($m); - if ($immediate) { - break; - } - } + $found->offsetSet($m); } } } - return $this->inst($found, null); + // jQuery returns the ancestors of a multi-element set in reverse document order, with + // duplicates removed. The selector is applied after that ordering, so a positional + // selector counts from the closest ancestor. + return $this->inst($this->filterCandidates($this->sortReverseDocumentOrder($found), $selector), null); } /** @@ -890,7 +885,7 @@ public function next($selector = null): Query $m = $m->nextSibling; if ($m->nodeType === XML_ELEMENT_NODE) { if (! empty($selector)) { - if (QueryPath::with($m, null, $this->options)->is($selector) > 0) { + if ($this->matchesNodeSelector($m, $selector)) { $found->offsetSet($m); break; } @@ -931,18 +926,12 @@ public function nextAll($selector = null): Query while (isset($m->nextSibling)) { $m = $m->nextSibling; if ($m->nodeType === XML_ELEMENT_NODE) { - if (! empty($selector)) { - if (QueryPath::with($m, null, $this->options)->is($selector) > 0) { - $found->offsetSet($m); - } - } else { - $found->offsetSet($m); - } + $found->offsetSet($m); } } } - return $this->inst($found, null); + return $this->inst($this->filterCandidates($found, $selector), null); } /** @@ -973,7 +962,7 @@ public function prev($selector = null): Query $m = $m->previousSibling; if ($m->nodeType === XML_ELEMENT_NODE) { if (! empty($selector)) { - if (QueryPath::with($m, null, $this->options)->is($selector)) { + if ($this->matchesNodeSelector($m, $selector)) { $found->offsetSet($m); break; } @@ -1014,18 +1003,12 @@ public function prevAll($selector = null): Query while (isset($m->previousSibling)) { $m = $m->previousSibling; if ($m->nodeType === XML_ELEMENT_NODE) { - if (! empty($selector)) { - if (QueryPath::with($m, null, $this->options)->is($selector)) { - $found->offsetSet($m); - } - } else { - $found->offsetSet($m); - } + $found->offsetSet($m); } } } - return $this->inst($found, null); + return $this->inst($this->filterCandidates($found, $selector), null); } /** @@ -1048,33 +1031,21 @@ public function prevAll($selector = null): Query */ public function children($selector = null): Query { - $found = new SplObjectStorage(); - $filter = is_string($selector) && strlen($selector) > 0; - - if ($filter) { - $tmp = new SplObjectStorage(); - } + $children = new SplObjectStorage(); foreach ($this->matches as $m) { foreach ($m->childNodes as $c) { if ($c->nodeType === XML_ELEMENT_NODE) { - // This is basically an optimized filter() just for children(). - if ($filter) { - $tmp->offsetSet($c); - $query = new DOMTraverser($tmp, true, $c); - $query->find($selector); - if (count($query->matches()) > 0) { - $found->offsetSet($c); - } - $tmp->offsetUnset($c); - } // No filter. Just attach it. - else { - $found->offsetSet($c); - } + $children->offsetSet($c); } } } - return $this->inst($found, null); + if (! is_string($selector) || strlen($selector) === 0) { + return $this->inst($children, null); + } + + // Filter the children as one set, for the same reason filter() does. + return $this->inst(NodeMatcher::filter($children, $selector), null); } /** @@ -1147,10 +1118,81 @@ public function siblings($selector = null): Query } } } - if (empty($selector)) { - return $this->inst($found, null); + + return $this->inst($this->filterCandidates($found, $selector), null); + } + + /** + * Test whether a single node, taken as an element, matches a CSS selector. + * + * The node itself is the only candidate, so this asks "is this node a match?" + * rather than "does this node contain a match?" — which is what running a + * find() against the node would ask. + * + * @param DOMNode $node + * The node to test. + * @param string $selector + * A valid CSS selector. + * + * @return bool + * TRUE if the node is an element and matches the selector. + * @throws ParseException + * @see NodeMatcher + */ + private function matchesNodeSelector($node, $selector): bool + { + return NodeMatcher::matchesNode($node, $selector); + } + + /** + * Sort a set of nodes into reverse document order. + * + * 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 + * The nodes to sort. Duplicates are already removed by SplObjectStorage. + * + * @return SplObjectStorage + * The same nodes, in reverse document order. + */ + private function sortReverseDocumentOrder(SplObjectStorage $nodes): SplObjectStorage + { + if (count($nodes) < 2) { + return $nodes; + } + + $ordered = array_reverse(Util::sortDocumentOrder(iterator_to_array($nodes, false))); + + $sorted = new SplObjectStorage(); + foreach ($ordered as $node) { + $sorted->offsetSet($node); + } + + return $sorted; + } + + /** + * Reduce a set of candidates to those matching a selector, preserving their order. + * + * The selector is evaluated against the candidates as one set. Evaluating it per node + * cannot answer a selector that describes a position within the set — :first, :eq(n), + * :odd — because each node would be the only member of its own one-element set. It also + * re-parses the selector once per candidate. + * + * @param SplObjectStorage $candidates + * @param string $selector + * + * @return SplObjectStorage + * @throws ParseException + */ + private function filterCandidates(SplObjectStorage $candidates, $selector): SplObjectStorage + { + if (empty($selector) || count($candidates) === 0) { + return $candidates; } - return $this->inst($found, null)->filter($selector); + return NodeMatcher::filter($candidates, $selector); } } diff --git a/tests/Issues/Issue51Test.php b/tests/Issues/Issue51Test.php new file mode 100644 index 0000000..930f0c2 --- /dev/null +++ b/tests/Issues/Issue51Test.php @@ -0,0 +1,154 @@ +foo

', 'p'); + + self::assertTrue($dom->is('p'), 'Should match an element held directly in the collection'); + self::assertFalse($dom->is('span'), 'Should not match a descendant of an element held in the collection'); + } + + /** + * The same document, but with the collection left at the document element. Neither the + *

nor the is in the collection, so neither may match. + */ + public function testIsDoesNotMatchDeeperDescendants(): void + { + $dom = html5qp('

foo

'); + + self::assertTrue($dom->is('html'), 'The collection holds the document element'); + self::assertFalse($dom->is('p'), 'Should not match a descendant of an element held in the collection'); + self::assertFalse($dom->is('span'), 'Should not match a descendant of an element held in the collection'); + } + + public function testIsDoesNotMatchAncestors(): void + { + $dom = html5qp('

foo

', 'span'); + + self::assertTrue($dom->is('span')); + self::assertFalse($dom->is('p'), 'Should not match the parent of an element held in the collection'); + self::assertFalse($dom->is('#outer'), 'Should not match an ancestor of an element held in the collection'); + } + + /** + * jQuery returns TRUE when *at least one* of the elements in the set matches. + */ + public function testIsMatchesWhenAnyElementInTheSetMatches(): void + { + $dom = html5qp('
  • 1
  • 2
  • 3
', 'li'); + + self::assertCount(3, $dom); + self::assertTrue($dom->is('li')); + self::assertTrue($dom->is('#one'), 'The first element matches'); + self::assertTrue($dom->is('.two'), 'The second element matches'); + self::assertTrue($dom->is('li, dt'), 'Selector groups are supported'); + self::assertFalse($dom->is('.missing')); + self::assertFalse($dom->is('ul'), 'Should not match the parent of the elements in the collection'); + } + + /** + * The elements are still tested in the context of their own document, so combinators and + * positional pseudo-classes continue to work. + */ + public function testIsSupportsFullSelectors(): void + { + $dom = html5qp('

foo

', 'p'); + + self::assertTrue($dom->is('div p'), 'Descendant combinators are evaluated against the document'); + self::assertTrue($dom->is('div > p'), 'Child combinators are evaluated against the document'); + self::assertTrue($dom->is('p#pp.one.two')); + self::assertTrue($dom->is('p[id="pp"]')); + self::assertTrue($dom->is(':first-child')); + self::assertFalse($dom->is('section p'), 'A non-matching ancestor means no match'); + self::assertFalse($dom->is(':root')); + } + + public function testIsOnAnEmptyCollectionIsFalse(): void + { + $dom = html5qp('

foo

', 'section'); + + self::assertCount(0, $dom); + self::assertFalse($dom->is('*')); + } + + /** + * Nodes that are not elements can never match a CSS selector, and must not raise an error. + */ + public function testIsOnNonElementNodesIsFalse(): void + { + $contents = html5qp('
Sample
', 'div')->contents(); + + self::assertCount(2, $contents); + self::assertFalse($contents->is('div')); + self::assertFalse($contents->is('*')); + } + + /** + * The DOMNode and Traversable overloads are unchanged. + */ + public function testIsStillAcceptsADomNode(): void + { + $dom = html5qp('
  • 1
  • 2
'); + $one = $dom->top('#one'); + $node = $one->get(0); + + self::assertTrue($one->is($node)); + self::assertFalse($dom->top('#two')->is($node)); + self::assertFalse($dom->top('li')->is($node), 'A single node cannot equal a set of two'); + } + + public function testIsStillAcceptsATraversable(): void + { + $dom = html5qp('
  • 1
  • 2
'); + + $list = new SplDoublyLinkedList(); + $list->push($dom->top('#one')->get(0)); + $list->push($dom->top('#two')->get(0)); + + self::assertTrue($dom->top('#one,#two')->is($list)); + self::assertFalse($dom->top('#one')->is($list)); + } + + /** + * has() is the migration path for anyone who relied on the old containment behaviour. + */ + public function testHasProvidesTheOldContainmentBehaviour(): void + { + $dom = html5qp('

foo

', 'p'); + + self::assertFalse($dom->is('span')); + self::assertCount(1, $dom->branch()->has('span')); + self::assertCount(0, $dom->branch()->has('em')); + } + + /** + * parents() filters with is(), so it inherited the descendant-matching bug: every ancestor + * that merely *contained* a matching element was returned. + */ + public function testParentsNoLongerMatchesAncestorsThatOnlyContainTheSelector(): void + { + $dom = html5qp('

foo

'); + + self::assertCount(1, $dom->top('#target')->parents('div')); + self::assertSame('outer', $dom->top('#target')->parents('div')->attr('id')); + } +} diff --git a/tests/Issues/Issue62Test.php b/tests/Issues/Issue62Test.php new file mode 100644 index 0000000..589362f --- /dev/null +++ b/tests/Issues/Issue62Test.php @@ -0,0 +1,239 @@ +tag(); + } + + return $tags; + } + + /** + * @throws Exception + */ + public function testParentsFiltersAncestorsBySelector() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(1, $qp->count()); + $this->assertEquals(['Demographics'], $this->tags($qp->parents('Demographics'))); + } + + /** + * The ancestor must be matched as an element, not by asking whether it + * contains something matching the selector. + * + * @throws Exception + */ + public function testParentsDoesNotMatchAncestorsThatMerelyContainTheSelector() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(['Age'], $this->tags($qp->parents('Age'))); + $this->assertEquals(['AmplifyReturn'], $this->tags($qp->parents('AmplifyReturn'))); + $this->assertEquals(0, $qp->parents('Name')->count()); + $this->assertEquals(0, $qp->parents('Value')->count()); + } + + /** + * @throws Exception + */ + public function testParentsWithoutSelectorReturnsEveryAncestor() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals( + ['Age', 'Demographics', 'AmplifyReturn', 'ns1:AmplifyResponse'], + $this->tags($qp->parents()) + ); + } + + /** + * @throws Exception + */ + public function testParentsMatchesNamespacedAncestors() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(['ns1:AmplifyResponse'], $this->tags($qp->parents('ns1|AmplifyResponse'))); + $this->assertEquals(['ns1:AmplifyResponse'], $this->tags($qp->parents('*|AmplifyResponse'))); + + // The namespaced root must not be reported for a selector it only contains. + $this->assertEquals(0, $qp->parents('ns1|Demographics')->count()); + } + + /** + * @throws Exception + */ + public function testParentsAcceptsFullSelectorsWithCombinators() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(['Demographics'], $this->tags($qp->parents('AmplifyReturn > Demographics'))); + $this->assertEquals(0, $qp->parents('Styles > Demographics')->count()); + } + + /** + * jQuery returns ancestors closest-first, and for a set built from more than + * one element the result is in reverse document order with duplicates removed. + * + * @throws Exception + */ + public function testParentsReturnsReverseDocumentOrder() + { + $xml = ''; + $qp = qp($xml, 'i'); + + $this->assertEquals(2, $qp->count()); + $this->assertEquals(['d', 'c', 'b', 'a', 'root'], $this->tags($qp->parents())); + } + + /** + * Shared ancestors must appear exactly once. + * + * @throws Exception + */ + public function testParentsRemovesDuplicates() + { + $xml = ''; + $qp = qp($xml, 'i'); + + $this->assertEquals(2, $qp->count()); + $this->assertEquals(['a', 'root'], $this->tags($qp->parents())); + } + + /** + * @throws Exception + */ + public function testParentsUntilStopsAtTheMatchingAncestor() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(['Age'], $this->tags($qp->parentsUntil('Demographics'))); + + // Before the fix AmplifyReturn was collected, because AmplifyReturn does + // not contain a descendant called AmplifyReturn. + $this->assertEquals(['Age', 'Demographics'], $this->tags($qp->parentsUntil('AmplifyReturn'))); + $this->assertEquals( + ['Age', 'Demographics', 'AmplifyReturn'], + $this->tags($qp->parentsUntil('ns1|AmplifyResponse')) + ); + } + + /** + * @throws Exception + */ + public function testClosestMatchesTheAncestorItself() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(['Age'], $this->tags($qp->closest('Age'))); + $this->assertEquals(['Demographics'], $this->tags($qp->closest('Demographics'))); + + // Before the fix this returned ns1:AmplifyResponse, the first ancestor + // that contained an AmplifyReturn element. + $this->assertEquals(['AmplifyReturn'], $this->tags($qp->closest('AmplifyReturn'))); + $this->assertEquals(['Name'], $this->tags($qp->closest('Name'))); + } + + /** + * @throws Exception + */ + public function testParentMatchesTheAncestorItself() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(['Age'], $this->tags($qp->parent('Age'))); + $this->assertEquals(['AmplifyReturn'], $this->tags($qp->parent('AmplifyReturn'))); + $this->assertEquals(0, $qp->parent('Name')->count()); + } + + /** + * @throws Exception + */ + public function testSiblingTraversalMatchesTheSiblingItself() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(['Value'], $this->tags($qp->siblings('Value'))); + $this->assertEquals(['Value'], $this->tags($qp->nextAll('Value'))); + $this->assertEquals(['Value'], $this->tags($qp->next('Value'))); + $this->assertEquals(0, $qp->siblings('Name')->count()); + } + + /** + * `nextUntil()`/`prevUntil()` must stop on a sibling that matches, not on a + * sibling that contains a match. + * + * @throws Exception + */ + public function testNextUntilAndPrevUntilStopOnMatchingSibling() + { + $xml = ''; + + $this->assertEquals(['b'], $this->tags(qp($xml, 'root > a')->nextUntil('c'))); + $this->assertEquals([], $this->tags(qp($xml, 'root > a')->nextUntil('b'))); + $this->assertEquals(['c', 'b'], $this->tags(qp($xml, 'root > d')->prevUntil('a'))); + } + + /** + * `not()` must exclude elements that match the selector themselves. + * + * @throws Exception + */ + public function testNotExcludesElementsThatMatchTheSelector() + { + $xml = ''; + $qp = qp($xml, 'a, b'); + + $this->assertEquals(3, $qp->count()); + $this->assertEquals(['b', 'b'], $this->tags($qp->not('a'))); + $this->assertEquals(['a'], $this->tags($qp->not('b'))); + } + + /** + * The candidate node must not be passed to the traverser as its scope node, or every + * candidate matches :scope and the selector stops filtering anything at all. + * + * @see \QueryPath\Helpers\NodeMatcher + */ + public function testScopePseudoClassIsResolvedAgainstTheDocument(): void + { + $xml = 'x'; + + $this->assertSame('root', qp($xml, 'c')->top()->find(':scope')->tag()); + + $parents = qp($xml, 'c')->parents(':scope'); + $this->assertCount(1, $parents, ':scope must match the document element, not every ancestor'); + $this->assertSame('root', $parents->tag()); + + $this->assertTrue(qp($xml, 'root')->is(':scope')); + $this->assertFalse(qp($xml, 'c')->is(':scope')); + } +} diff --git a/tests/QueryPath/DOMQueryTest.php b/tests/QueryPath/DOMQueryTest.php index 4e4027f..921247c 100644 --- a/tests/QueryPath/DOMQueryTest.php +++ b/tests/QueryPath/DOMQueryTest.php @@ -560,9 +560,54 @@ public function testIndex() public function testFilter() { $file = DATA_FILE; - $this->assertEquals(1, qp($file)->filter('li')->count()); - $this->assertEquals(2, qp($file, 'inner')->filter('li')->count()); - $this->assertEquals('inner-two', qp($file, 'inner')->filter('li')->eq(1)->attr('id')); + + // filter() narrows the current match set to the members that match the selector. + // It does not search their descendants: and are not
  • elements, + // so an 'li' filter removes them both. + $this->assertEquals(0, qp($file)->filter('li')->count()); + $this->assertEquals(0, qp($file, 'inner')->filter('li')->count()); + + $this->assertEquals(2, qp($file, 'inner')->filter('inner')->count()); + $this->assertEquals(5, qp($file, 'li')->filter('li')->count()); + + $this->assertEquals(1, qp($file, 'inner')->filter('#inner-two')->count()); + $this->assertEquals('inner-two', qp($file, 'inner')->filter('#inner-two')->attr('id')); + + // The original set's ordering is preserved. + $this->assertEquals('inner-one', qp($file, 'inner')->filter('inner')->eq(0)->attr('id')); + $this->assertEquals('inner-two', qp($file, 'inner')->filter('inner')->eq(1)->attr('id')); + } + + /** + * filter() used to keep any element that *contained* a match, which is what has() + * does. This pins the difference, and documents the migration path for anyone who + * was relying on the old behaviour. + */ + public function testFilterMatchesTheSetRatherThanItsDescendants() + { + $file = DATA_FILE; + + $this->assertEquals(0, qp($file, 'inner')->filter('li')->count()); + $this->assertEquals(2, qp($file, 'inner')->has('li')->count()); + + $this->assertEquals(0, qp($file)->filter('li')->count()); + $this->assertEquals(1, qp($file)->has('li')->count()); + } + + /** + * A selector that describes a position within the match set has to be evaluated + * against the whole set, not against each member in turn. + */ + public function testFilterEvaluatesTheSelectorAgainstTheWholeSet() + { + $file = DATA_FILE; + + $this->assertEquals(5, qp($file, 'li')->filter('li')->count()); + $this->assertEquals('one', qp($file, 'li')->filter('#one')->attr('id')); + + // :scope refers to the document element, exactly as it does in find(). + $this->assertEquals(0, qp($file, 'inner')->filter(':scope')->count()); + $this->assertEquals(1, qp($file, 'root')->filter(':scope')->count()); } public function testFilterPreg()