Make is() test the current match set, like jQuery

CHANGELOG.md

@@ -3,6 +3,8 @@ 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('<p><span>foo</span></p>', '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
 - 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()`

src/Helpers/NodeMatcher.php

@@ -0,0 +1,99 @@
+<?php
+
+namespace QueryPath\Helpers;
+
+use DOMElement;
+use QueryPath\CSS\DOMTraverser;
+use QueryPath\CSS\ParseException;
+use SplObjectStorage;
+
+/**
+ * Test nodes that are already in hand against a CSS selector.
+ *
+ * This is deliberately different from running a find(): find() searches the
+ * <em>descendants</em> of the nodes it is given, so for `<a><b/></a>` 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);
+
+		return $traverser->matches();
+	}
+
+	/**
+	 * Test whether at least one of the given nodes matches a selector.
+	 *
+	 * @param SplObjectStorage $nodes
+	 * @param string           $selector
+	 *
+	 * @return bool
+	 * @throws ParseException
+	 */
+	public static function matchesAny(SplObjectStorage $nodes, $selector): bool
+	{
+		return count(self::filter($nodes, $selector)) > 0;
+	}
+
+	/**
+	 * 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 self::matchesAny($nodes, $selector);
+	}
+}

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 <em>in</em> 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 <em>contains</em> 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 NodeMatcher::matchesAny($this->matches, $selector);
 	}
 
 	/**

tests/Issues/Issue51Test.php

@@ -0,0 +1,154 @@
+<?php
+/**
+ * @file
+ *
+ * Regression tests for https://github.com/GravityPDF/querypath/issues/51
+ *
+ * is() used to run a descendant search, which meant it returned TRUE whenever anything
+ * *below* the current match set matched the selector. It now behaves like jQuery's is():
+ * it tests the elements held in the match set and nothing else.
+ */
+
+namespace QueryPathTests;
+
+use SplDoublyLinkedList;
+
+class Issue51Test extends TestCase
+{
+
+	/**
+	 * The bug as reported: a descendant of an element in the collection made is() return TRUE.
+	 */
+	public function testIsDoesNotMatchDescendants(): void
+	{
+		$dom = html5qp('<p><span>foo</span></p>', '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
+	 * <p> nor the <span> is in the collection, so neither may match.
+	 */
+	public function testIsDoesNotMatchDeeperDescendants(): void
+	{
+		$dom = html5qp('<p><span>foo</span></p>');
+
+		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('<div id="outer"><p><span>foo</span></p></div>', '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('<ul><li id="one">1</li><li class="two">2</li><li>3</li></ul>', '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('<div><p id="pp" class="one two">foo</p></div>', '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('<div><p>foo</p></div>', '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('<div>Sample<!-- a comment --></div>', '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('<ul><li id="one">1</li><li id="two">2</li></ul>');
+		$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('<ul><li id="one">1</li><li id="two">2</li></ul>');
+
+		$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('<p><span>foo</span></p>', '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('<div id="outer"><section><p id="target">foo</p></section></div>');
+
+		self::assertCount(1, $dom->top('#target')->parents('div'));
+		self::assertSame('outer', $dom->top('#target')->parents('div')->attr('id'));
+	}
+}