[Form] fix choice value "false" in ChoiceType

symfony · fabpot · Feb 26, 2016 · Feb 11, 2016 · Feb 19, 2016 · Feb 22, 2016
commit 3eac4693ad8e63283bb715375ab690999717d92b
diff --git a/src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php b/src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php
@@ -76,7 +76,7 @@ public function __construct($choices, $value = null)
 
         if (null === $value && $this->castableToString($choices)) {
             $value = function ($choice) {
-                return (string) $choice;
+                return false === $choice ? '0' : (string) $choice;
             };
         }
 
@@ -235,11 +235,11 @@ private function castableToString(array $choices, array &$cache = array())
                 continue;
             } elseif (!is_scalar($choice)) {
                 return false;
-            } elseif (isset($cache[(string) $choice])) {
+            } elseif (isset($cache[$choice])) {
                 return false;
             }
 
-            $cache[(string) $choice] = true;
+            $cache[$choice] = true;
         }
 
         return true;

diff --git a/src/Symfony/Component/Form/Extension/Core/DataTransformer/ChoiceToValueTransformer.php b/src/Symfony/Component/Form/Extension/Core/DataTransformer/ChoiceToValueTransformer.php
@@ -39,8 +39,8 @@ public function transform($choice)
 
     public function reverseTransform($value)
     {
-        if (null !== $value && !is_scalar($value)) {
-            throw new TransformationFailedException('Expected a scalar.');
+        if (null !== $value && !is_string($value)) {
+            throw new TransformationFailedException('Expected a string or null.');
         }
 
         $choices = $this->choiceList->getChoicesForValues(array((string) $value));

diff --git a/src/Symfony/Component/Form/Tests/ChoiceList/ArrayChoiceListTest.php b/src/Symfony/Component/Form/Tests/ChoiceList/ArrayChoiceListTest.php
@@ -137,4 +137,29 @@ public function testGetChoicesForValuesWithContainingNull()
 
         $this->assertSame(array(0 => null), $choiceList->getChoicesForValues(array('0')));
     }
+
+    public function testGetChoicesForValuesWithContainingFalseAndNull()
+    {
+        $choiceList = new ArrayChoiceList(array('False' => false, 'Null' => null));
+
+        $this->assertSame(array(0 => null), $choiceList->getChoicesForValues(array('1')));
+        $this->assertSame(array(0 => false), $choiceList->getChoicesForValues(array('0')));
+    }
+
+    public function testGetChoicesForValuesWithContainingEmptyStringAndNull()
+    {
+        $choiceList = new ArrayChoiceList(array('Empty String' => '', 'Null' => null));
+
+        $this->assertSame(array(0 => ''), $choiceList->getChoicesForValues(array('0')));
+        $this->assertSame(array(0 => null), $choiceList->getChoicesForValues(array('1')));
+    }
+
+    public function testGetChoicesForValuesWithContainingEmptyStringAndBooleans()
+    {
+        $choiceList = new ArrayChoiceList(array('Empty String' => '', 'True' => true, 'False' => false));
+
+        $this->assertSame(array(0 => ''), $choiceList->getChoicesForValues(array('')));
+        $this->assertSame(array(0 => true), $choiceList->getChoicesForValues(array('1')));
+        $this->assertSame(array(0 => false), $choiceList->getChoicesForValues(array('0')));
+    }
 }
diff --git a/...fony/Component/Form/Tests/Extension/Core/DataTransformer/ChoiceToValueTransformerTest.php b/...fony/Component/Form/Tests/Extension/Core/DataTransformer/ChoiceToValueTransformerTest.php
@@ -17,60 +17,80 @@
 class ChoiceToValueTransformerTest extends \PHPUnit_Framework_TestCase
 {
     protected $transformer;
+    protected $transformerWithNull;
 
     protected function setUp()
     {
-        $list = new ArrayChoiceList(array('', false, 'X'));
+        $list = new ArrayChoiceList(array('', false, 'X', true));
+        $listWithNull = new ArrayChoiceList(array('', false, 'X', null));
 
         $this->transformer = new ChoiceToValueTransformer($list);
+        $this->transformerWithNull = new ChoiceToValueTransformer($listWithNull);
     }
 
     protected function tearDown()
     {
         $this->transformer = null;
+        $this->transformerWithNull = null;
     }
 
     public function transformProvider()
     {
         return array(
             // more extensive test set can be found in FormUtilTest
-            array('', '0'),
-            array(false, '1'),
+            array('', '', '', '0'),
+            array(false, '0', false, '1'),
+            array('X', 'X', 'X', '2'),
+            array(true, '1', null, '3'),
         );
     }
 
     /**
      * @dataProvider transformProvider
      */
-    public function testTransform($in, $out)
+    public function testTransform($in, $out, $inWithNull, $outWithNull)
     {
         $this->assertSame($out, $this->transformer->transform($in));
+        $this->assertSame($outWithNull, $this->transformerWithNull->transform($inWithNull));
     }
 
     public function reverseTransformProvider()
     {
         return array(
             // values are expected to be valid choice keys already and stay
             // the same
-            array('0', ''),
-            array('1', false),
-            array('2', 'X'),
+            array('', '', '0', ''),
+            array('0', false, '1', false),
+            array('X', 'X', '2', 'X'),
+            array('1', true, '3', null),
         );
     }
 
     /**
      * @dataProvider reverseTransformProvider
      */
-    public function testReverseTransform($in, $out)
+    public function testReverseTransform($in, $out, $inWithNull, $outWithNull)
     {
         $this->assertSame($out, $this->transformer->reverseTransform($in));
+        $this->assertSame($outWithNull, $this->transformerWithNull->reverseTransform($inWithNull));
+    }
+
+    public function reverseTransformExpectsStringOrNullProvider()
+    {
+        return array(
+            array(0),
+            array(true),
+            array(false),
+            array(array()),
+        );
     }
 
     /**
+     * @dataProvider reverseTransformExpectsStringOrNullProvider
      * @expectedException \Symfony\Component\Form\Exception\TransformationFailedException
      */
-    public function testReverseTransformExpectsScalar()
+    public function testReverseTransformExpectsStringOrNull($value)
     {
-        $this->transformer->reverseTransform(array());
+        $this->transformer->reverseTransform($value);
     }
 }
diff --git a/...ny/Component/Form/Tests/Extension/Core/DataTransformer/ChoicesToValuesTransformerTest.php b/...ny/Component/Form/Tests/Extension/Core/DataTransformer/ChoicesToValuesTransformerTest.php
@@ -17,24 +17,34 @@
 class ChoicesToValuesTransformerTest extends \PHPUnit_Framework_TestCase
 {
     protected $transformer;
+    protected $transformerWithNull;
 
     protected function setUp()
     {
         $list = new ArrayChoiceList(array('', false, 'X'));
+        $listWithNull = new ArrayChoiceList(array('', false, 'X', null));
+
         $this->transformer = new ChoicesToValuesTransformer($list);
+        $this->transformerWithNull = new ChoicesToValuesTransformer($listWithNull);
     }
 
     protected function tearDown()
     {
         $this->transformer = null;
+        $this->transformerWithNull = null;
     }
 
     public function testTransform()
     {
         $in = array('', false, 'X');
-        $out = array('0', '1', '2');
+        $out = array('', '0', 'X');
 
         $this->assertSame($out, $this->transformer->transform($in));
+
+        $in[] = null;
+        $outWithNull = array('0', '1', '2', '3');
+
+        $this->assertSame($outWithNull, $this->transformerWithNull->transform($in));
     }
 
     public function testTransformNull()
@@ -53,15 +63,21 @@ public function testTransformExpectsArray()
     public function testReverseTransform()
     {
         // values are expected to be valid choices and stay the same
-        $in = array('0', '1', '2');
+        $in = array('', '0', 'X');
         $out = array('', false, 'X');
 
         $this->assertSame($out, $this->transformer->reverseTransform($in));
+        // values are expected to be valid choices and stay the same
+        $inWithNull = array('0','1','2','3');
+        $out[] = null;
+
+        $this->assertSame($out, $this->transformerWithNull->reverseTransform($inWithNull));
     }
 
     public function testReverseTransformNull()
     {
         $this->assertSame(array(), $this->transformer->reverseTransform(null));
+        $this->assertSame(array(), $this->transformerWithNull->reverseTransform(null));
     }
 
     /**

diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php
@@ -26,6 +26,12 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         'Roman' => 'e',
     );
 
+    private $scalarChoices = array(
+        'Yes' => true,
+        'No' => false,
+        'n/a' => '',
+    );
+
     private $numericChoicesFlipped = array(
         0 => 'Bernhard',
         1 => 'Fabien',
@@ -139,6 +145,58 @@ public function testExpandedFlippedChoicesOptionsTurnIntoChildren()
         $this->assertCount(count($this->choices), $form, 'Each choice should become a new field');
     }
 
+    public function testChoiceListWithScalarValues()
+    {
+        $view = $this->factory->create('choice', null, array(
+            'choices' => $this->scalarChoices,
+            'choices_as_values' => true,
+        ))->createView();
+
+        $this->assertSame('1', $view->vars['choices'][0]->value);
+        $this->assertSame('0', $view->vars['choices'][1]->value);
+        $this->assertSame('', $view->vars['choices'][2]->value);
+        $this->assertFalse($view->vars['is_selected']($view->vars['choices'][0], $view->vars['value']), 'True value should not be pre selected');
+        $this->assertFalse($view->vars['is_selected']($view->vars['choices'][1], $view->vars['value']), 'False value should not be pre selected');
+        $this->assertFalse($view->vars['is_selected']($view->vars['choices'][2], $view->vars['value']), 'Empty value should not be pre selected');
+    }
+
+    public function testChoiceListWithScalarValuesAndFalseAsPreSetData()
+    {
+        $view = $this->factory->create('choice', false, array(
+            'choices' => $this->scalarChoices,
+            'choices_as_values' => true,
+        ))->createView();
+
+        $this->assertTrue($view->vars['is_selected']($view->vars['choices'][1]->value, $view->vars['value']), 'False value should be pre selected');
+    }
+
+    public function testExpandedChoiceListWithScalarValues()
+    {
+        $view = $this->factory->create('choice', null, array(
+            'choices' => $this->scalarChoices,
+            'choices_as_values' => true,
+            'expanded' => true,
+        ))->createView();
+
+        $this->assertFalse($view->children[0]->vars['checked'], 'True value should not be pre selected');
+        $this->assertFalse($view->children[1]->vars['checked'], 'False value should not be pre selected');
+        $this->assertTrue($view->children[2]->vars['checked'], 'Empty value should be pre selected');
+    }
+
+    public function testExpandedChoiceListWithScalarValuesAndFalseAsPreSetData()
+    {
+        $view = $this->factory->create('choice', false, array(
+            &#
F438
39;choices' => $this->scalarChoices,
+            'choices_as_values' => true,
+            'expanded' => true,
+        ))->createView();
+
+        $this->assertSame('1', $view->vars['choices'][0]->value);
+        $this->assertSame('0', $view->vars['choices'][1]->value);
+        $this->assertTrue($view->children[1]->vars['checked'], 'False value should be pre selected');
+        $this->assertFalse($view->children[2]->vars['checked'], 'Empty value should not be pre selected');
+    }
+
     public function testPlaceholderPresentOnNonRequiredExpandedSingleChoice()
     {
         $form = $this->factory->create('choice', null, array(
@@ -198,6 +256,100 @@ public function testPlaceholderNotPresentIfEmptyChoice()
         $this->assertCount(2, $form, 'Each choice should become a new field');
     }
 
+    public function testPlaceholderWithBooleanChoices()
+    {
+        $form = $this->factory->create('choice', null, array(
+            'multiple' => false,
+            'expanded' => false,
+            'required' => false,
+            'choices' => array(
+                'Yes' => true,
+                'No' => false,
+            ),
+            'placeholder' => 'Select an option',
+            'choices_as_values' => true,
+        ));
+
+        $view = $form->createView();
+
+        $this->assertSame('', $view->vars['value'], 'Value should be empty');
+        $this->assertSame('1', $view->vars['choices'][0]->value);
+        $this->assertSame('0', $view->vars['choices'][1]->value, 'Choice "false" should have "0" as value');
+        $this->assertFalse($view->vars['is_selected']($view->vars['choices'][1]->value, $view->vars['value']), 'Choice "false" should not be selected');
+    }
+
+    public function testPlaceholderWithBooleanChoicesWithFalseAsPreSetData()
+    {
+        $form = $this->factory->create('choice', false, array(
+            'multiple' => false,
+            'expanded' => false,
+            'required' => false,
+            'choices' => array(
+                'Yes' => true,
+                'No' => false,
+            ),
+            'placeholder' => 'Select an option',
+            'choices_as_values' => true,
+        ));
+
+        $view = $form->createView();
+
+        $this->assertSame('0', $view->vars['value'], 'Value should be "0"');
+        $this->assertSame('1', $view->vars['choices'][0]->value);
+        $this->assertSame('0', $view->vars['choices'][1]->value, 'Choice "false" should have "0" as value');
+        $this->assertTrue($view->vars['is_selected']($view->vars['choices'][1]->value, $view->vars['value']), 'Choice "false" should be selected');
+    }
+
+    public function testPlaceholderWithExpandedBooleanChoices()
+    {
+        $form = $this->factory->create('choice', null, array(
+            'multiple' => false,
+            'expanded' => true,
+            'required' => false,
+            'choices' => array(
+                'Yes' => true,
+                'No' => false,
+            ),
+            'placeholder' => 'Select an option',
+            'choices_as_values' => true,
+        ));
+
+        $this->assertTrue(isset($form['placeholder']), 'Placeholder should be set');
+        $this->assertCount(3, $form, 'Each choice should become a new field, placeholder included');
+
+        $view = $form->createView();
+
+        $this->assertSame('', $view->vars['value'], 'Value should be empty');
+        $this->assertSame('1', $view->vars['choices'][0]->value);
+        $this->assertSame('0', $view->vars['choices'][1]->value, 'Choice "false" should have "0" as value');
+        $this->assertFalse($view->children[1]->vars['checked'], 'Choice "false" should not be selected');
+    }
+
+    public function testPlaceholderWithExpandedBooleanChoicesAndWithFalseAsPreSetData()
+    {
+        $form = $this->factory->create('choice', false, array(
+            'multiple' => false,
+            'expanded' => true,
+            'required' => false,
+            'choices' => array(
+                'Yes' => true,
+                'No' => false,
+            ),
+            'placeholder' => 'Select an option',
+            'choices_as_values' => true,
+        ));
+
+        $this->assertTrue(isset($form['placeholder']), 'Placeholder should be set');
+        $this->assertCount(3, $form, 'Each choice should become a new field, placeholder included');
+
+        $view = $form->createView();
+
+        $this->assertSame('0', $view->vars['value'], 'Value should be "0"');
+        $this->assertSame('1', $view->vars['choices'][0]->value);
+        $this->assertSame('0', $view->vars['choices'][1]->value, 'Choice "false" should have "0" as value');
+        $this->assertTrue($view->children[1]->vars['checked'], 'Choice "false" should be selected');
+    }
+
     public function testExpandedChoicesOptionsAreFlattened()
     {
         $form = $this->factory->create('choice', null, array(