[Console] Explicitly passed options without value (or empty) should r…

…emain empty
symfony · fabpot · Mar 1, 2017 · Jan 10, 2017 · Feb 28, 2017 · 80867422acd83d2602187155e7eb978d6c3d0d44
commit 80867422acd83d2602187155e7eb978d6c3d0d44
diff --git a/UPGRADE-3.3.md b/UPGRADE-3.3.md
@@ -6,6 +6,62 @@ ClassLoader

 * The component is deprecated and will be removed in 4.0. Use Composer instead.

+Console
+-------
+
+* `Input::getOption()` no longer returns the default value for options
+  with value optional explicitly passed empty.
+
+  For:
+
+  ```php
+  protected function configure()
+  {
+      $this
+          // ...
+          ->setName('command')
+          ->addOption('foo', null, InputOption::VALUE_OPTIONAL, '', 'default')
+       ;
+  }
+
+  protected function execute(InputInterface $input, OutputInterface $output)
+  {
+      var_dump($input->getOption('foo'));
+  }
+  ```
+
+  Before:
+
+  ```
+  $ php console.php command
+  "default"
+
+  $ php console.php command --foo
+  "default"
+
+  $ php console.php command --foo ""
+  "default"
+
+  $ php console.php command --foo=
+  "default"
+  ```
+
+  After:
+
+  ```
+  $ php console.php command
+  "default"
+
+  $ php console.php command --foo
+  NULL
+
+  $ php console.php command --foo ""
+  ""
+
+  $ php console.php command --foo=
+  ""
+  ```
+
 Debug
 -----


diff --git a/src/Symfony/Component/Console/CHANGELOG.md b/src/Symfony/Component/Console/CHANGELOG.md
@@ -6,6 +6,8 @@ CHANGELOG

 * added `ExceptionListener`
 * added `AddConsoleCommandPass` (originally in FrameworkBundle)
+* [BC BREAK] `Input::getOption()` no longer returns the default value for options
+  with value optional explicitly passed empty

 3.2.0
 ------

diff --git a/src/Symfony/Component/Console/Input/ArgvInput.php b/src/Symfony/Component/Console/Input/ArgvInput.php
@@ -148,7 +148,12 @@ private function parseLongOption($token)

        if (false !== $pos = strpos($name, '=')) {
            if (0 === strlen($value = substr($name, $pos + 1))) {
-                array_unshift($this->parsed, null);
+                // if no value after "=" then substr() returns "" since php7 only, false before
+                // see http://php.net/manual/fr/migration70.incompatible.php#119151
+                if (PHP_VERSION_ID < 70000 && false === $value) {
+                    $value = '';
+                }
+                array_unshift($this->parsed, $value);
            }
            $this->addLongOption(substr($name, 0, $pos), $value);
        } else {
@@ -221,23 +226,16 @@ private function addLongOption($name, $value)

        $option = $this->definition->getOption($name);

-        // Convert empty values to null
-        if (!isset($value[0])) {
-            $value = null;
-        }
-
        if (null !== $value && !$option->acceptValue()) {
            throw new RuntimeException(sprintf('The "--%s" option does not accept a value.', $name));
        }

-        if (null === $value && $option->acceptValue() && count($this->parsed)) {
+        if (in_array($value, array('', null), true) && $option->acceptValue() && count($this->parsed)) {
            // if option accepts an optional or mandatory argument
            // let's see if there is one provided
            $next = array_shift($this->parsed);
-            if (isset($next[0]) && '-' !== $next[0]) {
+            if ((isset($next[0]) && '-' !== $next[0]) || in_array($next, array('', null), true)) {
                $value = $next;
-            } elseif (empty($next)) {
-                $value = null;
            } else {
                array_unshift($this->parsed, $next);
            }
@@ -248,8 +246,8 @@ private function addLongOption($name, $value)
                throw new RuntimeException(sprintf('The "--%s" option requires a value.', $name));
            }

-            if (!$option->isArray()) {
-                $value = $option->isValueOptional() ? $option->getDefault() : true;
+            if (!$option->isArray() && !$option->isValueOptional()) {
+                $value = true;
            }
        }


diff --git a/src/Symfony/Component/Console/Input/ArrayInput.php b/src/Symfony/Component/Console/Input/ArrayInput.php
@@ -179,7 +179,9 @@ private function addLongOption($name, $value)
                throw new InvalidOptionException(sprintf('The "--%s" option requires a value.', $name));
            }

-            $value = $option->isValueOptional() ? $option->getDefault() : true;
+            if (!$option->isValueOptional()) {
+                $value = true;
+            }
        }

        $this->options[$name] = $value;

diff --git a/src/Symfony/Component/Console/Input/Input.php b/src/Symfony/Component/Console/Input/Input.php
@@ -158,7 +158,7 @@ public function getOption($name)
            throw new InvalidArgumentException(sprintf('The "%s" option does not exist.', $name));
        }

-        return isset($this->options[$name]) ? $this->options[$name] : $this->definition->getOption($name)->getDefault();
+        return array_key_exists($name, $this->options) ? $this->options[$name] : $this->definition->getOption($name)->getDefault();
    }

    /**

diff --git a/src/Symfony/Component/Console/Tests/Input/ArgvInputTest.php b/src/Symfony/Component/Console/Tests/Input/ArgvInputTest.php
@@ -48,7 +48,7 @@ public function testParseOptions($input, $options, $expectedOptions, $message)
        $input = new ArgvInput($input);
        $input->bind(new InputDefinition($options));

-        $this->assertEquals($expectedOptions, $input->getOptions(), $message);
+        $this->assertSame($expectedOptions, $input->getOptions(), $message);
    }

    public function provideOptions()
@@ -75,14 +75,32 @@ public function provideOptions()
            array(
                array('cli.php', '--foo='),
                array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL)),
-                array('foo' => null),
-                '->parse() parses long options with optional value which is empty (with a = separator) as null',
+                array('foo' => ''),
+                '->parse() parses long options with optional value which is empty (with a = separator) as empty string',
            ),
            array(
                array('cli.php', '--foo=', 'bar'),
                array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::REQUIRED)),
+                array('foo' => ''),
+                '->parse() parses long options with optional value without value specified or an empty string (with a = separator) followed by an argument as empty string',
+            ),
+            array(
+                array('cli.php', 'bar', '--foo'),
+                array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::REQUIRED)),
+                array('foo' => null),
+                '->parse() parses long options with optional value which is empty (with a = separator) preceded by an argument',
+            ),
+            array(
+                array('cli.php', '--foo', '', 'bar'),
+                array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::REQUIRED)),
+                array('foo' => ''),
+                '->parse() parses long options with optional value which is empty as empty string even followed by an argument',
+            ),
+            array(
+                array('cli.php', '--foo'),
+                array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL)),
                array('foo' => null),
-                '->parse() parses long options with optional value which is empty (with a = separator) followed by an argument',
+                '->parse() parses long options with optional value specified with no separator and no value as null',
            ),
            array(
                array('cli.php', '-f'),
@@ -252,14 +270,14 @@ public function testParseArrayOption()

        $input = new ArgvInput(array('cli.php', '--name=foo', '--name=bar', '--name='));
        $input->bind(new InputDefinition(array(new InputOption('name', null, InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY))));
-        $this->assertSame(array('name' => array('foo', 'bar', null)), $input->getOptions(), '->parse() parses empty array options as null ("--option=value" syntax)');
+        $this->assertSame(array('name' => array('foo', 'bar', '')), $input->getOptions(), '->parse() parses empty array options as null ("--option=value" syntax)');

        $input = new ArgvInput(array('cli.php', '--name', 'foo', '--name', 'bar', '--name', '--anotherOption'));
        $input->bind(new InputDefinition(array(
            new InputOption('name', null, InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY),
            new InputOption('anotherOption', null, InputOption::VALUE_NONE),
        )));
-        $this->assertSame(array('name' => array('foo', 'bar', null), 'anotherOption' => true), $input->getOptions(), '->parse() parses empty array options as null ("--option value" syntax)');
+        $this->assertSame(array('name' => array('foo', 'bar', null), 'anotherOption' => true), $input->getOptions(), '->parse() parses empty array options ("--option value" syntax)');
    }

    public function testParseNegativeNumberAfterDoubleDash()

diff --git a/src/Symfony/Component/Console/Tests/Input/ArrayInputTest.php b/src/Symfony/Component/Console/Tests/Input/ArrayInputTest.php
@@ -90,9 +90,15 @@ public function provideOptions()
                '->parse() parses long options with a default value',
            ),
            array(
-                array('--foo' => null),
+                array(),
                array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL, '', 'default')),
                array('foo' => 'default'),
+                '->parse() uses the default value for long options with value optional which are not passed',
+            ),
+            array(
+                array('--foo' => null),
+                array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL, '', 'default')),
+                array('foo' => null),
                '->parse() parses long options with a default value',
            ),
            array(

diff --git a/src/Symfony/Component/Console/Tests/Input/InputTest.php b/src/Symfony/Component/Console/Tests/Input/InputTest.php
@@ -37,6 +37,14 @@ public function testOptions()
        $input = new ArrayInput(array('--name' => 'foo'), new InputDefinition(array(new InputOption('name'), new InputOption('bar', '', InputOption::VALUE_OPTIONAL, '', 'default'))));
        $this->assertEquals('default', $input->getOption('bar'), '->getOption() returns the default value for optional options');
        $this->assertEquals(array('name' => 'foo', 'bar' => 'default'), $input->getOptions(), '->getOptions() returns all option values, even optional ones');
+
+        $input = new ArrayInput(array('--name' => 'foo', '--bar' => ''), new InputDefinition(array(new InputOption('name'), new InputOption('bar', '', InputOption::VALUE_OPTIONAL, '', 'default'))));
+        $this->assertEquals('', $input->getOption('bar'), '->getOption() returns null for options explicitly passed without value (or an empty value)');
+        $this->assertEquals(array('name' => 'foo', 'bar' => ''), $input->getOptions(), '->getOptions() returns all option values.');
+
+        $input = new ArrayInput(array('--name' => 'foo', '--bar' => null), new InputDefinition(array(new InputOption('name'), new InputOption('bar', '', InputOption::VALUE_OPTIONAL, '', 'default'))));
+        $this->assertNull($input->getOption('bar'), '->getOption() returns null for options explicitly passed without value (or an empty value)');
+        $this->assertEquals(array('name' => 'foo', 'bar' => null), $input->getOptions(), '->getOptions() returns all option values');
    }

    /**