From 3410779975ef5cabc0152072211f2d8acee877ed Mon Sep 17 00:00:00 2001 From: boite Date: Mon, 7 Dec 2015 15:13:55 +0000 Subject: [PATCH 1/4] [Form] Use strict comparison assertions in tests Use assertSame instead of assertEquals in some ChoiceTypeTest methods which test placeholder/empty_value options, because the tested value may be any number of "falsey" values. --- .../Form/Tests/Extension/Core/Type/ChoiceTypeTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php index b404405d191d4..b4cf18f365385 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php @@ -1637,7 +1637,7 @@ public function testPassPlaceholderToView($multiple, $expanded, $required, $plac )); $view = $form->createView(); - $this->assertEquals($viewValue, $view->vars['placeholder']); + $this->assertSame($viewValue, $view->vars['placeholder']); $this->assertFalse($view->vars['placeholder_in_choices']); } @@ -1657,9 +1657,9 @@ public function testPassEmptyValueBC($multiple, $expanded, $required, $placehold )); $view = $form->createView(); - $this->assertEquals($viewValue, $view->vars['placeholder']); + $this->assertSame($viewValue, $view->vars['placeholder']); $this->assertFalse($view->vars['placeholder_in_choices']); - $this->assertEquals($viewValue, $view->vars['empty_value']); + $this->assertSame($viewValue, $view->vars['empty_value']); $this->assertFalse($view->vars['empty_value_in_choices']); } From 9ed36d195e34070a046ea1a36cdd6326f0574073 Mon Sep 17 00:00:00 2001 From: boite Date: Mon, 7 Dec 2015 15:15:43 +0000 Subject: [PATCH 2/4] [Form] Prefer placeholder option to empty_value Add an extra condition to the use of the value of the empty_value option as the placeholder: the value of the placeholder option must be null or an empty string (i.e. not explicitly set). Test the effects of setting both placeholder and empty_value. --- .../Form/Extension/Core/Type/ChoiceType.php | 4 +- .../Extension/Core/Type/ChoiceTypeTest.php | 134 ++++++++++++++++++ 2 files changed, 137 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index 1465551b906a6..fb563a41c8388 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -325,7 +325,9 @@ public function configureOptions(OptionsResolver $resolver) if (!is_object($options['empty_value']) || !$options['empty_value'] instanceof \Exception) { @trigger_error('The form option "empty_value" is deprecated since version 2.6 and will be removed in 3.0. Use "placeholder" instead.', E_USER_DEPRECATED); - $placeholder = $options['empty_value']; + if (null === $placeholder || '' === $placeholder) { + $placeholder = $options['empty_value']; + } } if ($options['multiple']) { diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php index b4cf18f365385..f845e566994ba 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php @@ -1726,6 +1726,140 @@ public function getOptionsWithPlaceholder() ); } + /** + * @dataProvider getOptionsWithPlaceholderAndEmptyValue + * @group legacy + */ + public function testPlaceholderOptionWithEmptyValueOption( + $multiple, + $expanded, + $required, + $placeholder, + $emptyValue, + $viewValue + ) { + $form = $this->factory->create('choice', null, array( + 'multiple' => $multiple, + 'expanded' => $expanded, + 'required' => $required, + 'placeholder' => $placeholder, + 'empty_value' => $emptyValue, + 'choices' => $this->choices, + )); + $view = $form->createView(); + + $this->assertSame($viewValue, $view->vars['placeholder']); + $this->assertFalse($view->vars['placeholder_in_choices']); + } + + public function getOptionsWithPlaceholderAndEmptyValue() + { + return array( + // single non-expanded, not required + 'A placeholder is not used if it is explicitly set to false' => array(false, false, false, false, false, null), + 'A placeholder is not used if it is explicitly set to false' => array(false, false, false, false, null, null), + 'A placeholder is not used if it is explicitly set to false' => array(false, false, false, false, '', null), + 'A placeholder is not used if it is explicitly set to false' => array(false, false, false, false, 'bar', null), + 'A placeholder is not used if empty_value is set to false [maintains BC]' => array(false, false, false, null, false, null), + 'An unset empty_value is automaticaly made an empty string in a non-required field (but null is expected here) [maintains BC]' => array(false, false, false, null, null, ''), + 'An empty string empty_value is used if placeholder is not set [maintains BC]' => array(false, false, false, null, '', ''), + 'A non-empty string empty_value is used if placeholder is not set [maintains BC]' => array(false, false, false, null, 'bar', 'bar'), + 'A placeholder is not used if it is an empty string and empty_value is set to false [mainatins BC]' => array(false, false, false, '', false, null), + 'An unset empty_value is automatically made an empty string in a non-required field (but null is expected here) [maintains BC]' => array(false, false, false, '', null, null), + 'An empty string empty_value is used if placeholder is also an empty string [maintains BC]' => array(false, false, false, '', '', ''), + 'A non-empty string empty_value is used if placeholder is an empty string [maintains BC]' => array(false, false, false, '', 'bar', 'bar'), + 'A non-empty string placeholder takes precedence over an empty_value set to false' => array(false, false, false, 'foo', false, 'foo'), + 'A non-empty string placeholder takes precendece over a not set empty_value' => array(false, false, false, 'foo', null, 'foo'), + 'A non-empty string placeholder takes precedence over an empty string empty_value' => array(false, false, false, 'foo', '', 'foo'), + 'A non-empty string placeholder takes precedence over a non-empty string empty_value' => array(false, false, false, 'foo', 'bar', 'foo'), + // single non-expanded, required + 'A placeholder is not used if it is explicitly set to false' => array(false, false, true, false, false, null), + 'A placeholder is not used if it is explicitly set to false' => array(false, false, true, false, null, null), + 'A placeholder is not used if it is explicitly set to false' => array(false, false, true, false, '', null), + 'A placeholder is not used if it is explicitly set to false' => array(false, false, true, false, 'bar', null), + 'A placeholder is not used if empty_value is set to false [maintains BC]' => array(false, false, true, null, false, null), + 'A placeholder is not used if empty_value is not set [maintains BC]' => array(false, false, true, null, null, null), + 'An empty string empty_value is used if placeholder is not set [maintains BC]' => array(false, false, true, null, '', ''), + 'A non-empty string empty_value is used if placeholder is not set [maintains BC]' => array(false, false, true, null, 'bar', 'bar'), + 'A placeholder is not used if it is an empty string and empty_value is set to false [mainatins BC]' => array(false, false, true, '', false, null), + 'A placeholder is not used if empty_value is not set [maintains BC]' => array(false, false, true, '', null, null), + 'An empty string empty_value is used if placeholder is also an empty string [maintains BC]' => array(false, false, true, '', '', ''), + 'A non-empty string empty_value is used if placeholder is an empty string [maintains BC]' => array(false, false, true, '', 'bar', 'bar'), + 'A non-empty string placeholder takes precedence over an empty_value set to false' => array(false, false, true, 'foo', false, 'foo'), + 'A non-empty string placeholder takes precendece over a not set empty_value' => array(false, false, true, 'foo', null, 'foo'), + 'A non-empty string placeholder takes precedence over an empty string empty_value' => array(false, false, true, 'foo', '', 'foo'), + 'A non-empty string placeholder takes precedence over a non-empty string empty_value' => array(false, false, true, 'foo', 'bar', 'foo'), + // single expanded, not required + 'A placeholder is not used if it is explicitly set to false' => array(false, true, false, false, false, null), + 'A placeholder is not used if it is explicitly set to false' => array(false, true, false, false, null, null), + 'A placeholder is not used if it is explicitly set to false' => array(false, true, false, false, '', null), + 'A placeholder is not used if it is explicitly set to false' => array(false, true, false, false, 'bar', null), + 'A placeholder is not used if empty_value is set to false [maintains BC]' => array(false, true, false, null, false, null), + 'An unset empty_value is automaticaly made an empty string in a non-required field (but null is expected here) [maintains BC]' => array(false, true, false, null, null, null), + 'An empty string empty_value is converted to "None" in an expanded single choice field [maintains BC]' => array(false, true, false, null, '', 'None'), + 'A non-empty string empty_value is used if placeholder is not set [maintains BC]' => array(false, true, false, null, 'bar', 'bar'), + 'A placeholder is not used if it is an empty string and empty_value is set to false [mainatins BC]' => array(false, true, false, '', false, null), + 'An unset empty_value is automatically made an empty string in a non-required field (but null is expected here) [maintains BC]' => array(false, true, false, '', null, null), + 'An empty string empty_value is converted to "None" in an expanded single choice field [maintains BC]' => array(false, true, false, '', '', 'None'), + 'A non-empty string empty_value is used if placeholder is an empty string [maintains BC]' => array(false, true, false, '', 'bar', 'bar'), + 'A non-empty string placeholder takes precedence over an empty_value set to false' => array(false, true, false, 'foo', false, 'foo'), + 'A non-empty string placeholder takes precendece over a not set empty_value' => array(false, true, false, 'foo', null, 'foo'), + 'A non-empty string placeholder takes precedence over an empty string empty_value' => array(false, true, false, 'foo', '', 'foo'), + 'A non-empty string placeholder takes precedence over a non-empty string empty_value' => array(false, true, false, 'foo', 'bar', 'foo'), + // single expanded, required + 'A placeholder is not used if it is explicitly set to false' => array(false, true, true, false, false, null), + 'A placeholder is not used if it is explicitly set to false' => array(false, true, true, false, null, null), + 'A placeholder is not used if it is explicitly set to false' => array(false, true, true, false, '', null), + 'A placeholder is not used if it is explicitly set to false' => array(false, true, true, false, 'bar', null), + 'A placeholder is not used if empty_value is set to false [maintains BC]' => array(false, true, true, null, false, null), + 'A placeholder is not used if empty_value is not set [maintains BC]' => array(false, true, true, null, null, null), + 'An empty string empty_value is converted to "None" in an expanded single choice field [maintains BC]' => array(false, true, true, null, '', 'None'), + 'A non-empty string empty_value is used if placeholder is not set [maintains BC]' => array(false, true, true, null, 'bar', 'bar'), + 'A placeholder is not used if it is an empty string and empty_value is set to false [mainatins BC]' => array(false, true, true, '', false, null), + 'A placeholder is not used if empty_value is not set [maintains BC]' => array(false, true, true, '', null, null), + 'An empty string empty_value is converted to "None" in an expanded single choice field [maintains BC]' => array(false, true, true, '', '', 'None'), + 'A non-empty string empty_value is used if placeholder is an empty string [maintains BC]' => array(false, true, true, '', 'bar', 'bar'), + 'A non-empty string placeholder takes precedence over an empty_value set to false' => array(false, true, true, 'foo', false, 'foo'), + 'A non-empty string placeholder takes precendece over a not set empty_value' => array(false, true, true, 'foo', null, 'foo'), + 'A non-empty string placeholder takes precedence over an empty string empty_value' => array(false, true, true, 'foo', '', 'foo'), + 'A non-empty string placeholder takes precedence over a non-empty string empty_value' => array(false, true, true, 'foo', 'bar', 'foo'), + // multiple expanded, not required + array(true, true, false, false, false, null), + array(true, true, false, false, null, null), + array(true, true, false, false, '', null), + array(true, true, false, false, 'bar', null), + array(true, true, false, null, false, null), + array(true, true, false, null, null, null), + array(true, true, false, null, '', null), + array(true, true, false, null, 'bar', null), + array(true, true, false, '', false, null), + array(true, true, false, '', null, null), + array(true, true, false, '', '', null), + array(true, true, false, '', 'bar', null), + array(true, true, false, 'foo', false, null), + array(true, true, false, 'foo', null, null), + array(true, true, false, 'foo', '', null), + array(true, true, false, 'foo', 'bar', null), + // multiple expanded, required + array(true, true, true, false, false, null), + array(true, true, true, false, null, null), + array(true, true, true, false, '', null), + array(true, true, true, false, 'bar', null), + array(true, true, true, null, false, null), + array(true, true, true, null, null, null), + array(true, true, true, null, '', null), + array(true, true, true, null, 'bar', null), + array(true, true, true, '', false, null), + array(true, true, true, '', null, null), + array(true, true, true, '', '', null), + array(true, true, true, '', 'bar', null), + array(true, true, true, 'foo', false, null), + array(true, true, true, 'foo', null, null), + array(true, true, true, 'foo', '', null), + array(true, true, true, 'foo', 'bar', null), + ); + } + public function testPassChoicesToView() { $choices = array('A' => 'a', 'B' => 'b', 'C' => 'c', 'D' => 'd'); From 3c31a91999ca7b1968cc9a3a260c415a9a3e2158 Mon Sep 17 00:00:00 2001 From: boite Date: Tue, 16 Feb 2016 23:57:52 +0000 Subject: [PATCH 3/4] Fix typos in test dataset names. --- .../Form/Tests/Extension/Core/Type/ChoiceTypeTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php index f845e566994ba..0ab634a88db9d 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php @@ -1764,7 +1764,7 @@ public function getOptionsWithPlaceholderAndEmptyValue() 'An unset empty_value is automaticaly made an empty string in a non-required field (but null is expected here) [maintains BC]' => array(false, false, false, null, null, ''), 'An empty string empty_value is used if placeholder is not set [maintains BC]' => array(false, false, false, null, '', ''), 'A non-empty string empty_value is used if placeholder is not set [maintains BC]' => array(false, false, false, null, 'bar', 'bar'), - 'A placeholder is not used if it is an empty string and empty_value is set to false [mainatins BC]' => array(false, false, false, '', false, null), + 'A placeholder is not used if it is an empty string and empty_value is set to false [maintains BC]' => array(false, false, false, '', false, null), 'An unset empty_value is automatically made an empty string in a non-required field (but null is expected here) [maintains BC]' => array(false, false, false, '', null, null), 'An empty string empty_value is used if placeholder is also an empty string [maintains BC]' => array(false, false, false, '', '', ''), 'A non-empty string empty_value is used if placeholder is an empty string [maintains BC]' => array(false, false, false, '', 'bar', 'bar'), @@ -1781,7 +1781,7 @@ public function getOptionsWithPlaceholderAndEmptyValue() 'A placeholder is not used if empty_value is not set [maintains BC]' => array(false, false, true, null, null, null), 'An empty string empty_value is used if placeholder is not set [maintains BC]' => array(false, false, true, null, '', ''), 'A non-empty string empty_value is used if placeholder is not set [maintains BC]' => array(false, false, true, null, 'bar', 'bar'), - 'A placeholder is not used if it is an empty string and empty_value is set to false [mainatins BC]' => array(false, false, true, '', false, null), + 'A placeholder is not used if it is an empty string and empty_value is set to false [maintains BC]' => array(false, false, true, '', false, null), 'A placeholder is not used if empty_value is not set [maintains BC]' => array(false, false, true, '', null, null), 'An empty string empty_value is used if placeholder is also an empty string [maintains BC]' => array(false, false, true, '', '', ''), 'A non-empty string empty_value is used if placeholder is an empty string [maintains BC]' => array(false, false, true, '', 'bar', 'bar'), @@ -1798,7 +1798,7 @@ public function getOptionsWithPlaceholderAndEmptyValue() 'An unset empty_value is automaticaly made an empty string in a non-required field (but null is expected here) [maintains BC]' => array(false, true, false, null, null, null), 'An empty string empty_value is converted to "None" in an expanded single choice field [maintains BC]' => array(false, true, false, null, '', 'None'), 'A non-empty string empty_value is used if placeholder is not set [maintains BC]' => array(false, true, false, null, 'bar', 'bar'), - 'A placeholder is not used if it is an empty string and empty_value is set to false [mainatins BC]' => array(false, true, false, '', false, null), + 'A placeholder is not used if it is an empty string and empty_value is set to false [maintains BC]' => array(false, true, false, '', false, null), 'An unset empty_value is automatically made an empty string in a non-required field (but null is expected here) [maintains BC]' => array(false, true, false, '', null, null), 'An empty string empty_value is converted to "None" in an expanded single choice field [maintains BC]' => array(false, true, false, '', '', 'None'), 'A non-empty string empty_value is used if placeholder is an empty string [maintains BC]' => array(false, true, false, '', 'bar', 'bar'), @@ -1815,7 +1815,7 @@ public function getOptionsWithPlaceholderAndEmptyValue() 'A placeholder is not used if empty_value is not set [maintains BC]' => array(false, true, true, null, null, null), 'An empty string empty_value is converted to "None" in an expanded single choice field [maintains BC]' => array(false, true, true, null, '', 'None'), 'A non-empty string empty_value is used if placeholder is not set [maintains BC]' => array(false, true, true, null, 'bar', 'bar'), - 'A placeholder is not used if it is an empty string and empty_value is set to false [mainatins BC]' => array(false, true, true, '', false, null), + 'A placeholder is not used if it is an empty string and empty_value is set to false [maintains BC]' => array(false, true, true, '', false, null), 'A placeholder is not used if empty_value is not set [maintains BC]' => array(false, true, true, '', null, null), 'An empty string empty_value is converted to "None" in an expanded single choice field [maintains BC]' => array(false, true, true, '', '', 'None'), 'A non-empty string empty_value is used if placeholder is an empty string [maintains BC]' => array(false, true, true, '', 'bar', 'bar'), From a5d4e3fb639b6bb14ff9edddf855f90bea7bb688 Mon Sep 17 00:00:00 2001 From: boite Date: Mon, 29 Feb 2016 19:24:17 +0000 Subject: [PATCH 4/4] Collapse multi-line test method sig. --- .../Form/Tests/Extension/Core/Type/ChoiceTypeTest.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php index 0ab634a88db9d..ed64858bd570e 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php @@ -1730,14 +1730,8 @@ public function getOptionsWithPlaceholder() * @dataProvider getOptionsWithPlaceholderAndEmptyValue * @group legacy */ - public function testPlaceholderOptionWithEmptyValueOption( - $multiple, - $expanded, - $required, - $placeholder, - $emptyValue, - $viewValue - ) { + public function testPlaceholderOptionWithEmptyValueOption($multiple, $expanded, $required, $placeholder, $emptyValue, $viewValue) + { $form = $this->factory->create('choice', null, array( 'multiple' => $multiple, 'expanded' => $expanded,