[Console] Fix first choice was invalid when using value

ogizanagi · ogizanagi · commit 03e4ab609b19 · 2015-05-15T16:46:10.000+02:00
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. Add test for all choices.
- Fix inconsistency by always returning values as strings
diff --git a/src/Symfony/Component/Console/Question/ChoiceQuestion.php b/src/Symfony/Component/Console/Question/ChoiceQuestion.php
@@ -149,19 +149,19 @@ private function getDefaultValidator()
                 $result = array_search($value, $choices);
 
                 if (!$isAssoc) {
-                    if (!empty($result)) {
+                    if (false !== $result) {
                         $result = $choices[$result];
                     } elseif (isset($choices[$value])) {
                         $result = $choices[$value];
                     }
-                } elseif (empty($result) && array_key_exists($value, $choices)) {
+                } elseif (false === $result && isset($choices[$value])) {
                     $result = $value;
                 }
 
-                if (empty($result)) {
+                if (false === $result) {
                     throw new \InvalidArgumentException(sprintf($errorMessage, $value));
                 }
-                array_push($multiselectChoices, $result);
+                array_push($multiselectChoices, (string) $result);
             }
 
             if ($multiselect) {
diff --git a/src/Symfony/Component/Console/Tests/Helper/QuestionHelperTest.php b/src/Symfony/Component/Console/Tests/Helper/QuestionHelperTest.php
@@ -210,6 +210,76 @@ public function testAskAndValidate()
         }
     }
 
+    /**
+     * @dataProvider simpleAnswerProvider
+     */
+    public function testSelectChoiceFromSimpleChoices($providedAnswer, $expectedValue)
+    {
+        $possibleChoices = array(
+            'My environment 1',
+            'My environment 2',
+            'My environment 3',
+        );
+
+        $dialog = new QuestionHelper();
+        $dialog->setInputStream($this->getInputStream($providedAnswer."\n"));
+        $helperSet = new HelperSet(array(new FormatterHelper()));
+        $dialog->setHelperSet($helperSet);
+
+        $question = new ChoiceQuestion('Please select the environment to load', $possibleChoices);
+        $answer = $dialog->ask($this->createInputInterfaceMock(), $this->createOutputInterface(), $question);
+
+        $this->assertSame($expectedValue, $answer);
+    }
+
+    public function simpleAnswerProvider()
+    {
+        return array(
+            array(0, 'My environment 1'),
+            array(1, 'My environment 2'),
+            array(2, 'My environment 3'),
+            array('My environment 1', 'My environment 1'),
+            array('My environment 2', 'My environment 2'),
+            array('My environment 3', 'My environment 3'),
+        );
+    }
+
+    /**
+     * @dataProvider mixedKeysChoiceListAnswerProvider
+     */
+    public function testChoiceFromChoicelistWithMixedKeys($providedAnswer, $expectedValue)
+    {
+        $possibleChoices = array(
+            '0' => 'No environment',
+            '1' => 'My environment 1',
+            'env_2' => 'My environment 2',
+            3 => 'My environment 3',
+        );
+
+        $dialog = new QuestionHelper();
+        $dialog->setInputStream($this->getInputStream($providedAnswer."\n"));
+        $helperSet = new HelperSet(array(new FormatterHelper()));
+        $dialog->setHelperSet($helperSet);
+
+        $question = new ChoiceQuestion('Please select the environment to load', $possibleChoices);
+        $question->setMaxAttempts(1);
+        $answer = $dialog->ask($this->createInputInterfaceMock(), $this->createOutputInterface(), $question);
+
+        $this->assertSame($expectedValue, $answer);
+    }
+
+    public function mixedKeysChoiceListAnswerProvider()
+    {
+        return array(
+            array('0', '0'),
+            array('No environment', '0'),
+            array('1', '1'),
+            array('env_2', 'env_2'),
+            array(3, '3'),
+            array('My environment 1', '1'),
+        );
+    }
+
     /**
      * @dataProvider answerProvider
      */
@@ -227,6 +297,7 @@ public function testSelectChoiceFromChoiceList($providedAnswer, $expectedValue)
         $dialog->setHelperSet($helperSet);
 
         $question = new ChoiceQuestion('Please select the environment to load', $possibleChoices);
+        $question->setMaxAttempts(1);
         $answer = $dialog->ask($this->createInputInterfaceMock(), $this->createOutputInterface(), $question);
 
         $this->assertSame($expectedValue, $answer);

Original file line number	Diff line number	Diff line change
`@@ -149,19 +149,19 @@ private function getDefaultValidator()`
`149`	`149`	`$result = array_search($value, $choices);`
`150`	`150`
`151`	`151`	`if (!$isAssoc) {`
`152`		`- if (!empty($result)) {`
	`152`	`+ if (false !== $result) {`
`153`	`153`	`$result = $choices[$result];`
`154`	`154`	`} elseif (isset($choices[$value])) {`
`155`	`155`	`$result = $choices[$value];`
`156`	`156`	`}`
`157`		`- } elseif (empty($result) && array_key_exists($value, $choices)) {`
	`157`	`+ } elseif (false === $result && isset($choices[$value])) {`
`158`	`158`	`$result = $value;`
`159`	`159`	`}`
`160`	`160`
`161`		`- if (empty($result)) {`
	`161`	`+ if (false === $result) {`
`162`	`162`	`throw new \InvalidArgumentException(sprintf($errorMessage, $value));`
`163`	`163`	`}`
`164`		`- array_push($multiselectChoices, $result);`
	`164`	`+ array_push($multiselectChoices, (string) $result);`
`165`	`165`	`}`
`166`	`166`
`167`	`167`	`if ($multiselect) {`