Skip to content

Commit

Permalink
bug #27918 [Console] correctly return parameter's default value on "-…
Browse files Browse the repository at this point in the history
…-" (seschwar)

This PR was merged into the 3.4 branch.

Discussion
----------

[Console] correctly return parameter's default value on "--"

Fixes #27916

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27916
| License       | MIT
| Doc PR        | n/a

The tests have been adjusted to use a default value different from the one in `A*Input::getParameterOption()`'s signature. This would have detected the bug in the first place and should prevent future regressions.

Commits
-------

d78dcc0 [Console] correctly return parameter's default value on "--"
  • Loading branch information
fabpot committed Jul 12, 2018
2 parents 08b7874 + d78dcc0 commit 137753d
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/Symfony/Component/Console/Input/ArgvInput.php
Expand Up @@ -306,7 +306,7 @@ public function getParameterOption($values, $default = false, $onlyParams = fals
while (0 < count($tokens)) {
$token = array_shift($tokens);
if ($onlyParams && '--' === $token) {
return false;
return $default;
}

foreach ($values as $value) {
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Console/Input/ArrayInput.php
Expand Up @@ -81,7 +81,7 @@ public function getParameterOption($values, $default = false, $onlyParams = fals

foreach ($this->parameters as $k => $v) {
if ($onlyParams && ('--' === $k || (is_int($k) && '--' === $v))) {
return false;
return $default;
}

if (is_int($k)) {
Expand Down
25 changes: 13 additions & 12 deletions src/Symfony/Component/Console/Tests/Input/ArgvInputTest.php
Expand Up @@ -395,25 +395,26 @@ public function testToString()
/**
* @dataProvider provideGetParameterOptionValues
*/
public function testGetParameterOptionEqualSign($argv, $key, $onlyParams, $expected)
public function testGetParameterOptionEqualSign($argv, $key, $default, $onlyParams, $expected)
{
$input = new ArgvInput($argv);
$this->assertEquals($expected, $input->getParameterOption($key, false, $onlyParams), '->getParameterOption() returns the expected value');
$this->assertEquals($expected, $input->getParameterOption($key, $default, $onlyParams), '->getParameterOption() returns the expected value');
}

public function provideGetParameterOptionValues()
{
return array(
array(array('app/console', 'foo:bar', '-e', 'dev'), '-e', false, 'dev'),
array(array('app/console', 'foo:bar', '--env=dev'), '--env', false, 'dev'),
array(array('app/console', 'foo:bar', '-e', 'dev'), array('-e', '--env'), false, 'dev'),
array(array('app/console', 'foo:bar', '--env=dev'), array('-e', '--env'), false, 'dev'),
array(array('app/console', 'foo:bar', '--env=dev', '--en=1'), array('--en'), false, '1'),
array(array('app/console', 'foo:bar', '--env=dev', '', '--en=1'), array('--en'), false, '1'),
array(array('app/console', 'foo:bar', '--env', 'val'), '--env', false, 'val'),
array(array('app/console', 'foo:bar', '--env', 'val', '--dummy'), '--env', false, 'val'),
array(array('app/console', 'foo:bar', '--', '--env=dev'), '--env', false, 'dev'),
array(array('app/console', 'foo:bar', '--', '--env=dev'), '--env', true, false),
array(array('app/console', 'foo:bar'), '-e', 'default', false, 'default'),
array(array('app/console', 'foo:bar', '-e', 'dev'), '-e', 'default', false, 'dev'),
array(array('app/console', 'foo:bar', '--env=dev'), '--env', 'default', false, 'dev'),
array(array('app/console', 'foo:bar', '-e', 'dev'), array('-e', '--env'), 'default', false, 'dev'),
array(array('app/console', 'foo:bar', '--env=dev'), array('-e', '--env'), 'default', false, 'dev'),
array(array('app/console', 'foo:bar', '--env=dev', '--en=1'), array('--en'), 'default', false, '1'),
array(array('app/console', 'foo:bar', '--env=dev', '', '--en=1'), array('--en'), 'default', false, '1'),
array(array('app/console', 'foo:bar', '--env', 'val'), '--env', 'default', false, 'val'),
array(array('app/console', 'foo:bar', '--env', 'val', '--dummy'), '--env', 'default', false, 'val'),
array(array('app/console', 'foo:bar', '--', '--env=dev'), '--env', 'default', false, 'dev'),
array(array('app/console', 'foo:bar', '--', '--env=dev'), '--env', 'default', true, 'default'),
);
}

Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Console/Tests/Input/ArrayInputTest.php
Expand Up @@ -47,14 +47,14 @@ public function testGetParameterOption()
{
$input = new ArrayInput(array('name' => 'Fabien', '--foo' => 'bar'));
$this->assertEquals('bar', $input->getParameterOption('--foo'), '->getParameterOption() returns the option of specified name');
$this->assertFalse($input->getParameterOption('--bar'), '->getParameterOption() returns the default if an option is not present in the passed parameters');
$this->assertEquals('default', $input->getParameterOption('--bar', 'default'), '->getParameterOption() returns the default value if an option is not present in the passed parameters');

$input = new ArrayInput(array('Fabien', '--foo' => 'bar'));
$this->assertEquals('bar', $input->getParameterOption('--foo'), '->getParameterOption() returns the option of specified name');

$input = new ArrayInput(array('--foo', '--', '--bar' => 'woop'));
$this->assertEquals('woop', $input->getParameterOption('--bar'), '->getParameterOption() returns the correct value if an option is present in the passed parameters');
$this->assertFalse($input->getParameterOption('--bar', false, true), '->getParameterOption() returns false if an option is present in the passed parameters after an end of options signal');
$this->assertEquals('default', $input->getParameterOption('--bar', 'default', true), '->getParameterOption() returns the default value if an option is present in the passed parameters after an end of options signal');
}

public function testParseArguments()
Expand Down

0 comments on commit 137753d

Please sign in to comment.