Skip to content

Commit f7d9701

Browse files
fabpotfabpot
committed
bug #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 #24987) | Deprecations? | no | Tests pass? | yes | Fixed tickets | #25825 | License | MIT | Doc PR | n/a Proposed resolution to #25825: - Back out #24987 - Fix getParameterOption for short options with values, e.g. `-edev` Commits ------- 35f98e2 Follow-on to #25825: Fix edge case in getParameterOption.
2 parents 12447d9 + 35f98e2 commit f7d9701

File tree

3 files changed

+49
-6
lines changed

3 files changed

+49
-6
lines changed

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

+13-6
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,11 @@ public function hasParameterOption($values)
279279

280280
foreach ($this->tokens as $token) {
281281
foreach ($values as $value) {
282-
if ($token === $value || 0 === strpos($token, $value.'=')) {
282+
// Options with values:
283+
// For long options, test for '--option=' at beginning
284+
// For short options, test for '-o' at beginning
285+
$leading = 0 === strpos($value, '--') ? $value.'=' : $value;
286+
if ($token === $value || 0 === strpos($token, $leading)) {
283287
return true;
284288
}
285289
}
@@ -300,13 +304,16 @@ public function getParameterOption($values, $default = false)
300304
$token = array_shift($tokens);
301305

302306
foreach ($values as $value) {
303-
if ($token === $value || 0 === strpos($token, $value.'=')) {
304-
if (false !== $pos = strpos($token, '=')) {
305-
return substr($token, $pos + 1);
306-
}
307-
307+
if ($token === $value) {
308308
return array_shift($tokens);
309309
}
310+
// Options with values:
311+
// For long options, test for '--option=' at beginning
312+
// For short options, test for '-o' at beginning
313+
$leading = 0 === strpos($value, '--') ? $value.'=' : $value;
314+
if (0 === strpos($token, $leading)) {
315+
return substr($token, strlen($leading));
316+
}
310317
}
311318
}
312319

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

+4
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ public function getFirstArgument();
3030
*
3131
* This method is to be used to introspect the input parameters
3232
* before they have been validated. It must be used carefully.
33+
* Does not necessarily return the correct result for short options
34+
* when multiple flags are combined in the same option.
3335
*
3436
* @param string|array $values The values to look for in the raw parameters (can be an array)
3537
*
@@ -42,6 +44,8 @@ public function hasParameterOption($values);
4244
*
4345
* This method is to be used to introspect the input parameters
4446
* before they have been validated. It must be used carefully.
47+
* Does not necessarily return the correct result for short options
48+
* when multiple flags are combined in the same option.
4549
*
4650
* @param string|array $values The value(s) to look for in the raw parameters (can be an array)
4751
* @param mixed $default The default value to return if no result is found

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

+32
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,10 @@ public function testHasParameterOption()
296296
$input = new ArgvInput(array('cli.php', '-f', 'foo'));
297297
$this->assertTrue($input->hasParameterOption('-f'), '->hasParameterOption() returns true if the given short option is in the raw input');
298298

299+
$input = new ArgvInput(array('cli.php', '-etest'));
300+
$this->assertTrue($input->hasParameterOption('-e'), '->hasParameterOption() returns true if the given short option is in the raw input');
301+
$this->assertFalse($input->hasParameterOption('-s'), '->hasParameterOption() returns true if the given short option is in the raw input');
302+
299303
$input = new ArgvInput(array('cli.php', '--foo', 'foo'));
300304
$this->assertTrue($input->hasParameterOption('--foo'), '->hasParameterOption() returns true if the given short option is in the raw input');
301305

@@ -306,6 +310,33 @@ public function testHasParameterOption()
306310
$this->assertTrue($input->hasParameterOption('--foo'), '->hasParameterOption() returns true if the given option with provided value is in the raw input');
307311
}
308312

313+
public function testHasParameterOptionEdgeCasesAndLimitations()
314+
{
315+
$input = new ArgvInput(array('cli.php', '-fh'));
316+
// hasParameterOption does not know if the previous short option, -f,
317+
// takes a value or not. If -f takes a value, then -fh does NOT include
318+
// -h; Otherwise it does. Since we do not know which short options take
319+
// values, hasParameterOption does not support this use-case.
320+
$this->assertFalse($input->hasParameterOption('-h'), '->hasParameterOption() returns true if the given short option is in the raw input');
321+
// hasParameterOption does detect that `-fh` contains `-f`, since
322+
// `-f` is the first short option in the set.
323+
$this->assertTrue($input->hasParameterOption('-f'), '->hasParameterOption() returns true if the given short option is in the raw input');
324+
// The test below happens to pass, although it might make more sense
325+
// to disallow it, and require the use of
326+
// $input->hasParameterOption('-f') && $input->hasParameterOption('-h')
327+
// instead.
328+
$this->assertTrue($input->hasParameterOption('-fh'), '->hasParameterOption() returns true if the given short option is in the raw input');
329+
// In theory, if -fh is supported, then -hf should also work.
330+
// However, this is not supported.
331+
$this->assertFalse($input->hasParameterOption('-hf'), '->hasParameterOption() returns true if the given short option is in the raw input');
332+
333+
$input = new ArgvInput(array('cli.php', '-f', '-h'));
334+
// If hasParameterOption('-fh') is supported for 'cli.php -fh', then
335+
// one might also expect that it should also be supported for
336+
// 'cli.php -f -h'. However, this is not supported.
337+
$this->assertFalse($input->hasParameterOption('-fh'), '->hasParameterOption() returns true if the given short option is in the raw input');
338+
}
339+
309340
public function testToString()
310341
{
311342
$input = new ArgvInput(array('cli.php', '-f', 'foo'));
@@ -327,6 +358,7 @@ public function testGetParameterOptionEqualSign($argv, $key, $expected)
327358
public function provideGetParameterOptionValues()
328359
{
329360
return array(
361+
array(array('app/console', 'foo:bar', '-edev'), '-e', 'dev'),
330362
array(array('app/console', 'foo:bar', '-e', 'dev'), '-e', 'dev'),
331363
array(array('app/console', 'foo:bar', '--env=dev'), '--env', 'dev'),
332364
array(array('app/console', 'foo:bar', '-e', 'dev'), array('-e', '--env'), 'dev'),

0 commit comments

Comments
 (0)