Make find() search descendants only, as jQuery does

CHANGELOG.md

@@ -9,6 +9,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 `<inner>` 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
 - 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`

src/CSS/DOMTraverser.php

@@ -476,7 +476,7 @@ protected function initialMatchOnID(SimpleSelector $selector, SplObjectStorage $
 		// Now we try to find any matching IDs.
 		/** @var DOMElement $node */
 		foreach ($matches as $node) {
-			if ($node->getAttribute('id') === $id) {
+			if ($this->isDocumentElement($node) && $node->getAttribute('id') === $id) {
 				$found->offsetSet($node);
 			}
 
@@ -521,7 +521,7 @@ protected function initialMatchOnClasses(SimpleSelector $selector, SplObjectStor
 		/** @var DOMElement $node */
 		foreach ($matches as $node) {
 			// Refactor me!
-			if ($node->hasAttribute('class')) {
+			if ($this->isDocumentElement($node) && $node->hasAttribute('class')) {
 				$intersect = array_intersect($selector->classes, explode(' ', $node->getAttribute('class')));
 				if (count($intersect) === count($selector->classes)) {
 					$found->offsetSet($node);
@@ -578,6 +578,26 @@ private function initialXpathQuery(DOMXPath $xpath, DOMElement $node, string $qu
 	 *
 	 * @return SplObjectStorage
 	 */
+	/**
+	 * Is this node the element the match set was seeded with?
+	 *
+	 * jQuery's find() searches descendants only, so a node already in the match set is not
+	 * a candidate for its own selector. The document element is the one exception: QueryPath
+	 * seeds the match set with it rather than with the document, so it stands in for the
+	 * document and qp($xml)->find('root') has to keep matching.
+	 *
+	 * All three initial matchers ask this, and they have to agree — otherwise find('#a1')
+	 * would match a node that find('a') does not.
+	 *
+	 * @param DOMNode $node
+	 *
+	 * @return bool
+	 */
+	private function isDocumentElement($node): bool
+	{
+		return $node->parentNode instanceof DOMDocument;
+	}
+
 	protected function initialMatchOnElement(SimpleSelector $selector, SplObjectStorage $matches): SplObjectStorage
 	{
 		$element = $selector->element;
@@ -587,9 +607,8 @@ protected function initialMatchOnElement(SimpleSelector $selector, SplObjectStor
 		$found = $this->newMatches();
 		/** @var DOMDocument $node */
 		foreach ($matches as $node) {
-			// Capture the case where the initial element is the root element.
-			if ($node->tagName === $element
-				|| ($element === '*' && $node->parentNode instanceof DOMDocument)) {
+			if ($this->isDocumentElement($node)
+				&& ($element === '*' || $node->tagName === $element)) {
 				$found->offsetSet($node);
 			}
 			$nl = $node->getElementsByTagName($element);

tests/QueryPath/DOMQueryTest.php

@@ -918,7 +918,9 @@ public function testPrependTo()
 	public function testBefore()
 	{
 		$file = DATA_FILE;
-		$this->assertEquals(1, qp($file, 'unary')->before('<test/>')->find(':root > test ~ unary')->count());
+		// find() searches descendants, so the search starts from the top of the document —
+		// the same shape as the top() calls below.
+		$this->assertEquals(1, qp($file, 'unary')->before('<test/>')->top()->find(':root > test ~ unary')->count());
 		$this->assertEquals(1, qp($file, 'unary')->before('<test/>')->top('head ~ test')->count());
 		$this->assertEquals(
 			'unary',

tests/QueryPath/FindDescendantOnlyTest.php

@@ -0,0 +1,81 @@
+<?php
+
+namespace QueryPathTests;
+
+/**
+ * find() searches descendants, as jQuery's find() does.
+ *
+ * A node already in the match set is not a candidate for its own selector. The document
+ * element is the one exception, because QueryPath seeds the match set with the document
+ * element rather than with the document node.
+ */
+class FindDescendantOnlyTest extends TestCase
+{
+
+	const XML = '<?xml version="1.0"?><root><a id="a1"><a id="a2"/><b id="b1"/></a></root>';
+
+	public function testFindDoesNotMatchTheNodesItStartsFrom(): void
+	{
+		$a1 = qp(self::XML, '#a1');
+		$this->assertSame('a1', $a1->attr('id'));
+
+		// <a id="a1"> is in the match set, so it is not a candidate: only the nested
+		// <a id="a2"> is found.
+		$found = $a1->find('a');
+		$this->assertCount(1, $found);
+		$this->assertSame('a2', $found->attr('id'));
+	}
+
+	public function testWildcardDoesNotMatchTheNodesItStartsFrom(): void
+	{
+		$ids = [];
+		foreach (qp(self::XML, '#a1')->find('*') as $node) {
+			$ids[] = $node->attr('id');
+		}
+
+		$this->assertSame(['a2', 'b1'], $ids);
+	}
+
+	public function testTheDocumentElementIsStillReachable(): void
+	{
+		// qp() seeds the match set with the document element, which stands in for the
+		// document, so a selector naming it has to keep working.
+		$this->assertCount(1, qp(self::XML)->find('root'));
+		$this->assertSame('root', qp(self::XML)->find('root')->tag());
+
+		$this->assertCount(1, qp(self::XML)->find(':root'));
+	}
+
+	public function testDescendantsOfTheDocumentElementAreStillFound(): void
+	{
+		$this->assertCount(2, qp(self::XML)->find('a'));
+		$this->assertCount(1, qp(self::XML)->find('b'));
+	}
+
+	/**
+	 * filter() is the jQuery-equivalent way to ask whether the nodes in hand match.
+	 */
+	public function testFilterIsTheWayToMatchTheNodesInHand(): void
+	{
+		$this->assertCount(1, qp(self::XML, '#a1')->filter('a'));
+		$this->assertSame('a1', qp(self::XML, '#a1')->filter('a')->attr('id'));
+	}
+
+	/**
+	 * The element, ID and class initial matchers each have their own self-test, so all three
+	 * have to agree — otherwise find('#a1') would match a node that find('a') does not.
+	 */
+	public function testIdAndClassSelectorsAreDescendantOnlyToo(): void
+	{
+		$xml = '<?xml version="1.0"?><root><a id="a1" class="c"><a id="a2" class="c"/></a></root>';
+
+		$this->assertCount(0, qp($xml, '#a1')->find('#a1'));
+
+		$found = qp($xml, '#a1')->find('.c');
+		$this->assertCount(1, $found);
+		$this->assertSame('a2', $found->attr('id'));
+
+		// The document element keeps its exception here as well.
+		$this->assertCount(1, qp($xml)->find('#a1'));
+	}
+}