Handle negative and overflowed offsets on TokensList. (#582)

* Handle negative and overflowed offsets on TokensList.

* Fix PHPCS issue.

Author: niconoe-

src/TokensList.php

@@ -6,6 +6,7 @@
 
 use ArrayAccess;
 
+use function array_splice;
 use function count;
 use function in_array;
 use function is_array;
@@ -213,25 +214,29 @@ public function getNextOfTypeAndFlag(int $type, int $flag): ?Token
     }
 
     /**
-     * Sets an value inside the container.
+     * Sets a Token inside the list of tokens.
+     * When defined, offset must be positive otherwise the offset is ignored.
+     * If the offset is not defined (like in array_push) or if it is greater than the number of Tokens already stored,
+     * the Token is appended to the list of tokens.
      *
-     * @param int|null $offset the offset to be set
+     * @param int|null $offset the offset to be set. Must be positive otherwise, nothing will be stored.
      * @param Token    $value  the token to be saved
      *
      * @return void
      */
     #[\ReturnTypeWillChange]
     public function offsetSet($offset, $value)
     {
-        if ($offset === null) {
+        if ($offset === null || $offset >= $this->count) {
             $this->tokens[$this->count++] = $value;
-        } else {
+        } elseif ($offset >= 0) {
             $this->tokens[$offset] = $value;
         }
     }
 
     /**
-     * Gets a value from the container.
+     * Gets a Token from the list of tokens.
+     * If the offset is negative or above the number of tokens set in the list, will return null.
      *
      * @param int $offset the offset to be returned
      *
@@ -240,11 +245,12 @@ public function offsetSet($offset, $value)
     #[\ReturnTypeWillChange]
     public function offsetGet($offset)
     {
-        return $offset < $this->count ? $this->tokens[$offset] : null;
+        return $this->offsetExists($offset) ? $this->tokens[$offset] : null;
     }
 
     /**
      * Checks if an offset was previously set.
+     * If the offset is negative or above the number of tokens set in the list, will return false.
      *
      * @param int $offset the offset to be checked
      *
@@ -253,11 +259,11 @@ public function offsetGet($offset)
     #[\ReturnTypeWillChange]
     public function offsetExists($offset)
     {
-        return $offset < $this->count;
+        return $offset >= 0 && $offset < $this->count;
     }
 
     /**
-     * Unsets the value of an offset.
+     * Unsets the value of an offset, if the offset exists.
      *
      * @param int $offset the offset to be unset
      *
@@ -266,12 +272,11 @@ public function offsetExists($offset)
     #[\ReturnTypeWillChange]
     public function offsetUnset($offset)
     {
-        unset($this->tokens[$offset]);
-        --$this->count;
-        for ($i = $offset; $i < $this->count; ++$i) {
-            $this->tokens[$i] = $this->tokens[$i + 1];
+        if (! $this->offsetExists($offset)) {
+            return;
         }
 
-        unset($this->tokens[$this->count]);
+        array_splice($this->tokens, $offset, 1);
+        --$this->count;
     }
 }

tests/Lexer/TokensListTest.php

@@ -137,17 +137,41 @@ public function testArrayAccess(): void
         // offsetSet($offset, $value)
         $list[2] = $this->tokens[2];
 
+        // offsetSet($offset, $value) with overflowed offset
+        $list[$list->count] = $this->tokens[1];
+        $this->assertSame($list[$list->count - 1], $this->tokens[1]);
+        $this->assertSame($list->count, count($this->tokens) + 1);
+
+        // offsetGet($offset) with negative offset
+        $this->assertNull($list[-1]);
+
+        // offsetGet($offset) with overflow offset
+        $this->assertNull($list[$list->count]);
+
         // offsetGet($offset)
         for ($i = 0, $count = count($this->tokens); $i < $count; ++$i) {
-            $this->assertEquals($this->tokens[$i], $list[$i]);
+            $this->assertSame($this->tokens[$i], $list[$i]);
         }
 
         // offsetExists($offset)
         $this->assertArrayHasKey(2, $list);
-        $this->assertArrayNotHasKey(13, $list);
+        $this->assertArrayNotHasKey(14, $list);
+        $this->assertArrayNotHasKey(-8, $list);
 
         // offsetUnset($offset)
+        $currentCountTokens = $list->count;
         unset($list[2]);
-        $this->assertEquals($this->tokens[3], $list[2]);
+        $newCountTokens = $list->count;
+        $this->assertSame($this->tokens[3], $list[2]);
+        $this->assertSame($currentCountTokens - 1, $newCountTokens);
+
+        // offsetUnset($offset) with invalid offset (negative or overflowed)
+        $currentListTokens = $list->tokens;
+        unset($list[-1]);
+        $this->assertSame($currentListTokens, $list->tokens);
+        $this->assertSame($newCountTokens, $list->count); // No unset actually performed.
+        unset($list[999]);
+        $this->assertSame($currentListTokens, $list->tokens);
+        $this->assertSame($newCountTokens, $list->count); // No unset actually performed.
     }
 }