Merge pull request #5206 from kenjis/fix-command-params

Fix options are not passed to Command $params

Author: kenjis

phpstan-baseline.neon.dist

@@ -105,11 +105,6 @@ parameters:
 			count: 1
 			path: system/CodeIgniter.php
 
-		-
-			message: "#^Call to an undefined method CodeIgniter\\\\HTTP\\\\Request\\:\\:getSegments\\(\\)\\.$#"
-			count: 1
-			path: system/CodeIgniter.php
-
 		-
 			message: "#^Call to an undefined method CodeIgniter\\\\HTTP\\\\Request\\:\\:setLocale\\(\\)\\.$#"
 			count: 1

system/CodeIgniter.php

@@ -858,21 +858,34 @@ protected function createController()
     /**
      * Runs the controller, allowing for _remap methods to function.
      *
+     * CI4 supports three types of requests:
+     *  1. Web: URI segments become parameters, sent to Controllers via Routes,
+     *      output controlled by Headers to browser
+     *  2. Spark: accessed by CLI via the spark command, arguments are Command arguments,
+     *      sent to Commands by CommandRunner, output controlled by CLI class
+     *  3. PHP CLI: accessed by CLI via php public/index.php, arguments become URI segments,
+     *      sent to Controllers via Routes, output varies
+     *
      * @param mixed $class
      *
      * @return false|ResponseInterface|string|void
      */
     protected function runController($class)
     {
-        // If this is a console request then use the input segments as parameters
-        $params = $this->isSparked() ? $this->request->getSegments() : $this->router->params();
-
-        if (method_exists($class, '_remap')) {
-            $output = $class->_remap($this->method, ...$params);
+        if ($this->isSparked()) {
+            // This is a Spark request
+            /** @var CLIRequest $request */
+            $request = $this->request;
+            $params  = $request->getArgs();
         } else {
-            $output = $class->{$this->method}(...$params);
+            // This is a Web request or PHP CLI request
+            $params = $this->router->params();
         }
 
+        $output = method_exists($class, '_remap')
+            ? $class->_remap($this->method, ...$params)
+            : $class->{$this->method}(...$params);
+
         $this->benchmark->stop('controller');
 
         return $output;

system/HTTP/CLIRequest.php

@@ -41,6 +41,13 @@ class CLIRequest extends Request
      */
     protected $options = [];
 
+    /**
+     * Command line arguments (segments and options).
+     *
+     * @var array
+     */
+    protected $args = [];
+
     /**
      * Set the expected HTTP verb
      *
@@ -94,6 +101,14 @@ public function getOptions(): array
         return $this->options;
     }
 
+    /**
+     * Returns an array of all CLI arguments (segments and options).
+     */
+    public function getArgs(): array
+    {
+        return $this->args;
+    }
+
     /**
      * Returns the path segments.
      */
@@ -173,6 +188,7 @@ protected function parseCommand()
                     $optionValue = false;
                 } else {
                     $this->segments[] = $arg;
+                    $this->args[]     = $arg;
                 }
 
                 continue;
@@ -187,6 +203,7 @@ protected function parseCommand()
             }
 
             $this->options[$arg] = $value;
+            $this->args[$arg]    = $value;
         }
     }
 

tests/system/CLI/CommandRunnerTest.php

@@ -118,6 +118,9 @@ public function testBadCommand()
         $this->assertStringContainsString('Command "bogus" not found', CITestStreamFilter::$buffer);
     }
 
+    /**
+     * @TODO When the first param is empty? Use case?
+     */
     public function testRemapEmptyFirstParams()
     {
         self::$runner->_remap('anyvalue', null, 'list');

tests/system/HTTP/CLIRequestTest.php

@@ -162,6 +162,34 @@ public function testParsingNoOptions()
         $this->assertSame($expected, $this->request->getOptionString());
     }
 
+    public function testParsingArgs()
+    {
+        $_SERVER['argv'] = [
+            'spark',
+            'command',
+            'param1',
+            'param2',
+            '--opt1',
+            'opt1val',
+            '--opt-2',
+            'opt 2 val',
+            'param3',
+        ];
+
+        // reinstantiate it to force parsing
+        $this->request = new CLIRequest(new App());
+
+        $options = [
+            'command',
+            'param1',
+            'param2',
+            'opt1'  => 'opt1val',
+            'opt-2' => 'opt 2 val',
+            'param3',
+        ];
+        $this->assertSame($options, $this->request->getArgs());
+    }
+
     public function testParsingPath()
     {
         $_SERVER['argv'] = [