From 94942b3aff480dd6bbab1d7614b5726783c6166f Mon Sep 17 00:00:00 2001
From: dualfroz <me@dualfroz.com>
Date: Fri, 4 Sep 2026 09:21:56 +0200
Subject: [PATCH] fix: do not read an undefined offset when a value argument is
 last

The argument parser looked up the next command-line token via $argv[$key + 1]
guarded only by count($argv) === 0. The remaining arguments array is not
reindexed after earlier arguments are removed, so a value-taking argument placed
last (after an unconsumed token) read an undefined offset and raised an Undefined
array key warning under PHP 8, and the argument was set to empty instead of
falling back to its default. Check that the next offset exists with isset().
---
 PR.md                          | 88 ++++++++++++++++++++++++++++++++++
 src/Argument/Parser.php        |  8 +++-
 tests/Argument/ManagerTest.php | 24 ++++++++++
 3 files changed, 119 insertions(+), 1 deletion(-)
 create mode 100644 PR.md

diff --git a/PR.md b/PR.md
new file mode 100644
index 0000000..36b9333
--- /dev/null
+++ b/PR.md
@@ -0,0 +1,88 @@
+# fix: do not read an undefined offset when a value argument is last
+
+## Problem
+
+When an argument that expects a value is the last token on the command line and
+is preceded by an unconsumed non-prefixed token, the parser emits an
+`Undefined array key` warning under PHP 8:
+
+```php
+$climate = new League\CLImate\CLImate();
+$climate->arguments->add([
+    'name' => ['prefix' => 'n', 'longPrefix' => 'name', 'defaultValue' => 'default-name'],
+]);
+
+$climate->arguments->parse(['script', 'somefile', '-n']);
+// PHP Warning: Undefined array key 2 in Parser.php on line 270
+```
+
+On top of the warning, the argument is then set to an empty value instead of
+falling back to its configured default, which is inconsistent with the behaviour
+when a value argument is the only remaining token (that case correctly falls back
+to the default).
+
+## Root cause
+
+`src/Argument/Parser.php`, `setArgumentValue()`:
+
+```php
+if (is_null($value)) {
+    if (count($argv) === 0) {
+        return $argv;
+    }
+
+    // If the value wasn't previously defined in "key=value"
+    // format then define it from the next command argument.
+    $nextArgvValue = $argv[$key + 1];
+    ...
+}
+```
+
+The remaining arguments array is not reindexed after previously matched arguments
+are removed with `unset()`. `count($argv) === 0` is therefore not a reliable test
+for "there is no following token": when an unconsumed token still sits at another
+offset (for example `somefile` at offset 0), `count($argv)` is non-zero even
+though offset `$key + 1` does not exist, so `$argv[$key + 1]` reads an undefined
+key.
+
+## The fix
+
+Check that the next offset actually exists instead of relying on the overall
+count. When there is no following token the method returns without setting a
+value, so the argument falls back to its default (matching the existing
+last-token behaviour):
+
+```php
+if (is_null($value)) {
+    if (!isset($argv[$key + 1])) {
+        return $argv;
+    }
+
+    $nextArgvValue = $argv[$key + 1];
+    if ($this->isValidArgumentValue($nextArgvValue)) {
+        $argument->setValue($nextArgvValue);
+        unset($argv[$key + 1]);
+        return $argv;
+    }
+}
+```
+
+## Test
+
+Added `ManagerTest::testItParsesAValueTakingArgumentThatIsLastWithoutAWarning`,
+which installs an error handler that turns any warning into an exception, parses
+`['command', 'somefile', '-n']`, and asserts that no warning is raised and the
+argument falls back to its default value.
+
+## Gate
+
+```
+composer install && composer test
+```
+
+`composer test` runs `vendor/bin/phpunit`. Result: exit code `0` -
+`OK (274 tests, 152 assertions)`.
+
+Counterfactual: reverting `setArgumentValue()` to the `count($argv) === 0` guard
+makes the new test fail with `ErrorException: Undefined array key 2` raised at
+`Parser.php:270`.
diff --git a/src/Argument/Parser.php b/src/Argument/Parser.php
index 66bd209..7b7573d 100644
--- a/src/Argument/Parser.php
+++ b/src/Argument/Parser.php
@@ -261,7 +261,13 @@ class Parser
         }
 
         if (is_null($value)) {
-            if (count($argv) === 0) {
+            // If the value wasn't previously defined in "key=value" format
+            // then define it from the next command argument, if there is one.
+            // The array is not reindexed after prior arguments are removed, so
+            // check that the next offset actually exists rather than relying on
+            // the overall count (which may be non-zero because of remaining
+            // tokens sitting at other offsets).
+            if (!isset($argv[$key + 1])) {
                 return $argv;
             }
 
diff --git a/tests/Argument/ManagerTest.php b/tests/Argument/ManagerTest.php
index 15648e1..5af6d82 100644
--- a/tests/Argument/ManagerTest.php
+++ b/tests/Argument/ManagerTest.php
@@ -87,6 +87,30 @@ class ManagerTest extends TestCase
         $this->assertEquals('', $this->manager->get('foo'));
     }
 
+    public function testItParsesAValueTakingArgumentThatIsLastWithoutAWarning()
+    {
+        $this->manager->add([
+            'name' => ['prefix' => 'n', 'longPrefix' => 'name', 'defaultValue' => 'default-name'],
+        ]);
+
+        set_error_handler(static function (int $errno, string $errstr): bool {
+            throw new \ErrorException($errstr, 0, $errno);
+        });
+
+        try {
+            // "somefile" is an unconsumed non-prefixed token, so "-n" is the
+            // last element but not at a reindexed offset; reading the next
+            // offset must not raise an "Undefined array key" warning.
+            $this->manager->parse(['command', 'somefile', '-n']);
+        } finally {
+            restore_error_handler();
+        }
+
+        // No value was supplied for -n, so it must fall back to its default.
+        $this->assertFalse($this->manager->provided('name'));
+        $this->assertEquals('default-name', $this->manager->get('name'));
+    }
+
     public function testItStoresTrailingInArray()
     {
         $this->manager->add([