From 419525f06edf427c30f589cc7cdcbe82fbe945b9 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 13:23:48 +1100 Subject: [PATCH 1/2] Select with DOMTraverser in remove() and replaceAll() remove($selector) and replaceAll($selector) were the last callers of the legacy QueryPathEventHandler engine, which matched the nodes in hand, did not know :matches(), and evaluated a positional :not() argument one element at a time. They now select what find() does. Deprecate QueryPathEventHandler, which QueryPath no longer uses. Fixes #126, #123, #124. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 +- src/CSS/QueryPathEventHandler.php | 1 + src/DOMQuery.php | 9 +- src/Helpers/QueryMutators.php | 15 +-- tests/Issues/Issue126Test.php | 160 ++++++++++++++++++++++++++++++ tests/Issues/Issue49Test.php | 5 +- tests/Issues/Issue65Test.php | 2 +- tests/Issues/Issue66Test.php | 5 +- tests/QueryPath/DOMQueryTest.php | 7 +- 9 files changed, 175 insertions(+), 34 deletions(-) create mode 100644 tests/Issues/Issue126Test.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 63489f9..2ac9391 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ QueryPath Changelog - On PHP 8.0 and later, `filterLambda()` and `eachLambda()` now throw a `QueryPath\Exception` that names their replacements, `filterCallback()` and `each()`, instead of an `Error` about the undefined `create_function()` (issue #81). They still work on PHP 7.1–7.4 and will be removed in 5.0 - **Behaviour change.** Fix the jQuery positional pseudo-classes (`:eq()`, `:first`, `:last`, `:lt()`, `:gt()`, `:odd`, `:even`, and the `:nth()` alias of `:eq()`), which indexed an element's siblings instead of the matched set (issue #66). They are now applied, zero-indexed and in document order, to the set the selector matched, as in jQuery. Most visibly, `:eq(0)` now matches the first result instead of never matching. `:lt()`, `:gt()`, `:eq()` and `:nth()` are zero-indexed where they were previously one-indexed, `:odd`/`:even` now index the matched set rather than sibling position, and a negative index counts back from the end of the set (`:eq(-1)` is the last match). The CSS structural pseudo-classes (`:nth-child()`, `:nth-of-type()`, `:first-child`, `:last-child`, ...) are unchanged and still count siblings, one-indexed -- Make `QueryPath\CSS\QueryPathEventHandler`, the legacy engine still used by `remove()` and `replaceAll()`, agree with `QueryPath\CSS\DOMTraverser` on all of the positional pseudo-classes. `remove('li:first')` and `find('li:first')` now select the same element +- Make `QueryPath\CSS\QueryPathEventHandler`, the legacy selector engine, agree with `QueryPath\CSS\DOMTraverser` on the positional pseudo-classes, except in the argument of `:not()` - Support positional pseudo-classes on any simple selector in a chain, not just the last one: `ul:first li` selects the list items of the first `ul`, and each comma-separated group is filtered independently before the union is returned in document order - Support positional pseudo-classes as the argument to `:not()`, `:has()` and `:matches()`, so the common `li:not(:first)` idiom now excludes the first match rather than every match - `QueryPath\CSS\DOMTraverser\PseudoClass::elementMatches()` now throws `QueryPath\CSS\NotImplementedException` for the positional pseudo-classes. They cannot be answered for a node in isolation; use a `Traverser`, which applies them after traversal @@ -15,7 +15,7 @@ QueryPath Changelog - 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 -- **Breaking behaviour change:** `find()` now searches descendants only, as jQuery's `.find()` does. A node already in the match set is no longer a candidate for its own selector, so `qp($xml, '#a1')->find('a')`, `find('#a1')` and `find('.c')` no longer return `#a1` itself. The document element remains matchable, because `qp()` seeds the match set with it rather than with the document. Use `filter()` to match the nodes in hand. `detach($selector)`, which uses `find()`, follows suit. `remove($selector)` and `replaceAll($selector)` still use the legacy engine and still match the nodes in hand, so `qp($xml, 'p')->remove('p')` removes the `

` elements while `detach('p')` detaches nothing (#126) +- **Breaking behaviour change:** `find()` now searches descendants only, as jQuery's `.find()` does. A node already in the match set is no longer a candidate for its own selector, so `qp($xml, '#a1')->find('a')`, `find('#a1')` and `find('.c')` no longer return `#a1` itself. The document element remains matchable, because `qp()` seeds the match set with it rather than with the document. Use `filter()` to match the nodes in hand. `detach($selector)`, which uses `find()`, follows suit - 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 - `nextAll()` and `prevAll()` stop walking at the first sibling already collected from an earlier starting element, rather than re-walking to the end of the list. On a 4,000-item list `nextAll()` fell from about 430 ms to under 10 ms @@ -53,6 +53,7 @@ QueryPath Changelog - Fix selectors with two or more levels of nested parentheses, such as `li:not(:not(:nth-child(1)))` or `ul:has(li:not(:nth-child(1)))`, throwing "Recursion error detected" (issue #120). The scanner skipped the character after every nested argument, so the outer argument never saw its closing paren. This also stops `li:not(:nth-child(1)) b` from folding ` b` into the `:not()` argument, and lets quoted arguments contain parentheses, as in `:contains("a)b")` - 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 # 4.2.0 diff --git a/src/CSS/QueryPathEventHandler.php b/src/CSS/QueryPathEventHandler.php index bbb73c9..cf2d6d3 100644 --- a/src/CSS/QueryPathEventHandler.php +++ b/src/CSS/QueryPathEventHandler.php @@ -75,6 +75,7 @@ * $matches will be an array of zero or more DOMElement objects. * * @ingroup querypath_css + * @deprecated QueryPath no longer uses this engine. Use {@see DOMTraverser}, which backs find(). */ class QueryPathEventHandler implements EventHandler, Traverser { diff --git a/src/DOMQuery.php b/src/DOMQuery.php index 0e69f5d..4c9028b 100644 --- a/src/DOMQuery.php +++ b/src/DOMQuery.php @@ -17,7 +17,6 @@ use DOMNode; use DOMXPath; use QueryPath\CSS\DOMTraverser; -use QueryPath\CSS\QueryPathEventHandler; use Masterminds\HTML5; use QueryPath\Entities; use QueryPath\Exception; @@ -134,11 +133,9 @@ public function top($selector = null): Query * @throws CSS\ParseException * @see is() * @todo If a find() returns zero matches, then a subsequent find() will - * also return zero matches, even if that find has a selector like :root. - * The reason for this is that the {@link QueryPathEventHandler} does - * not set the root of the document tree if it cannot find any elements - * from which to determine what the root is. The workaround is to use - * {@link top()} to select the root element again. + * also return zero matches, even if that find has a selector like :root, + * because there is no element left from which to find the root. The + * workaround is to use {@link top()} to select the root element again. * @see filter() */ public function find($selector): Query diff --git a/src/Helpers/QueryMutators.php b/src/Helpers/QueryMutators.php index d923323..68dfda6 100644 --- a/src/Helpers/QueryMutators.php +++ b/src/Helpers/QueryMutators.php @@ -5,7 +5,6 @@ use DOMDocument; use DOMDocumentFragment; use QueryPath\CSS\ParseException; -use QueryPath\CSS\QueryPathEventHandler; use QueryPath\DOMQuery; use QueryPath\Exception; use QueryPath\Query; @@ -795,14 +794,7 @@ public function appendTo(DOMQuery $dest): Query */ public function remove($selector = null): Query { - if (! empty($selector)) { - // Do a non-destructive find. - $query = new QueryPathEventHandler($this->matches); - $query->find($selector); - $matches = $query->getMatches(); - } else { - $matches = $this->matches; - } + $matches = empty($selector) ? $this->matches : $this->find($selector)->matches; $found = new SplObjectStorage(); foreach ($matches as $item) { @@ -846,10 +838,7 @@ public function replaceAll($selector, DOMDocument $document): Query { $replacement = $this->matches->count() > 0 ? $this->getFirstMatch() : $this->document->createTextNode(''); - $c = new QueryPathEventHandler($document); - $c->find($selector); - $temp = $c->getMatches(); - foreach ($temp as $item) { + foreach (QueryPath::with($document, $selector, $this->options)->get() as $item) { $node = $replacement->cloneNode(); $node = $document->importNode($node); $item->parentNode->replaceChild($node, $item); diff --git a/tests/Issues/Issue126Test.php b/tests/Issues/Issue126Test.php new file mode 100644 index 0000000..a30a117 --- /dev/null +++ b/tests/Issues/Issue126Test.php @@ -0,0 +1,160 @@ +
    '; + + private const LIST_HTML = '
    '; + + private const NESTED_XML = '

    ax

    b

    c    '; + + /** + * The tables from #123 and #124, as find() answers them. + * + * @return array + */ + public function selectorProvider(): array + { + return [ + 'matches (#123)' => ['li:matches(.x)', ['b']], + 'not eq(0) (#124)' => ['li:not(:eq(0))', ['b', 'c']], + 'not first (#124)' => ['li:not(:first)', ['b', 'c']], + 'not last (#124)' => ['li:not(:last)', ['a', 'b']], + 'not odd (#124)' => ['li:not(:odd)', ['a', 'c']], + 'not class (#124)' => ['li:not(.x)', ['a', 'c']], + ]; + } + + /** + * Each selector from selectorProvider() against each factory's document. + * + * @return array + */ + public function factorySelectorProvider(): array + { + $cases = []; + foreach (['qp' => self::LIST_XML, 'htmlqp' => self::LIST_HTML, 'html5qp' => self::LIST_HTML] as $factory => $markup) { + foreach ($this->selectorProvider() as $name => $case) { + $cases[$factory . ': ' . $name] = array_merge([$factory, $markup], $case); + } + } + + return $cases; + } + + /** + * @dataProvider factorySelectorProvider + */ + public function testRemoveAgreesWithFind(string $factory, string $markup, string $selector, array $expected) + { + $this->assertSame($expected, $this->ids($factory($markup)->find($selector))); + + $qp = $factory($markup); + $this->assertSame($expected, $this->ids($qp->remove($selector))); + $this->assertSame(array_values(array_diff(['a', 'b', 'c'], $expected)), $this->ids($qp->find('li'))); + } + + /** + * @dataProvider factorySelectorProvider + */ + public function testReplaceAllAgreesWithFind(string $factory, string $markup, string $selector, array $expected) + { + $document = $factory($markup)->get(0)->ownerDocument; + qp('', 'z')->replaceAll($selector, $document); + + $this->assertSame(array_values(array_diff(['a', 'b', 'c'], $expected)), $this->ids(qp($document)->find('li'))); + $this->assertCount(count($expected), qp($document)->find('z')); + } + + public function testRemoveSearchesDescendantsOnly() + { + $this->assertSame(0, qp(self::NESTED_XML, 'p')->remove('p')->count()); + $this->assertSame(3, qp(self::NESTED_XML, 'p')->remove('*')->count()); + + $qp = qp(self::NESTED_XML, 'p'); + $this->assertSame(['a', 'b'], $this->text($qp->remove('i'))); + $this->assertSame(['x', 'c'], $this->text($qp->top()->find('b, i'))); + } + + public function testRemoveCanMatchTheDocumentElement() + { + $qp = qp(self::LIST_XML); + + $this->assertSame(1, $qp->remove('ul')->count()); + $this->assertNull($qp->get(0)->ownerDocument->documentElement); + } + + /** + * replaceAll() searches the whole target document, whatever the match set holds. + */ + public function testReplaceAllSearchesTheWholeDocument() + { + $document = qp(self::LIST_XML)->get(0)->ownerDocument; + qp('', 'z')->replaceAll('ul', $document); + + $this->assertSame('z', $document->documentElement->tagName); + + $document = qp(self::NESTED_XML)->get(0)->ownerDocument; + qp('', 'z')->replaceAll('p', $document); + + $this->assertSame(['z', 'z', 'i'], $this->tags(qp($document)->children())); + } + + /** + * @return string[] + */ + private function ids(DOMQuery $qp): array + { + $ids = []; + foreach ($qp->get() as $node) { + $ids[] = $node->getAttribute('id'); + } + + return $ids; + } + + /** + * @return string[] + */ + private function text(DOMQuery $qp): array + { + $text = []; + foreach ($qp->get() as $node) { + $text[] = $node->textContent; + } + + return $text; + } + + /** + * @return string[] + */ + private function tags(DOMQuery $qp): array + { + $tags = []; + foreach ($qp->get() as $node) { + $tags[] = $node->tagName; + } + + return $tags; + } +} diff --git a/tests/Issues/Issue49Test.php b/tests/Issues/Issue49Test.php index ad9f8bc..6385b1c 100644 --- a/tests/Issues/Issue49Test.php +++ b/tests/Issues/Issue49Test.php @@ -113,10 +113,9 @@ public function testTextSelectorMatchesTheInputItself(): void } /** - * remove() runs the selector through the legacy CSS engine, which must agree - * with find(). + * remove() must agree with find(). */ - public function testTextSelectorInTheLegacyEngine(): void + public function testTextSelectorThroughRemove(): void { $q = html5qp(self::INPUT_HTML, 'div'); diff --git a/tests/Issues/Issue65Test.php b/tests/Issues/Issue65Test.php index 2f51feb..1198ce8 100644 --- a/tests/Issues/Issue65Test.php +++ b/tests/Issues/Issue65Test.php @@ -289,7 +289,7 @@ public function testASecondQueryOverTheSameDocumentSerializesCorrectly($fromDocu } /** - * remove() runs the legacy selector engine and returns a new DOMQuery over the same document. + * remove() returns a new DOMQuery over the same document. */ public function testRemoveKeepsTheDocumentSerializingCorrectly() { diff --git a/tests/Issues/Issue66Test.php b/tests/Issues/Issue66Test.php index d5615d3..2c24e3c 100644 --- a/tests/Issues/Issue66Test.php +++ b/tests/Issues/Issue66Test.php @@ -63,15 +63,14 @@ public function testPositionalPseudoClassesIndexTheMatchedSet($selector, array $ } /** - * remove() and replaceAll() use the legacy QueryPathEventHandler engine. - * It has to give the same answer as find(). + * remove() has to give the same answer as find(). * * @dataProvider positionalProvider * * @param string $selector * @param array $expected */ - public function testLegacyEngineAgreesWithTheTraverser($selector, array $expected) + public function testRemoveAgreesWithFind($selector, array $expected) { $removed = $this->textOf(html5qp(self::HTML)->remove($selector)); sort($removed); diff --git a/tests/QueryPath/DOMQueryTest.php b/tests/QueryPath/DOMQueryTest.php index f65c8e4..2e78b58 100644 --- a/tests/QueryPath/DOMQueryTest.php +++ b/tests/QueryPath/DOMQueryTest.php @@ -2375,9 +2375,7 @@ public function testDetachWithSelectorAgreesWithRemove() { $xml = '

    ax

    b

    c    '; - // remove() still lets a selector match the nodes in hand, while detach() searches - // descendants only, as find() does, so selectors such as 'p' and '*' disagree (#126). - foreach (['i', 'p > i', 'noSuchElement'] as $selector) { + foreach (['i', 'p > i', 'noSuchElement', 'p', '*'] as $selector) { $removeQp = qp($xml, 'p'); $removed = $removeQp->remove($selector); $detachQp = qp($xml, 'p'); @@ -2386,9 +2384,6 @@ public function testDetachWithSelectorAgreesWithRemove() $this->assertSame($removed->count(), $detached->count(), $selector); $this->assertSame($removeQp->top()->xml(), $detachQp->top()->xml(), $selector); } - - $this->assertSame(0, qp($xml, 'p')->detach('p')->count()); - $this->assertSame(3, qp($xml, 'p')->detach('*')->count()); } public function testAttachAfterDetachWithSelector() From ba44e042e68d5d35075cc6fce57f1aab2dfb7a5b Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 13:26:57 +1100 Subject: [PATCH 2/2] Call DOMTraverser directly in remove() and replaceAll() find() deep-clones the match set through inst(), and QueryPath::with() builds a whole DOMQuery, only for the matches to be read and discarded. Co-Authored-By: Claude Opus 5.5 --- src/Helpers/QueryMutators.php | 12 ++++- src/qp_functions.php | 2 +- tests/Issues/Issue126Test.php | 83 ++++++++++++----------------------- tests/Issues/Issue66Test.php | 8 +--- 4 files changed, 41 insertions(+), 64 deletions(-) diff --git a/src/Helpers/QueryMutators.php b/src/Helpers/QueryMutators.php index 68dfda6..93ab6b6 100644 --- a/src/Helpers/QueryMutators.php +++ b/src/Helpers/QueryMutators.php @@ -4,6 +4,7 @@ use DOMDocument; use DOMDocumentFragment; +use QueryPath\CSS\DOMTraverser; use QueryPath\CSS\ParseException; use QueryPath\DOMQuery; use QueryPath\Exception; @@ -794,7 +795,8 @@ public function appendTo(DOMQuery $dest): Query */ public function remove($selector = null): Query { - $matches = empty($selector) ? $this->matches : $this->find($selector)->matches; + // Not find(), which deep-clones the match set via inst(). + $matches = empty($selector) ? $this->matches : (new DOMTraverser($this->matches))->find($selector)->matches(); $found = new SplObjectStorage(); foreach ($matches as $item) { @@ -838,7 +840,13 @@ public function replaceAll($selector, DOMDocument $document): Query { $replacement = $this->matches->count() > 0 ? $this->getFirstMatch() : $this->document->createTextNode(''); - foreach (QueryPath::with($document, $selector, $this->options)->get() as $item) { + // 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); + } + + foreach ((new DOMTraverser($scope))->find($selector)->matches() as $item) { $node = $replacement->cloneNode(); $node = $document->importNode($node); $item->parentNode->replaceChild($node, $item); diff --git a/src/qp_functions.php b/src/qp_functions.php index 6ae202d..d6c808f 100644 --- a/src/qp_functions.php +++ b/src/qp_functions.php @@ -40,7 +40,7 @@ * The core classes that compose QueryPath. * * The QueryPath classes contain the brunt of the QueryPath code. If you are - * interested in working with just the CSS engine, you may want to look at CssEventHandler, + * interested in working with just the CSS engine, you may want to look at QueryPath\CSS\DOMTraverser, * which can be used without the rest of QueryPath. If you are interested in looking * carefully at QueryPath's implementation details, then the QueryPath class is where you * should begin. If you are interested in writing extensions, than you may want to look at diff --git a/tests/Issues/Issue126Test.php b/tests/Issues/Issue126Test.php index a30a117..f9e3337 100644 --- a/tests/Issues/Issue126Test.php +++ b/tests/Issues/Issue126Test.php @@ -28,33 +28,26 @@ class Issue126Test extends TestCase private const NESTED_XML = '

    ax

    b

    c    '; /** - * The tables from #123 and #124, as find() answers them. + * The tables from #123 and #124 against each factory's document: the selector, what + * find() matches, and the list items left behind. * * @return array */ public function selectorProvider(): array { - return [ - 'matches (#123)' => ['li:matches(.x)', ['b']], - 'not eq(0) (#124)' => ['li:not(:eq(0))', ['b', 'c']], - 'not first (#124)' => ['li:not(:first)', ['b', 'c']], - 'not last (#124)' => ['li:not(:last)', ['a', 'b']], - 'not odd (#124)' => ['li:not(:odd)', ['a', 'c']], - 'not class (#124)' => ['li:not(.x)', ['a', 'c']], + $selectors = [ + 'li:matches(.x)' => [['b'], ['a', 'c']], + 'li:not(:eq(0))' => [['b', 'c'], ['a']], + 'li:not(:first)' => [['b', 'c'], ['a']], + 'li:not(:last)' => [['a', 'b'], ['c']], + 'li:not(:odd)' => [['a', 'c'], ['b']], + 'li:not(.x)' => [['a', 'c'], ['b']], ]; - } - /** - * Each selector from selectorProvider() against each factory's document. - * - * @return array - */ - public function factorySelectorProvider(): array - { $cases = []; foreach (['qp' => self::LIST_XML, 'htmlqp' => self::LIST_HTML, 'html5qp' => self::LIST_HTML] as $factory => $markup) { - foreach ($this->selectorProvider() as $name => $case) { - $cases[$factory . ': ' . $name] = array_merge([$factory, $markup], $case); + foreach ($selectors as $selector => list($matched, $left)) { + $cases[$factory . ' ' . $selector] = [$factory, $markup, $selector, $matched, $left]; } } @@ -62,27 +55,27 @@ public function factorySelectorProvider(): array } /** - * @dataProvider factorySelectorProvider + * @dataProvider selectorProvider */ - public function testRemoveAgreesWithFind(string $factory, string $markup, string $selector, array $expected) + public function testRemoveAgreesWithFind(string $factory, string $markup, string $selector, array $matched, array $left) { - $this->assertSame($expected, $this->ids($factory($markup)->find($selector))); + $this->assertSame($matched, $this->ids($factory($markup)->find($selector))); $qp = $factory($markup); - $this->assertSame($expected, $this->ids($qp->remove($selector))); - $this->assertSame(array_values(array_diff(['a', 'b', 'c'], $expected)), $this->ids($qp->find('li'))); + $this->assertSame($matched, $this->ids($qp->remove($selector))); + $this->assertSame($left, $this->ids($qp->find('li'))); } /** - * @dataProvider factorySelectorProvider + * @dataProvider selectorProvider */ - public function testReplaceAllAgreesWithFind(string $factory, string $markup, string $selector, array $expected) + public function testReplaceAllAgreesWithFind(string $factory, string $markup, string $selector, array $matched, array $left) { $document = $factory($markup)->get(0)->ownerDocument; qp('', 'z')->replaceAll($selector, $document); - $this->assertSame(array_values(array_diff(['a', 'b', 'c'], $expected)), $this->ids(qp($document)->find('li'))); - $this->assertCount(count($expected), qp($document)->find('z')); + $this->assertSame($left, $this->ids(qp($document)->find('li'))); + $this->assertCount(count($matched), qp($document)->find('z')); } public function testRemoveSearchesDescendantsOnly() @@ -119,42 +112,24 @@ public function testReplaceAllSearchesTheWholeDocument() $this->assertSame(['z', 'z', 'i'], $this->tags(qp($document)->children())); } - /** - * @return string[] - */ private function ids(DOMQuery $qp): array { - $ids = []; - foreach ($qp->get() as $node) { - $ids[] = $node->getAttribute('id'); - } - - return $ids; + return array_map(function ($node) { + return $node->getAttribute('id'); + }, $qp->get()); } - /** - * @return string[] - */ private function text(DOMQuery $qp): array { - $text = []; - foreach ($qp->get() as $node) { - $text[] = $node->textContent; - } - - return $text; + return array_map(function ($node) { + return $node->textContent; + }, $qp->get()); } - /** - * @return string[] - */ private function tags(DOMQuery $qp): array { - $tags = []; - foreach ($qp->get() as $node) { - $tags[] = $node->tagName; - } - - return $tags; + return array_map(function ($node) { + return $node->tagName; + }, $qp->get()); } } diff --git a/tests/Issues/Issue66Test.php b/tests/Issues/Issue66Test.php index 2c24e3c..ae20b5e 100644 --- a/tests/Issues/Issue66Test.php +++ b/tests/Issues/Issue66Test.php @@ -72,13 +72,7 @@ public function testPositionalPseudoClassesIndexTheMatchedSet($selector, array $ */ public function testRemoveAgreesWithFind($selector, array $expected) { - $removed = $this->textOf(html5qp(self::HTML)->remove($selector)); - sort($removed); - - $sorted = $expected; - sort($sorted); - - $this->assertSame($sorted, $removed); + $this->assertSame($expected, $this->textOf(html5qp(self::HTML)->remove($selector))); } /**