8000 Fixes #25825: Back out #24987 and fix edge case in getParameterOption. · symfony/symfony@542cf6d · GitHub
[go: up one dir, main page]

Skip to content

Commit 542cf6d

Browse files
Fixes #25825: Back out #24987 and fix edge case in getParameterOption.
1 parent 34b6bdc commit 542cf6d

File tree

3 files changed

+47
-20
lines changed

3 files changed

+47
-20
lines changed

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

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -279,19 +279,13 @@ 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
}
285-
286-
if (0 === strpos($token, '-') && 0 !== strpos($token, '--')) {
287-
$noValue = explode('=', $token);
288-
$token = $noValue[0];
289-
$searchableToken = str_replace('-', '', $token);
290-
$searchableValue = str_replace('-', '', $value);
291-
if ('' !== $searchableToken && '' !== $searchableValue && false !== strpos($searchableToken, $searchableValue)) {
292-
return true;
293-
}
294-
}
295289
}
296290
}
297291

@@ -310,13 +304,16 @@ public function getParameterOption($values, $default = false)
310304
$token = array_shift($tokens);
311305

312306
foreach ($values as $value) {
313-
if ($token === $value || 0 === strpos($token, $value.'=')) {
314-
if (false !== $pos = strpos($token, '=')) {
315-
return substr($token, $pos + 1);
316-
}
317-
307+
if ($token === $value) {
318308
return array_shift($tokens);
319309
}
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+
}
320317
}
321318
}
322319

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

Lines changed: 4 additions & 0 deletions
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

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,10 +296,8 @@ 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', '-fh'));
300-
$this->assertTrue($input->hasParameterOption('-fh'), '->hasParameterOption() returns true if the given short option is in the raw input');
301-
302-
$input = new ArgvInput(array('cli.php', '-e=test'));
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');
303301
$this->assertFalse($input->hasParameterOption('-s'), '->hasParameterOption() returns true if the given short option is in the raw input');
304302

305303
$input = new ArgvInput(array('cli.php', '--foo', 'foo'));
@@ -312,6 +310,33 @@ public function testHasParameterOption()
312310
$this->assertTrue($input->hasParameterOption('--foo'), '->hasParameterOption() returns true if the given option with provided value is in the raw input');
313311
}
314312

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+
315340
public function testToString()
316341
{
317342
$input = new ArgvInput(array('cli.php', '-f', 'foo'));
@@ -333,6 +358,7 @@ public function testGetParameterOptionEqualSign($argv, $key, $expected)
333358
public function provideGetParameterOptionValues()
334359
{
335360
return array(
361+
array(array('app/console', 'foo:bar', '-edev'), '-e', 'dev'),
336362
array(array('app/console', 'foo:bar', '-e', 'dev'), '-e', 'dev'),
337363
array(array('app/console', 'foo:bar', '--env=dev'), '--env', 'dev'),
338364
array(array('app/console', 'foo:bar', '-e', 'dev'), array('-e', '--env'), 'dev'),

0 commit comments

Comments
 (0)
0