Skip to content

Commit 6502cb6

Browse files
chalasrchalasr
committed
[Console] Explicitly passed options without value (or empty) should remain empty
Fix empty strings handling
1 parent b860ec3 commit 6502cb6

File tree

7 files changed

+59
-20
lines changed

7 files changed

+59
-20
lines changed

‎src/Symfony/Component/Console/CHANGELOG.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
CHANGELOG
22
=========
33

4+
2.7.25
5+
------
6+
7+
* fixed return value of `Input::getOption()`.
8+
Passing an empty value to an option with value optional now returns an empty value
9+
instead of the option default value.
10+
411
2.6.0
512
-----
613

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

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,12 @@ private function parseLongOption($token)
146146

147147
if (false !== $pos = strpos($name, '=')) {
148148
if (0 === strlen($value = substr($name, $pos + 1))) {
149-
array_unshift($this->parsed, null);
149+
// if no value after "=" then substr() returns "" since php7 only, false before
150+
// see http://php.net/manual/fr/migration70.incompatible.php#119151
151+
if (PHP_VERSION_ID < 70000 && false === $value) {
152+
$value = '';
153+
}
154+
array_unshift($this->parsed, $value);
150155
}
151156
$this->addLongOption(substr($name, 0, $pos), $value);
152157
} else {
@@ -219,23 +224,16 @@ private function addLongOption($name, $value)
219224

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

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

231-
if (null === $value && $option->acceptValue() && count($this->parsed)) {
231+
if (in_array($value, array('', null), true) && $option->acceptValue() && count($this->parsed)) {
232232
// if option accepts an optional or mandatory argument
233233
// let's see if there is one provided
234234
$next = array_shift($this->parsed);
235-
if (isset($next[0]) && '-' !== $next[0]) {
235+
if ((isset($next[0]) && '-' !== $next[0]) || in_array($next, array('', null), true)) {
236236
$value = $next;
237-
} elseif (empty($next)) {
238-
$value = null;
239237
} else {
240238
array_unshift($this->parsed, $next);
241239
}
@@ -246,8 +244,8 @@ private function addLongOption($name, $value)
246244
throw new \RuntimeException(sprintf('The "--%s" option requires a value.', $name));
247245
}
248246

249-
if (!$option->isArray()) {
250-
$value = $option->isValueOptional() ? $option->getDefault() : true;
247+
if (!$option->isArray() && !$option->isValueOptional()) {
248+
$value = true;
251249
}
252250
}
253251

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,9 @@ private function addLongOption($name, $value)
165165
throw new \InvalidArgumentException(sprintf('The "--%s" option requires a value.', $name));
166166
}
167167

168-
$value = $option->isValueOptional() ? $option->getDefault() : true;
168+
if (!$option->isValueOptional()) {
169+
$value = true;
170+
}
169171
}
170172

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ public function getOption($name)
154154
throw new \InvalidArgumentException(sprintf('The "%s" option does not exist.', $name));
155155
}
156156

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

160160
/**

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

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public function testParseOptions($input, $options, $expectedOptions, $message)
4747
$input = new ArgvInput($input);
4848
$input->bind(new InputDefinition($options));
4949

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

5353
public function provideOptions()
@@ -74,14 +74,32 @@ public function provideOptions()
7474
array(
7575
array('cli.php', '--foo='),
7676
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL)),
77-
array('foo' => null),
78-
'->parse() parses long options with optional value which is empty (with a = separator) as null',
77+
array('foo' => ''),
78+
'->parse() parses long options with optional value which is empty (with a = separator) as empty string',
7979
),
8080
array(
8181
array('cli.php', '--foo=', 'bar'),
8282
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::REQUIRED)),
83+
array('foo' => ''),
84+
'->parse() parses long options with optional value without value specified or an empty string (with a = separator) followed by an argument as empty string',
85+
),
86+
array(
87+
array('cli.php', 'bar', '--foo'),
88+
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::REQUIRED)),
89+
array('foo' => null),
90+
'->parse() parses long options with optional value which is empty (with a = separator) preceded by an argument',
91+
),
92+
array(
93+
array('cli.php', '--foo', '', 'bar'),
94+
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::REQUIRED)),
95+
array('foo' => ''),
96+
'->parse() parses long options with optional value which is empty as empty string even followed by an argument',
97+
),
98+
array(
99+
array('cli.php', '--foo'),
100+
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL)),
83101
array('foo' => null),
84-
'->parse() parses long options with optional value which is empty (with a = separator) followed by an argument',
102+
'->parse() parses long options with optional value specified with no separator and no value as null',
85103
),
86104
array(
87105
array('cli.php', '-f'),
@@ -246,7 +264,7 @@ public function testParseArrayOption()
246264

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

251269
$input = new ArgvInput(array('cli.php', '--name', 'foo', '--name', 'bar', '--name', '--anotherOption'));
252270
$input->bind(new InputDefinition(array(

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,15 @@ public function provideOptions()
8080
'->parse() parses long options with a default value',
8181
),
8282
array(
83-
array('--foo' => null),
83+
array(),
8484
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL, '', 'default')),
8585
array('foo' => 'default'),
86+
'->parse() uses the default value for long options value option which are not passed',
87+
),
88+
array(
89+
array('--foo' => null),
90+
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL, '', 'default')),
91+
array('foo' => null),
8692
'->parse() parses long options with a default value',
8793
),
8894
array(

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,14 @@ public function testOptions()
3636
$input = new ArrayInput(array('--name' => 'foo'), new InputDefinition(array(new InputOption('name'), new InputOption('bar', '', InputOption::VALUE_OPTIONAL, '', 'default'))));
3737
$this->assertEquals('default', $input->getOption('bar'), '->getOption() returns the default value for optional options');
3838
$this->assertEquals(array('name' => 'foo', 'bar' => 'default'), $input->getOptions(), '->getOptions() returns all option values, even optional ones');
39+
40+
$input = new ArrayInput(array('--name' => 'foo', '--bar' => ''), new InputDefinition(array(new InputOption('name'), new InputOption('bar', '', InputOption::VALUE_OPTIONAL, '', 'default'))));
41+
$this->assertEquals('', $input->getOption('bar'), '->getOption() returns null for options explicitly passed without value (or an empty value)');
42+
$this->assertEquals(array('name' => 'foo', 'bar' => ''), $input->getOptions(), '->getOptions() returns all option values.');
43+
44+
$input = new ArrayInput(array('--name' => 'foo', '--bar' => null), new InputDefinition(array(new InputOption('name'), new InputOption('bar', '', InputOption::VALUE_OPTIONAL, '', 'default'))));
45+
$this->assertNull($input->getOption('bar'), '->getOption() returns null for options explicitly passed without value (or an empty value)');
46+
$this->assertEquals(array('name' => 'foo', 'bar' => null), $input->getOptions(), '->getOptions() returns all option values');
3947
}
4048

4149
/**

0 commit comments

Comments
 (0)