Correctly handle optional arguments whose default value is null.

consolidation · May 21, 2016 · ed279df · ed279df
1 parent 35466de
commit ed279df
Show file tree

Hide file tree

Showing 5 changed files with 80 additions and 27 deletions.
diff --git a/src/AnnotatedCommand.php b/src/AnnotatedCommand.php
@@ -154,14 +154,15 @@ protected function setCommandArgumentsFromParameters($commandInfo)
         $args = $commandInfo->arguments()->getValues();
         foreach ($args as $name => $defaultValue) {
             $description = $commandInfo->arguments()->getDescription($name);
-            $parameterMode = $this->getCommandArgumentMode($defaultValue);
+            $hasDefault = $commandInfo->arguments()->hasDefault($name);
+            $parameterMode = $this->getCommandArgumentMode($hasDefault, $defaultValue);
             $this->addArgument($name, $parameterMode, $description, $defaultValue);
         }
     }
 
-    protected function getCommandArgumentMode($defaultValue)
+    protected function getCommandArgumentMode($hasDefault, $defaultValue)
     {
-        if (!isset($defaultValue)) {
+        if (!$hasDefault) {
             return InputArgument::REQUIRED;
         }
         if (is_array($defaultValue)) {

diff --git a/src/Parser/CommandInfo.php b/src/Parser/CommandInfo.php
@@ -84,7 +84,7 @@ public function __construct($classNameOrInstance, $methodName)
         // This can be overridden via @command or @name annotations.
         $this->name = $this->convertName($this->reflection->name);
         $this->options = new DefaultsWithDescriptions($this->determineOptionsFromParameters(), false);
-        $this->arguments = new DefaultsWithDescriptions($this->determineAgumentClassifications());
+        $this->arguments = $this->determineAgumentClassifications();
     }
 
     /**
@@ -361,46 +361,39 @@ protected function findExistingOption($optionName)
      */
     protected function determineAgumentClassifications()
     {
-        $args = [];
+        $result = new DefaultsWithDescriptions();
         $params = $this->reflection->getParameters();
         $optionsFromParameters = $this->determineOptionsFromParameters();
         if (!empty($optionsFromParameters)) {
             array_pop($params);
         }
         foreach ($params as $param) {
-            $defaultValue = $this->getArgumentClassification($param);
-            if ($defaultValue !== false) {
-                $args[$param->name] = $defaultValue;
-            }
+            $this->addParameterToResult($result, $param);
         }
-        return $args;
+        return $result;
     }
 
     /**
      * Examine the provided parameter, and determine whether it
      * is a parameter that will be filled in with a positional
      * commandline argument.
-     *
-     * @return false|null|string|array
      */
-    protected function getArgumentClassification($param)
+    protected function addParameterToResult($result, $param)
     {
-        $defaultValue = null;
+        // Commandline arguments must be strings, so ignore any
+        // parameter that is typehinted to any non-primative class.
+        if ($param->getClass() != null) {
+            return;
+        }
+        $result->add($param->name);
         if ($param->isDefaultValueAvailable()) {
             $defaultValue = $param->getDefaultValue();
-            if ($this->isAssoc($defaultValue)) {
-                return false;
+            if (!$this->isAssoc($defaultValue)) {
+                $result->setDefaultValue($param->name, $defaultValue);
             }
+        } elseif ($param->isArray()) {
+            $result->setDefaultValue($param->name, []);
         }
-        if ($param->isArray()) {
-            return [];
-        }
-        // Commandline arguments must be strings, so ignore
-        // any parameter that is typehinted to anything else.
-        if (($param->getClass() != null) && ($param->getClass() != 'string')) {
-            return false;
-        }
-        return $defaultValue;
     }
 
     /**

diff --git a/src/Parser/DefaultsWithDescriptions.php b/src/Parser/DefaultsWithDescriptions.php
@@ -12,6 +12,12 @@ class DefaultsWithDescriptions
      */
     protected $values;
 
+    /**
+     * @var array Associative array used like a set to indicate default value
+     * exists for the key.
+     */
+    protected $hasDefault;
+
     /**
      * @var array Associative array of key : description mappings
      */
@@ -23,9 +29,10 @@ class DefaultsWithDescriptions
      */
     protected $defaultDefault;
 
-    public function __construct($values, $defaultDefault = null)
+    public function __construct($values = [], $defaultDefault = null)
     {
         $this->values = $values;
+        $this->hasDefault = [];
         $this->descriptions = [];
         $this->defaultDefault = $defaultDefault;
     }
@@ -86,7 +93,7 @@ public function getDescription($key)
      * @param string $description Help text for the argument.
      * @param mixed $defaultValue The default value for the argument.
      */
-    public function add($key, $description, $defaultValue = null)
+    public function add($key, $description = '', $defaultValue = null)
     {
         if (!$this->exists($key) || isset($defaultValue)) {
             $this->values[$key] = isset($defaultValue) ? $defaultValue : $this->defaultDefault;
@@ -106,6 +113,18 @@ public function add($key, $description, $defaultValue = null)
     public function setDefaultValue($key, $defaultValue)
     {
         $this->values[$key] = $defaultValue;
+        $this->hasDefault[$key] = true;
+    }
+
+    /**
+     * Check to see if the named argument definitively has a default value.
+     *
+     * @param string $key
+     * @return bool
+     */
+    public function hasDefault($key)
+    {
+        return array_key_exists($key, $this->hasDefault);
     }
 
     /**

diff --git a/tests/src/ExampleCommandFile.php b/tests/src/ExampleCommandFile.php
@@ -167,4 +167,22 @@ public function validateTestHello($args)
             return new CommandError("I won't say hello to Donald Duck.");
         }
     }
+
+    /**
+     * Test default values in arguments
+     *
+     * @param string|null $one
+     * @param string|null $two
+     * @return string
+     */
+    public function defaults($one = null, $two = null)
+    {
+        if ($one && $two) {
+            return "$one and $two";
+        }
+        if ($one) {
+            return "only $one";
+        }
+        return "nothing provided";
+    }
 }
diff --git a/tests/testAnnotatedCommandFactory.php b/tests/testAnnotatedCommandFactory.php
@@ -54,6 +54,28 @@ function testMyCatCommand()
         $this->assertRunCommandViaApplicationEquals($command, $input, 'alphabet');
     }
 
+    function testDefaultsCommand()
+    {
+        $commandFileInstance = new \Consolidation\TestUtils\ExampleCommandFile;
+        $commandFactory = new AnnotatedCommandFactory();
+        $commandInfo = $commandFactory->createCommandInfo($commandFileInstance, 'defaults');
+
+        $command = $commandFactory->createCommand($commandInfo, $commandFileInstance);
+
+        $this->assertInstanceOf('\Symfony\Component\Console\Command\Command', $command);
+        $this->assertEquals('defaults', $command->getName());
+        $this->assertEquals('Test default values in arguments', $command->getDescription());
+
+        $input = new StringInput('defaults');
+        $this->assertRunCommandViaApplicationEquals($command, $input, 'nothing provided');
+
+        $input = new StringInput('defaults ichi');
+        $this->assertRunCommandViaApplicationEquals($command, $input, 'only ichi');
+
+        $input = new StringInput('defaults I II');
+        $this->assertRunCommandViaApplicationEquals($command, $input, 'I and II');
+    }
+
     function testCommandWithNoOptions()
     {
         $commandFileInstance = new \Consolidation\TestUtils\ExampleCommandFile;