From 3349d3ce8974a0935fa94f545933f2cedc53b4d2 Mon Sep 17 00:00:00 2001 From: Sean Templeton Date: Tue, 3 Dec 2019 11:54:04 -0600 Subject: [PATCH 1/3] Remove restriction for choices to be strings --- src/Symfony/Component/Console/Question/ChoiceQuestion.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Console/Question/ChoiceQuestion.php b/src/Symfony/Component/Console/Question/ChoiceQuestion.php index 020b733f13..dfb7db67ca 100644 --- a/src/Symfony/Component/Console/Question/ChoiceQuestion.php +++ b/src/Symfony/Component/Console/Question/ChoiceQuestion.php @@ -169,7 +169,7 @@ class ChoiceQuestion extends Question throw new InvalidArgumentException(sprintf($errorMessage, $value)); } - $multiselectChoices[] = (string) $result; + $multiselectChoices[] = $result; } if ($multiselect) { From a0223088a05a1dd38736ce3c1e172485911dc1ac Mon Sep 17 00:00:00 2001 From: YaFou <33806646+YaFou@users.noreply.github.com> Date: Sun, 23 Aug 2020 18:56:11 +0200 Subject: [PATCH 2/3] [Console] Add tests for removing restriction for choices to be strings See 3349d3ce8974a0935fa94f545933f2cedc53b4d2 --- .../Tests/Question/ChoiceQuestionTest.php | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/Symfony/Component/Console/Tests/Question/ChoiceQuestionTest.php b/src/Symfony/Component/Console/Tests/Question/ChoiceQuestionTest.php index 9db12f8528..ff81ed4d61 100644 --- a/src/Symfony/Component/Console/Tests/Question/ChoiceQuestionTest.php +++ b/src/Symfony/Component/Console/Tests/Question/ChoiceQuestionTest.php @@ -77,4 +77,34 @@ class ChoiceQuestionTest extends TestCase $this->assertSame(['First response ', ' Second response'], $question->getValidator()('First response , Second response')); } + + public function testSelectWithNonStringChoices() + { + $question = new ChoiceQuestion('A question', [ + $result1 = new StringChoice('foo'), + $result2 = new StringChoice('bar'), + $result3 = new StringChoice('baz'), + ]); + + $this->assertSame($result1, $question->getValidator()('foo')); + + $question->setMultiselect(true); + + $this->assertSame([$result3, $result2], $question->getValidator()('baz, bar')); + } +} + +class StringChoice +{ + private $string; + + public function __construct(string $string) + { + $this->string = $string; + } + + public function __toString() + { + return $this->string; + } } From d276cc9ca30f1798bfaa3dd71c7008d281ffcb3f Mon Sep 17 00:00:00 2001 From: Maxime Steinhausser Date: Mon, 31 Aug 2020 16:36:30 +0200 Subject: [PATCH 3/3] [Console] Cast associative choices questions keys to string to prevent inconsistency when choosing by key (getting a string as result) or by value (getting an int as result) --- .../Console/Question/ChoiceQuestion.php | 3 +- .../Tests/Helper/QuestionHelperTest.php | 35 --------------- .../Tests/Question/ChoiceQuestionTest.php | 44 ++++++++++++++++++- 3 files changed, 45 insertions(+), 37 deletions(-) diff --git a/src/Symfony/Component/Console/Question/ChoiceQuestion.php b/src/Symfony/Component/Console/Question/ChoiceQuestion.php index dfb7db67ca..445630d046 100644 --- a/src/Symfony/Component/Console/Question/ChoiceQuestion.php +++ b/src/Symfony/Component/Console/Question/ChoiceQuestion.php @@ -169,7 +169,8 @@ class ChoiceQuestion extends Question throw new InvalidArgumentException(sprintf($errorMessage, $value)); } - $multiselectChoices[] = $result; + // For associative choices, consistently return the key as string: + $multiselectChoices[] = $isAssoc ? (string) $result : $result; } if ($multiselect) { diff --git a/src/Symfony/Component/Console/Tests/Helper/QuestionHelperTest.php b/src/Symfony/Component/Console/Tests/Helper/QuestionHelperTest.php index fcba3b3b2f..7e3e9963f8 100644 --- a/src/Symfony/Component/Console/Tests/Helper/QuestionHelperTest.php +++ b/src/Symfony/Component/Console/Tests/Helper/QuestionHelperTest.php @@ -570,41 +570,6 @@ class QuestionHelperTest extends AbstractQuestionHelperTest ]; } - /** - * @dataProvider mixedKeysChoiceListAnswerProvider - */ - public function testChoiceFromChoicelistWithMixedKeys($providedAnswer, $expectedValue) - { - $possibleChoices = [ - '0' => 'No environment', - '1' => 'My environment 1', - 'env_2' => 'My environment 2', - 3 => 'My environment 3', - ]; - - $dialog = new QuestionHelper(); - $helperSet = new HelperSet([new FormatterHelper()]); - $dialog->setHelperSet($helperSet); - - $question = new ChoiceQuestion('Please select the environment to load', $possibleChoices); - $question->setMaxAttempts(1); - $answer = $dialog->ask($this->createStreamableInputInterfaceMock($this->getInputStream($providedAnswer."\n")), $this->createOutputInterface(), $question); - - $this->assertSame($expectedValue, $answer); - } - - public function mixedKeysChoiceListAnswerProvider() - { - return [ - ['0', '0'], - ['No environment', '0'], - ['1', '1'], - ['env_2', 'env_2'], - [3, '3'], - ['My environment 1', '1'], - ]; - } - /** * @dataProvider answerProvider */ diff --git a/src/Symfony/Component/Console/Tests/Question/ChoiceQuestionTest.php b/src/Symfony/Component/Console/Tests/Question/ChoiceQuestionTest.php index ff81ed4d61..18063eaac9 100644 --- a/src/Symfony/Component/Console/Tests/Question/ChoiceQuestionTest.php +++ b/src/Symfony/Component/Console/Tests/Question/ChoiceQuestionTest.php @@ -59,6 +59,18 @@ class ChoiceQuestionTest extends TestCase ['First response', 'Second response'], 'When passed multiple answers on MultiSelect, the defaultValidator must return these answers as an array', ], + [ + false, + [0], + 'First response', + 'When passed single answer using choice\'s key, the defaultValidator must return the choice value', + ], + [ + true, + ['0, 2'], + ['First response', 'Third response'], + 'When passed multiple answers using choices\' key, the defaultValidator must return the choice values in an array', + ], ]; } @@ -78,6 +90,35 @@ class ChoiceQuestionTest extends TestCase $this->assertSame(['First response ', ' Second response'], $question->getValidator()('First response , Second response')); } + /** + * @dataProvider selectAssociativeChoicesProvider + */ + public function testSelectAssociativeChoices($providedAnswer, $expectedValue) + { + $question = new ChoiceQuestion('A question', [ + '0' => 'First choice', + 'foo' => 'Foo', + '99' => 'N°99', + 'string object' => new StringChoice('String Object'), + ]); + + $this->assertSame($expectedValue, $question->getValidator()($providedAnswer)); + } + + public function selectAssociativeChoicesProvider() + { + return [ + 'select "0" choice by key' => ['0', '0'], + 'select "0" choice by value' => ['First choice', '0'], + 'select by key' => ['foo', 'foo'], + 'select by value' => ['Foo', 'foo'], + 'select by key, with numeric key' => ['99', '99'], + 'select by value, with numeric key' => ['N°99', '99'], + 'select by key, with string object value' => ['string object', 'string object'], + 'select by value, with string object value' => ['String Object', 'string object'], + ]; + } + public function testSelectWithNonStringChoices() { $question = new ChoiceQuestion('A question', [ @@ -86,7 +127,8 @@ class ChoiceQuestionTest extends TestCase $result3 = new StringChoice('baz'), ]); - $this->assertSame($result1, $question->getValidator()('foo')); + $this->assertSame($result1, $question->getValidator()('foo'), 'answer can be selected by its string value'); + $this->assertSame($result1, $question->getValidator()(0), 'answer can be selected by index'); $question->setMultiselect(true);