bug symfony#25893 [Console] Fix hasParameterOption / getParameterOption when used with multiple flags (greg-1-anderson)

This PR was merged into the 2.7 branch.

Discussion
----------

[Console] Fix hasParameterOption / getParameterOption when used with multiple flags

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no (Fixes BC break in symfony#24987)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony#25825
| License       | MIT
| Doc PR        | n/a

Proposed resolution to symfony#25825:
- Back out symfony#24987
- Fix getParameterOption for short options with values, e.g. `-edev`

Commits
-------

35f98e2 Follow-on to symfony#25825: Fix edge case in getParameterOption.

Author: fabpot

src/Symfony/Component/Console/Input/ArgvInput.php

@@ -279,7 +279,11 @@ public function hasParameterOption($values)
 
         foreach ($this->tokens as $token) {
             foreach ($values as $value) {
-                if ($token === $value || 0 === strpos($token, $value.'=')) {
+                // Options with values:
+                //   For long options, test for '--option=' at beginning
+                //   For short options, test for '-o' at beginning
+                $leading = 0 === strpos($value, '--') ? $value.'=' : $value;
+                if ($token === $value || 0 === strpos($token, $leading)) {
                     return true;
                 }
             }
@@ -300,13 +304,16 @@ public function getParameterOption($values, $default = false)
             $token = array_shift($tokens);
 
             foreach ($values as $value) {
-                if ($token === $value || 0 === strpos($token, $value.'=')) {
-                    if (false !== $pos = strpos($token, '=')) {
-                        return substr($token, $pos + 1);
-                    }
-
+                if ($token === $value) {
                     return array_shift($tokens);
                 }
+                // Options with values:
+                //   For long options, test for '--option=' at beginning
+                //   For short options, test for '-o' at beginning
+                $leading = 0 === strpos($value, '--') ? $value.'=' : $value;
+                if (0 === strpos($token, $leading)) {
+                    return substr($token, strlen($leading));
+                }
             }
         }
 

src/Symfony/Component/Console/Input/InputInterface.php

@@ -30,6 +30,8 @@ public function getFirstArgument();
      *
      * This method is to be used to introspect the input parameters
      * before they have been validated. It must be used carefully.
+     * Does not necessarily return the correct result for short options
+     * when multiple flags are combined in the same option.
      *
      * @param string|array $values The values to look for in the raw parameters (can be an array)
      *
@@ -42,6 +44,8 @@ public function hasParameterOption($values);
      *
      * This method is to be used to introspect the input parameters
      * before they have been validated. It must be used carefully.
+     * Does not necessarily return the correct result for short options
+     * when multiple flags are combined in the same option.
      *
      * @param string|array $values  The value(s) to look for in the raw parameters (can be an array)
      * @param mixed        $default The default value to return if no result is found

src/Symfony/Component/Console/Tests/Input/ArgvInputTest.php

@@ -296,6 +296,10 @@ public function testHasParameterOption()
         $input = new ArgvInput(array('cli.php', '-f', 'foo'));
         $this->assertTrue($input->hasParameterOption('-f'), '->hasParameterOption() returns true if the given short option is in the raw input');
 
+        $input = new ArgvInput(array('cli.php', '-etest'));
+        $this->assertTrue($input->hasParameterOption('-e'), '->hasParameterOption() returns true if the given short option is in the raw input');
+        $this->assertFalse($input->hasParameterOption('-s'), '->hasParameterOption() returns true if the given short option is in the raw input');
+
         $input = new ArgvInput(array('cli.php', '--foo', 'foo'));
         $this->assertTrue($input->hasParameterOption('--foo'), '->hasParameterOption() returns true if the given short option is in the raw input');
 
@@ -306,6 +310,33 @@ public function testHasParameterOption()
         $this->assertTrue($input->hasParameterOption('--foo'), '->hasParameterOption() returns true if the given option with provided value is in the raw input');
     }
 
+    public function testHasParameterOptionEdgeCasesAndLimitations()
+    {
+        $input = new ArgvInput(array('cli.php', '-fh'));
+        // hasParameterOption does not know if the previous short option, -f,
+        // takes a value or not. If -f takes a value, then -fh does NOT include
+        // -h; Otherwise it does. Since we do not know which short options take
+        // values, hasParameterOption does not support this use-case.
+        $this->assertFalse($input->hasParameterOption('-h'), '->hasParameterOption() returns true if the given short option is in the raw input');
+        // hasParameterOption does detect that `-fh` contains `-f`, since
+        // `-f` is the first short option in the set.
+        $this->assertTrue($input->hasParameterOption('-f'), '->hasParameterOption() returns true if the given short option is in the raw input');
+        // The test below happens to pass, although it might make more sense
+        // to disallow it, and require the use of
+        // $input->hasParameterOption('-f') && $input->hasParameterOption('-h')
+        // instead.
+        $this->assertTrue($input->hasParameterOption('-fh'), '->hasParameterOption() returns true if the given short option is in the raw input');
+        // In theory, if -fh is supported, then -hf should also work.
+        // However, this is not supported.
+        $this->assertFalse($input->hasParameterOption('-hf'), '->hasParameterOption() returns true if the given short option is in the raw input');
+
+        $input = new ArgvInput(array('cli.php', '-f', '-h'));
+        // If hasParameterOption('-fh') is supported for 'cli.php -fh', then
+        // one might also expect that it should also be supported for
+        // 'cli.php -f -h'. However, this is not supported.
+        $this->assertFalse($input->hasParameterOption('-fh'), '->hasParameterOption() returns true if the given short option is in the raw input');
+    }
+
     public function testToString()
     {
         $input = new ArgvInput(array('cli.php', '-f', 'foo'));
@@ -327,6 +358,7 @@ public function testGetParameterOptionEqualSign($argv, $key, $expected)
     public function provideGetParameterOptionValues()
     {
         return array(
+            array(array('app/console', 'foo:bar', '-edev'), '-e', 'dev'),
             array(array('app/console', 'foo:bar', '-e', 'dev'), '-e', 'dev'),
             array(array('app/console', 'foo:bar', '--env=dev'), '--env', 'dev'),
             array(array('app/console', 'foo:bar', '-e', 'dev'), array('-e', '--env'), 'dev'),