Count and keep every node in even(), odd(), first() and last()

CHANGELOG.md

@@ -59,6 +59,7 @@ QueryPath Changelog
 - **Behaviour change.** `prevAll()`, `prevUntil()`, `nextAll()`, `nextUntil()`, `siblings()`, `parent()` and `closest()` now return the results for a multi-element set in jQuery's order, instead of grouped by starting element (issue #117). `prevAll()` and `prevUntil()` return reverse document order, as `parents()` and `parentsUntil()` already did, and the others return document order. `qp($xml, 'c')->add('d')->prevAll()` now returns `c, b, a` rather than `b, a, c`. `next()` and `prev()` keep the order of the starting elements, as in jQuery, and the results for a single starting element are unchanged
 - **Behaviour change.** A positional pseudo-class (`:first`, `:eq()`, `:last`, ...) in the stop selector of `nextUntil()`, `prevUntil()` and `parentsUntil()` now counts across the whole document, as jQuery's does, instead of seeing each sibling or ancestor as a one-element set (issue #118). `nextUntil(':first')` used to stop at once and now walks to the end, and `nextUntil(':eq(3)')` stops at the fourth element in the document. `closest()` no longer matches a positional selector at all, as in jQuery, where `closest(':first')` used to return the element itself. `next()`, `prev()` and `parent()` still test each candidate on its own, which agrees with jQuery for a single starting element
 - **Breaking behaviour change.** `even()` and `odd()` now count from zero, as jQuery's `.even()` and `.odd()` and QueryPath's own `:even` and `:odd` pseudo-classes do (issue #145). `even()` keeps the 1st, 3rd, 5th, ... matches and `odd()` keeps the 2nd, 4th, ..., the reverse of before. Swap the calls to keep the previous result
+- **Behaviour change.** `even()`, `odd()`, `first()` and `last()` now count and return every node in the match set, including text, comment, CDATA and processing-instruction nodes, as jQuery does (issues #143, #146). `even()` and `odd()` used to count those nodes but leave them out of the result, so `contents()->odd()` could be empty, and `first()` and `last()` skipped them, so `first()` could disagree with `eq(0)`. `first()` is now the same as `eq(0)`, and `last()` returns the last node of any type. To work on elements only, use `children()->first()` or `filter('*')->first()`, and likewise for `last()`, `even()` and `odd()`
 
 # 4.2.0
 

src/Helpers/QueryFilters.php

@@ -374,7 +374,8 @@ public function eachLambda($lambda): Query
 
 	/**
 	 * Get the matches at zero-based even positions (0, 2, 4, ...), as jQuery's .even() and
-	 * the :even pseudo-class do.
+	 * the :even pseudo-class do. Every node counts, whatever its type; use filter('*')->even()
+	 * to count only the elements.
 	 *
 	 * @return DOMQuery
 	 *  A DOMQuery wrapping the matches at even positions.
@@ -390,7 +391,8 @@ public function even(): Query
 
 	/**
 	 * Get the matches at zero-based odd positions (1, 3, 5, ...), as jQuery's .odd() and
-	 * the :odd pseudo-class do.
+	 * the :odd pseudo-class do. Every node counts, whatever its type; use filter('*')->odd()
+	 * to count only the elements.
 	 *
 	 * @return DOMQuery
 	 *  A DOMQuery wrapping the matches at odd positions.
@@ -416,35 +418,30 @@ private function positional(string $name): Query
 	{
 		$found = new SplObjectStorage();
 		foreach (Util::applyPositionalPseudoClass(iterator_to_array($this->matches, false), $name) as $m) {
-			if ($m->nodeType === XML_ELEMENT_NODE) {
-				$found->offsetSet($m);
-			}
+			$found->offsetSet($m);
 		}
 
 		return $this->inst($found, null);
 	}
 
 	/**
-	 * Get the first matching element.
+	 * Get the first match.
 	 *
+	 * As in jQuery, this is the same as eq(0): the first node of any type, so
+	 * after contents() it may be a text node. Use children()->first() or
+	 * filter('*')->first() for the first element.
 	 *
 	 * @return DOMQuery
-	 *  A DOMQuery wrapping all of the children.
+	 *  A DOMQuery wrapping the first match, or an empty one.
 	 * @throws ParseException
-	 * @see    prev()
+	 * @see    last()
+	 * @see    eq()
 	 * @since  2.1
 	 * @author eabrand
-	 * @see    next()
 	 */
 	public function first(): Query
 	{
-		$found = new SplObjectStorage();
-		foreach ($this->elements() as $m) {
-			$found->offsetSet($m);
-			break;
-		}
-
-		return $this->inst($found, null);
+		return $this->eq(0);
 	}
 
 	/**
@@ -475,29 +472,23 @@ public function firstChild(): Query
 	}
 
 	/**
-	 * Get the last matching element.
+	 * Get the last match.
 	 *
+	 * As in jQuery, this is the last node of any type, so after contents() it
+	 * may be a text node. Use children()->last() or filter('*')->last() for
+	 * the last element.
 	 *
 	 * @return DOMQuery
-	 *  A DOMQuery wrapping all of the children.
+	 *  A DOMQuery wrapping the last match, or an empty one.
 	 * @throws ParseException
-	 * @see    prev()
+	 * @see    first()
+	 * @see    eq()
 	 * @since  2.1
 	 * @author eabrand
-	 * @see    next()
 	 */
 	public function last(): Query
 	{
-		$found = new SplObjectStorage();
-		$item  = null;
-		foreach ($this->elements() as $m) {
-			$item = $m;
-		}
-		if ($item) {
-			$found->offsetSet($item);
-		}
-
-		return $this->inst($found, null);
+		return $this->eq($this->count() - 1);
 	}
 
 	/**

tests/Issues/Issue143Test.php

@@ -0,0 +1,78 @@
+<?php
+/**
+ * @file
+ *
+ * Regression tests for https://github.com/GravityPDF/querypath/issues/143
+ * and https://github.com/GravityPDF/querypath/issues/146
+ *
+ * even(), odd(), first() and last() count and keep every node in the set, not
+ * just the elements. Expected values are jQuery 3.7.1's.
+ */
+
+namespace QueryPathTests;
+
+class Issue143Test extends TestCase
+{
+	// contents(): "x", a, "y", b, "z".
+	private const XML = '<?xml version="1.0"?><r>x<i id="a"/>y<i id="b"/>z</r>';
+
+	public function testEvenCountsAndKeepsEveryNode()
+	{
+		$this->assertSame(['x', 'y', 'z'], $this->describe($this->contents()->even()));
+	}
+
+	public function testOddCountsAndKeepsEveryNode()
+	{
+		$this->assertSame(['a', 'b'], $this->describe($this->contents()->odd()));
+	}
+
+	public function testFirstIsTheFirstNodeOfAnyType()
+	{
+		$c = $this->contents();
+
+		$this->assertSame(['x'], $this->describe($c->first()));
+	}
+
+	public function testLastIsTheLastNodeOfAnyType()
+	{
+		$c = $this->contents();
+
+		$this->assertSame(['z'], $this->describe($c->last()));
+	}
+
+	public function testOnlyElementsAfterFilteringOrChildren()
+	{
+		$c = $this->contents();
+
+		$this->assertSame(['a'], $this->describe($c->filter('*')->first()));
+		$this->assertSame(['b'], $this->describe($c->filter('*')->last()));
+		$this->assertSame(['a'], $this->describe($c->filter('*')->even()));
+		$this->assertSame(['b'], $this->describe($c->filter('*')->odd()));
+		$this->assertSame(['a'], $this->describe(qp(self::XML, 'r')->children()->first()));
+	}
+
+	public function testEmptySet()
+	{
+		$qp = qp(self::XML, 'nothing');
+
+		$this->assertSame(0, $qp->first()->count());
+		$this->assertSame(0, $qp->last()->count());
+		$this->assertSame(0, $qp->even()->count());
+		$this->assertSame(0, $qp->odd()->count());
+	}
+
+	private function contents()
+	{
+		return qp(self::XML, 'r')->contents();
+	}
+
+	/**
+	 * Each node's ID if it is an element, or its text otherwise.
+	 */
+	private function describe($query): array
+	{
+		return array_map(function ($node) {
+			return $node instanceof \DOMElement ? $node->getAttribute('id') : $node->textContent;
+		}, $query->get());
+	}
+}

tests/QueryPath/DOMQueryTest.php

@@ -756,8 +756,10 @@ public function testFirstAndLastWithNonElementNodes($xml)
 	{
 		[$qp, $a] = $this->contentsWithElement($xml);
 
-		$this->assertSame([$a], $qp->first()->get());
-		$this->assertSame([$a], $qp->last()->get());
+		$this->assertSame([$qp->get(0)], $qp->first()->get());
+		$this->assertSame([$qp->get($qp->count() - 1)], $qp->last()->get());
+		$this->assertSame([$a], $qp->filter('*')->first()->get());
+		$this->assertSame([$a], $qp->filter('*')->last()->get());
 	}
 
 	public function testEq()