Count negative indexes from the end in eq(), get() and slice(), and take an end index in slice()

CHANGELOG.md

@@ -58,6 +58,8 @@ QueryPath Changelog
 - Consolidate the sibling and ancestor walks behind `next()`, `prev()`, `nextAll()`, `prevAll()`, `nextUntil()`, `prevUntil()`, `parent()`, `parents()`, `parentsUntil()` and `closest()` into one helper, so a fix to one no longer has to be repeated in its mirror (issue #115). As a side effect, `parentsUntil()` no longer loops forever on an element that is not attached to the document, and `nextUntil('')` and `prevUntil('')` treat an empty selector as no selector, as the other methods already did, instead of throwing `QueryPath\CSS\ParseException`. `closest('')` returns an empty set, as in jQuery. `nextUntil()`, `prevUntil()` and `parentsUntil()` also stop at an element already collected from an earlier starting element, as `nextAll()` and `prevAll()` do, so `nextUntil()` on each of 2000 siblings falls from about 100 ms to 1 ms
 - **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
+- **Behaviour change.** `eq()`, `get()` and `slice()` now count a negative index back from the end of the match set, as jQuery does (issue #142). `eq(-1)` and `get(-1)` are the last match, where they used to give an empty set and `NULL`. `slice(-2)` now returns the last two matches instead of the whole set. An index that is still out of range gives an empty set, or `NULL` from `get()`
+- **Breaking behaviour change.** `slice($start, $end)` now takes an exclusive end index as its second argument, as jQuery's `.slice()` does, instead of a length (issue #144). `slice(1, 3)` returns the second and third matches, where it used to return three matches. A negative `$end` counts back from the end, so `slice(1, -1)` drops the first and last matches. Leaving out `$end` still slices to the end of the set, but an explicit `0`, which used to mean "to the end", now gives an empty set. To keep a length, use `slice($start, $start + $length)`
 
 # 4.2.0
 

src/CSS/DOMTraverser/Util.php

@@ -246,14 +246,14 @@ public static function sortDocumentOrder(array $nodes): array
 	}
 
 	/**
-	 * Resolve a (possibly negative) positional index against a set size.
+	 * Resolve a zero-based index against a set size, counting a negative one back from the end.
 	 *
 	 * @param mixed $value
 	 * @param int   $count
 	 *
 	 * @return int
 	 */
-	private static function normalizePositionalIndex($value, int $count): int
+	public static function normalizePositionalIndex($value, int $count): int
 	{
 		$index = (int) $value;
 

src/DOM.php

@@ -10,6 +10,7 @@
 use IteratorAggregate;
 use Masterminds\HTML5;
 use QueryPath\CSS\DOMTraverser;
+use QueryPath\CSS\DOMTraverser\Util;
 use QueryPath\Entities;
 use SimpleXMLElement;
 use SplObjectStorage;
@@ -621,14 +622,18 @@ protected function isXMLish($string)
 	 * The internal data structure used in DOMQuery does not have
 	 * strong random access support, so we suppliment it with this method.
 	 *
-	 * @param $index
+	 * @param int $index
+	 *  Zero-based. A negative index counts back from the end of the matches.
 	 *
-	 * @return object|void
+	 * @return object|null
+	 *  NULL if the index is out of range.
 	 */
 	protected function getNthMatch(int $index)
 	{
-		if ($index < 0 || $index > $this->matches->count()) {
-			return;
+		$count = $this->matches->count();
+		$index = Util::normalizePositionalIndex($index, $count);
+		if ($index < 0 || $index >= $count) {
+			return null;
 		}
 
 		$i = 0;

src/DOMQuery.php

@@ -266,7 +266,8 @@ public function count(): int
 	 * in QueryPath, see {@link examples/techniques.php}.
 	 *
 	 * @param int|null $index
-	 *   If specified, then only this index value will be returned. If this
+	 *   If specified, then only this index value will be returned. A negative
+	 *   index counts back from the end, so -1 is the last match. If this
 	 *   index is out of bounds, a NULL will be returned.
 	 * @param boolean $asObject
 	 *   If this is TRUE, an SplObjectStorage object will be returned
@@ -281,7 +282,7 @@ public function count(): int
 	public function get($index = null, $asObject = false)
 	{
 		if ($index !== null) {
-			return ($this->count() > $index) ? $this->getNthMatch($index) : null;
+			return $this->getNthMatch($index);
 		}
 		// Retain support for legacy.
 		if (! $asObject) {

src/Helpers/QueryFilters.php

@@ -262,35 +262,30 @@ public function map($callback): Query
 	/**
 	 * Narrow the items in this object down to only a slice of the starting items.
 	 *
-	 * @param integer $start
-	 *  Where in the list of matches to begin the slice.
-	 * @param integer $length
-	 *  The number of items to include in the slice. If nothing is specified, the
-	 *  all remaining matches (from $start onward) will be included in the sliced
-	 *  list.
+	 * As in jQuery's .slice(), the slice runs from $start up to, but not
+	 * including, $end. Both are zero-based indexes, and a negative index counts
+	 * back from the end, so slice(1, -1) drops the first and last items.
+	 *
+	 * @param integer      $start
+	 *  The index of the first item to include.
+	 * @param integer|null $end
+	 *  The index to stop before. If omitted, the slice runs to the end of the
+	 *  matches. An $end at or before $start gives an empty set.
 	 *
 	 * @return DOMQuery
 	 * @throws ParseException
-	 * @see array_slice()
 	 */
-	public function slice($start, $length = 0): Query
+	public function slice($start, $end = null): Query
 	{
-		$end   = $length;
-		$found = new SplObjectStorage();
-		if ($start >= $this->count()) {
-			return $this->inst($found, null);
-		}
+		$count = $this->count();
+		$start = max(0, Util::normalizePositionalIndex($start, $count));
+		$end   = $end === null ? $count : Util::normalizePositionalIndex($end, $count);
 
-		$i = $j = 0;
-		foreach ($this->matches as $m) {
-			if ($i >= $start) {
-				if ($end > 0 && $j >= $end) {
-					break;
-				}
+		$found = new SplObjectStorage();
+		if ($start < $end) {
+			foreach (array_slice(iterator_to_array($this->matches, false), $start, $end - $start) as $m) {
 				$found->offsetSet($m);
-				++$j;
 			}
-			++$i;
 		}
 
 		return $this->inst($found, null);
@@ -620,8 +615,9 @@ public function parentsUntil($selector = null): Query
 	 * with {@link end()}.
 	 *
 	 * @param $index
-	 *  The index of the element to keep. The rest will be
-	 *  discarded.
+	 *  The zero-based index of the item to keep. The rest will be
+	 *  discarded. A negative index counts back from the end, so -1 keeps
+	 *  the last item. An index out of range gives an empty set.
 	 *
 	 * @return DOMQuery
 	 * @throws ParseException

tests/Issues/Issue142Test.php

@@ -0,0 +1,48 @@
+<?php
+/**
+ * @file
+ *
+ * Regression tests for https://github.com/GravityPDF/querypath/issues/142
+ *
+ * Negative indexes in eq(), get() and slice(). Expected values are jQuery 3.7.1's.
+ */
+
+namespace QueryPathTests;
+
+class Issue142Test extends TestCase
+{
+	private const XML = '<?xml version="1.0"?><r><i id="a"/><i id="b"/><i id="c"/><i id="d"/><i id="e"/></r>';
+
+	public function testEqCountsANegativeIndexFromTheEnd()
+	{
+		$qp = qp(self::XML, 'i');
+
+		$this->assertSame('e', $this->idList($qp->eq(-1)));
+		$this->assertSame('d', $this->idList($qp->eq(-2)));
+		$this->assertSame('a', $this->idList($qp->eq(-5)));
+		$this->assertSame('', $this->idList($qp->eq(-6)));
+		$this->assertSame('', $this->idList($qp->eq(5)));
+		$this->assertSame('', $this->idList(qp(self::XML, 'nothing')->eq(-1)));
+	}
+
+	public function testGetCountsANegativeIndexFromTheEnd()
+	{
+		$qp = qp(self::XML, 'i');
+
+		$this->assertSame('e', $qp->get(-1)->getAttribute('id'));
+		$this->assertSame('a', $qp->get(-5)->getAttribute('id'));
+		$this->assertNull($qp->get(-6));
+		$this->assertNull($qp->get(5));
+		$this->assertNull(qp(self::XML, 'nothing')->get(-1));
+	}
+
+	public function testSliceCountsANegativeStartFromTheEnd()
+	{
+		$qp = qp(self::XML, 'i');
+
+		$this->assertSame('d,e', $this->idList($qp->slice(-2)));
+		$this->assertSame('e', $this->idList($qp->slice(-1)));
+		$this->assertSame('a,b,c,d,e', $this->idList($qp->slice(-5)));
+		$this->assertSame('a,b,c,d,e', $this->idList($qp->slice(-9)));
+	}
+}

tests/Issues/Issue144Test.php

@@ -0,0 +1,43 @@
+<?php
+/**
+ * @file
+ *
+ * Regression tests for https://github.com/GravityPDF/querypath/issues/144
+ *
+ * slice($start, $end) takes an exclusive end index. Expected values are jQuery 3.7.1's.
+ */
+
+namespace QueryPathTests;
+
+class Issue144Test extends TestCase
+{
+	private const XML = '<?xml version="1.0"?><r><i id="a"/><i id="b"/><i id="c"/><i id="d"/><i id="e"/></r>';
+
+	/**
+	 * @dataProvider sliceProvider
+	 */
+	public function testSliceTakesAnEndIndex(array $args, string $expected)
+	{
+		$this->assertSame($expected, $this->idList(qp(self::XML, 'i')->slice(...$args)));
+	}
+
+	public function sliceProvider(): array
+	{
+		return [
+			'slice(1, 3)'   => [[1, 3], 'b,c'],
+			'slice(1, -1)'  => [[1, -1], 'b,c,d'],
+			'slice(2, 0)'   => [[2, 0], ''],
+			'slice(0)'      => [[0], 'a,b,c,d,e'],
+			'slice(2)'      => [[2], 'c,d,e'],
+			'slice(1, 9)'   => [[1, 9], 'b,c,d,e'],
+			'slice(3, 2)'   => [[3, 2], ''],
+			'slice(2, 2)'   => [[2, 2], ''],
+			'slice(-3, -1)' => [[-3, -1], 'c,d'],
+			'slice(-2, 5)'  => [[-2, 5], 'd,e'],
+			'slice(0, -5)'  => [[0, -5], ''],
+			'slice(0, -9)'  => [[0, -9], ''],
+			'slice(-9, 2)'  => [[-9, 2], 'a,b'],
+			'slice(5)'      => [[5], ''],
+		];
+	}
+}

tests/QueryPath/DOMQueryTest.php

@@ -1011,7 +1011,7 @@ public function testSlice()
 		$this->assertEquals(0, qp($file, 'li')->slice(9)->count());
 
 		// The first item should be #two, the last #three
-		$qp = qp($file, 'li')->slice(1, 2);
+		$qp = qp($file, 'li')->slice(1, 3);
 		$this->assertEquals(2, $qp->count());
 		$this->assertEquals('two', $qp->attr('id'));
 		$this->assertEquals('three', $qp->eq(1)->attr('id'));