Skip to content

Commit efd68e6

Browse files
fabpotfabpot
committed
bug #14648 [Console] Fix first choice was invalid when using value (ogizanagi)
This PR was merged into the 2.7 branch. Discussion ---------- [Console] Fix first choice was invalid when using value | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This PR solves the following issues encountered using question helper and choices questions: - First choice was not selectable by value. - ChoiceList with associative choices with mixed string and int keys has same issue with first choice. - Fix inconsistency by always returning values as strings. First point exemple: ![screenshot 2015-05-15 a 17 16 12](https://cloud.githubusercontent.com/assets/2211145/7655757/3344b39a-fb26-11e4-9fe7-0775616619bf.PNG) Last two points are mainly edge cases. Indeed, if a QuestionChoice has something like : ```php array( '0' => 'No environment', '1' => 'My environment 1', 'env_2' => 'My environment 2', 3 => 'My environment 3', ); ``` as choices, you will not be able to select the first choice and get an `InvalidArgumentException`: ``` There were 2 errors: 1) Symfony\Component\Console\Tests\Helper\QuestionHelperTest::testChoiceFromChoicelistWithMixedKeys with data set #0 ('0', '0') InvalidArgumentException: Value "0" is invalid 2) Symfony\Component\Console\Tests\Helper\QuestionHelperTest::testChoiceFromChoicelistWithMixedKeys with data set #1 ('No environment', '0') InvalidArgumentException: Value "No environment" is invalid ``` Moreover, even if you were able to select by value (`No environment`), you'll get an integer instead of a string: ``` Failed asserting that '0' is identical to 0. ``` For more consistency, the solution is to always return a string. The issue does not exist in 2.6, as the `QuestionChoice::getDefaultValidator` handled things differently. Commits ------- 03e4ab6 [Console] Fix first choice was invalid when using value
2 parents c7e0538 + 03e4ab6 commit efd68e6

File tree

2 files changed

+74
-4
lines changed

2 files changed

+74
-4
lines changed

‎src/Symfony/Component/Console/Question/ChoiceQuestion.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,19 +149,19 @@ private function getDefaultValidator()
149149
$result = array_search($value, $choices);
150150

151151
if (!$isAssoc) {
152-
if (!empty($result)) {
152+
if (false !== $result) {
153153
$result = $choices[$result];
154154
} elseif (isset($choices[$value])) {
155155
$result = $choices[$value];
156156
}
157-
} elseif (empty($result) && array_key_exists($value, $choices)) {
157+
} elseif (false === $result && isset($choices[$value])) {
158158
$result = $value;
159159
}
160160

161-
if (empty($result)) {
161+
if (false === $result) {
162162
throw new \InvalidArgumentException(sprintf($errorMessage, $value));
163163
}
164-
array_push($multiselectChoices, $result);
164+
array_push($multiselectChoices, (string) $result);
165165
}
166166

167167
if ($multiselect) {

‎src/Symfony/Component/Console/Tests/Helper/QuestionHelperTest.php‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,76 @@ public function testAskAndValidate()
210210
}
211211
}
212212

213+
/**
214+
* @dataProvider simpleAnswerProvider
215+
*/
216+
public function testSelectChoiceFromSimpleChoices($providedAnswer, $expectedValue)
217+
{
218+
$possibleChoices = array(
219+
'My environment 1',
220+
'My environment 2',
221+
'My environment 3',
222+
);
223+
224+
$dialog = new QuestionHelper();
225+
$dialog->setInputStream($this->getInputStream($providedAnswer."\n"));
226+
$helperSet = new HelperSet(array(new FormatterHelper()));
227+
$dialog->setHelperSet($helperSet);
228+
229+
$question = new ChoiceQuestion('Please select the environment to load', $possibleChoices);
230+
$answer = $dialog->ask($this->createInputInterfaceMock(), $this->createOutputInterface(), $question);
231+
232+
$this->assertSame($expectedValue, $answer);
233+
}
234+
235+
public function simpleAnswerProvider()
236+
{
237+
return array(
238+
array(0, 'My environment 1'),
239+
array(1, 'My environment 2'),
240+
array(2, 'My environment 3'),
241+
array('My environment 1', 'My environment 1'),
242+
array('My environment 2', 'My environment 2'),
243+
array('My environment 3', 'My environment 3'),
244+
);
245+
}
246+
247+
/**
248+
* @dataProvider mixedKeysChoiceListAnswerProvider
249+
*/
250+
public function testChoiceFromChoicelistWithMixedKeys($providedAnswer, $expectedValue)
251+
{
252+
$possibleChoices = array(
253+
'0' => 'No environment',
254+
'1' => 'My environment 1',
255+
'env_2' => 'My environment 2',
256+
3 => 'My environment 3',
257+
);
258+
259+
$dialog = new QuestionHelper();
260+
$dialog->setInputStream($this->getInputStream($providedAnswer."\n"));
261+
$helperSet = new HelperSet(array(new FormatterHelper()));
262+
$dialog->setHelperSet($helperSet);
263+
264+
$question = new ChoiceQuestion('Please select the environment to load', $possibleChoices);
265+
$question->setMaxAttempts(1);
266+
$answer = $dialog->ask($this->createInputInterfaceMock(), $this->createOutputInterface(), $question);
267+
268+
$this->assertSame($expectedValue, $answer);
269+
}
270+
271+
public function mixedKeysChoiceListAnswerProvider()
272+
{
273+
return array(
274+
array('0', '0'),
275+
array('No environment', '0'),
276+
array('1', '1'),
277+
array('env_2', 'env_2'),
278+
array(3, '3'),
279+
array('My environment 1', '1'),
280+
);
281+
}
282+
213283
/**
214284
* @dataProvider answerProvider
215285
*/

0 commit comments

Comments
 (0)