[Form] refactor CheckboxListMapper and RadioListMapper

fixes #14712 and #17789.

`ChoiceType` now always use `ChoiceToValueTransformer` or
`ChoicesToValuesTransformer` depending on `multiple` option.
Hence `CheckboxListMapper` and `RadioListMapper` don’t handle
the transformation anymore.
Fixes pre selection of choice with model values such as `null`,
`false` or empty string.

src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php

@@ -261,7 +261,7 @@ class EntityTypeTest extends TypeTestCase
         $field->submit(null);
 
         $this->assertNull($field->getData());
-        $this->assertNull($field->getViewData());
+        $this->assertSame('', $field->getViewData(), 'View data is always a string');
     }
 
     public function testSubmitSingleNonExpandedNull()

src/Symfony/Bridge/Doctrine/composer.json

@@ -22,7 +22,7 @@
     "require-dev": {
         "symfony/stopwatch": "~2.2",
         "symfony/dependency-injection": "~2.2",
-        "symfony/form": "~2.7,>=2.7.1",
+        "symfony/form": "~2.7.12|~2.8.5",
         "symfony/http-kernel": "~2.2",
         "symfony/property-access": "~2.3",
         "symfony/security": "~2.2",

src/Symfony/Component/Form/Extension/Core/DataMapper/CheckboxListMapper.php

@@ -11,9 +11,8 @@
 
 namespace Symfony\Component\Form\Extension\Core\DataMapper;
 
-use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
 use Symfony\Component\Form\DataMapperInterface;
-use Symfony\Component\Form\Exception\TransformationFailedException;
+use Symfony\Component\Form\Exception\UnexpectedTypeException;
 
 /**
  * Maps choices to/from checkbox forms.
@@ -26,16 +25,6 @@ use Symfony\Component\Form\Exception\TransformationFailedException;
  */
 class CheckboxListMapper implements DataMapperInterface
 {
-    /**
-     * @var ChoiceListInterface
-     */
-    private $choiceList;
-
-    public function __construct(ChoiceListInterface $choiceList)
-    {
-        $this->choiceList = $choiceList;
-    }
-
     /**
      * {@inheritdoc}
      */
@@ -46,22 +35,12 @@ class CheckboxListMapper implements DataMapperInterface
         }
 
         if (!is_array($choices)) {
-            throw new TransformationFailedException('Expected an array.');
+            throw new UnexpectedTypeException($choices, 'array');
-        }
-
-        try {
-            $valueMap = array_flip($this->choiceList->getValuesForChoices($choices));
-        } catch (\Exception $e) {
-            throw new TransformationFailedException(
-                'Can not read the choices from the choice list.',
-                $e->getCode(),
-                $e
-            );
         }
 
         foreach ($checkboxes as $checkbox) {
             $value = $checkbox->getConfig()->getOption('value');
-            $checkbox->setData(isset($valueMap[$value]) ? true : false);
+            $checkbox->setData(in_array($value, $choices, true));
         }
     }
 
@@ -70,6 +49,10 @@ class CheckboxListMapper implements DataMapperInterface
      */
     public function mapFormsToData($checkboxes, &$choices)
     {
+        if (!is_array($choices)) {
+            throw new UnexpectedTypeException($choices, 'array');
+        }
+
         $values = array();
 
         foreach ($checkboxes as $checkbox) {
@@ -79,14 +62,6 @@ class CheckboxListMapper implements DataMapperInterface
             }
         }
 
-        try {
+        $choices = $values;
-            $choices = $this->choiceList->getChoicesForValues($values);
-        } catch (\Exception $e) {
-            throw new TransformationFailedException(
-                'Can not read the values from the choice list.',
-                $e->getCode(),
-                $e
-            );
-        }
     }
 }

src/Symfony/Component/Form/Extension/Core/DataMapper/RadioListMapper.php

@@ -11,8 +11,8 @@
 
 namespace Symfony\Component\Form\Extension\Core\DataMapper;
 
-use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
 use Symfony\Component\Form\DataMapperInterface;
+use Symfony\Component\Form\Exception\UnexpectedTypeException;
 
 /**
  * Maps choices to/from radio forms.
@@ -25,26 +25,18 @@ use Symfony\Component\Form\DataMapperInterface;
  */
 class RadioListMapper implements DataMapperInterface
 {
-    /**
-     * @var ChoiceListInterface
-     */
-    private $choiceList;
-
-    public function __construct(ChoiceListInterface $choiceList)
-    {
-        $this->choiceList = $choiceList;
-    }
-
     /**
      * {@inheritdoc}
      */
     public function mapDataToForms($choice, $radios)
     {
-        $valueMap = array_flip($this->choiceList->getValuesForChoices(array($choice)));
+        if (!is_string($choice)) {
+            throw new UnexpectedTypeException($choice, 'string');
+        }
 
         foreach ($radios as $radio) {
             $value = $radio->getConfig()->getOption('value');
-            $radio->setData(isset($valueMap[$value]) ? true : false);
+            $radio->setData($choice === $value);
         }
     }
 
@@ -53,6 +45,10 @@ class RadioListMapper implements DataMapperInterface
      */
     public function mapFormsToData($radios, &$choice)
     {
+        if (null !== $choice && !is_string($choice)) {
+            throw new UnexpectedTypeException($choice, 'null or string');
+        }
+
         $choice = null;
 
         foreach ($radios as $radio) {
@@ -61,8 +57,7 @@ class RadioListMapper implements DataMapperInterface
                     return;
                 }
 
-                $value = $radio->getConfig()->getOption('value');
+                $choice = $radio->getConfig()->getOption('value');
-                $choice = current($this->choiceList->getChoicesForValues(array($value)));
 
                 return;
             }

src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

@@ -60,9 +60,7 @@ class ChoiceType extends AbstractType
     public function buildForm(FormBuilderInterface $builder, array $options)
     {
         if ($options['expanded']) {
-            $builder->setDataMapper($options['multiple']
+            $builder->setDataMapper($options['multiple'] ? new CheckboxListMapper() : new RadioListMapper());
-                ? new CheckboxListMapper($options['choice_list'])
-                : new RadioListMapper($options['choice_list']));
 
             // Initialize all choices before doing the index check below.
             // This helps in cases where index checks are optimized for non
@@ -133,22 +131,13 @@ class ChoiceType extends AbstractType
 
                 $event->setData($data);
             });
+        }
 
-            if (!$options['multiple']) {
+        if ($options['multiple']) {
-                // For radio lists, transform empty arrays to null
+            // <select> tag with "multiple" option or list of checkbox inputs
-                // This is kind of a hack necessary because the RadioListMapper
-                // is not invoked for forms without choices
-                $builder->addEventListener(FormEvents::SUBMIT, function (FormEvent $event) {
-                    if (array() === $event->getData()) {
-                        $event->setData(null);
-                    }
-                });
-            }
-        } elseif ($options['multiple']) {
-            // <select> tag with "multiple" option
             $builder->addViewTransformer(new ChoicesToValuesTransformer($options['choice_list']));
         } else {
-            // <select> tag without "multiple" option
+            // <select> tag without "multiple" option or list of radio inputs
             $builder->addViewTransformer(new ChoiceToValueTransformer($options['choice_list']));
         }
 
@@ -247,7 +236,11 @@ class ChoiceType extends AbstractType
         $choiceListFactory = $this->choiceListFactory;
 
         $emptyData = function (Options $options) {
-            if ($options['multiple'] || $options['expanded']) {
+            if ($options['expanded'] && !$options['multiple']) {
+                return;
+            }
+
+            if ($options['multiple']) {
                 return array();
             }
 

src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php

@@ -32,6 +32,12 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         'n/a' => '',
     );
 
+    private $booleanChoicesWithNull = array(
+        'Yes' => true,
+        'No' => false,
+        'n/a' => null,
+    );
+
     private $numericChoicesFlipped = array(
         0 => 'Bernhard',
         1 => 'Fabien',
@@ -183,6 +189,19 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $this->assertTrue($view->children[2]->vars['checked'], 'Empty value should be pre selected');
     }
 
+    public function testExpandedChoiceListWithBooleanAndNullValues()
+    {
+        $view = $this->factory->create('choice', null, array(
+            'choices' => $this->booleanChoicesWithNull,
+            '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(
@@ -197,6 +216,19 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $this->assertFalse($view->children[2]->vars['checked'], 'Empty value should not be pre selected');
     }
 
+    public function testExpandedChoiceListWithBooleanAndNullValuesAndFalseAsPreSetData()
+    {
+        $view = $this->factory->create('choice', false, array(
+            'choices' => $this->booleanChoicesWithNull,
+            'choices_as_values' => true,
+            'expanded' => true,
+        ))->createView();
+
+        $this->assertFalse($view->children[0]->vars['checked'], 'True value should not be pre selected');
+        $this->assertTrue($view->children[1]->vars['checked'], 'False value should be pre selected');
+        $this->assertFalse($view->children[2]->vars['checked'], 'Null value should not be pre selected');
+    }
+
     public function testPlaceholderPresentOnNonRequiredExpandedSingleChoice()
     {
         $form = $this->factory->create('choice', null, array(
@@ -319,7 +351,7 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
 
         $view = $form->createView();
 
-        $this->assertSame('', $view->vars['value'], 'Value should be empty');
+        $this->assertSame('', $view->vars['value'], 'Value should be an empty string');
         $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');
@@ -940,8 +972,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $form->submit(null);
 
         $this->assertNull($form->getData());
-        $this->assertNull($form->getViewData());
+        $this->assertSame('', $form->getViewData(), 'View data should always be a string');
-        $this->assertEmpty($form->getExtraData());
+        $this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
         $this->assertTrue($form->isSynchronized());
 
         $this->assertFalse($form[0]->getData());
@@ -972,8 +1004,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $form->submit(null);
 
         $this->assertNull($form->getData());
-        $this->assertNull($form->getViewData());
+        $this->assertSame('', $form->getViewData(), 'View data should always be a string');
-        $this->assertEmpty($form->getExtraData());
+        $this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
         $this->assertTrue($form->isSynchronized());
     }
 
@@ -990,8 +1022,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $form->submit('');
 
         $this->assertNull($form->getData());
-        $this->assertNull($form->getViewData());
+        $this->assertSame('', $form->getViewData(), 'View data should always be a string');
-        $this->assertEmpty($form->getExtraData());
+        $this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
         $this->assertTrue($form->isSynchronized());
 
         $this->assertFalse($form[0]->getData());
@@ -1022,8 +1054,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $form->submit('');
 
         $this->assertNull($form->getData());
-        $this->assertNull($form->getViewData());
+        $this->assertSame('', $form->getViewData(), 'View data should always be a string');
-        $this->assertEmpty($form->getExtraData());
+        $this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
         $this->assertTrue($form->isSynchronized());
     }
 
@@ -1040,8 +1072,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $form->submit(false);
 
         $this->assertNull($form->getData());
-        $this->assertNull($form->getViewData());
+        $this->assertSame('', $form->getViewData(), 'View data should always be a string');
-        $this->assertEmpty($form->getExtraData());
+        $this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
         $this->assertTrue($form->isSynchronized());
 
         $this->assertFalse($form[0]->getData());
@@ -1072,8 +1104,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $form->submit(false);
 
         $this->assertNull($form->getData());
-        $this->assertNull($form->getViewData());
+        $this->assertSame('', $form->getViewData(), 'View data should always be a string');
-        $this->assertEmpty($form->getExtraData());
+        $this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
         $this->assertTrue($form->isSynchronized());
     }
 
@@ -1090,8 +1122,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $form->submit(null);
 
         $this->assertNull($form->getData());
-        $this->assertNull($form->getViewData());
+        $this->assertSame('', $form->getViewData(), 'View data should always be a string');
-        $this->assertEmpty($form->getExtraData());
+        $this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
         $this->assertTrue($form->isSynchronized());
 
         $this->assertTrue($form['placeholder']->getData());
@@ -1124,8 +1156,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $form->submit(null);
 
         $this->assertNull($form->getData());
-        $this->assertNull($form->getViewData());
+        $this->assertSame('', $form->getViewData(), 'View data should always be a string');
-        $this->assertEmpty($form->getExtraData());
+        $this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
         $this->assertTrue($form->isSynchronized());
     }
 
@@ -1142,8 +1174,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $form->submit('');
 
         $this->assertNull($form->getData());
-        $this->assertNull($form->getViewData());
+        $this->assertSame('', $form->getViewData(), 'View data should always be a string');
-        $this->assertEmpty($form->getExtraData());
+        $this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
         $this->assertTrue($form->isSynchronized());
 
         $this->assertTrue($form['placeholder']->getData());
@@ -1176,8 +1208,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $form->submit('');
 
         $this->assertNull($form->getData());
-        $this->assertNull($form->getViewData());
+        $this->assertSame('', $form->getViewData(), 'View data should always be a string');
-        $this->assertEmpty($form->getExtraData());
+        $this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
         $this->assertTrue($form->isSynchronized());
     }
 
@@ -1194,8 +1226,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $form->submit(false);
 
         $this->assertNull($form->getData());
-        $this->assertNull($form->getViewData());
+        $this->assertSame('', $form->getViewData(), 'View data should always be a string');
-        $this->assertEmpty($form->getExtraData());
+        $this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
         $this->assertTrue($form->isSynchronized());
 
         $this->assertTrue($form['placeholder']->getData());
@@ -1228,8 +1260,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $form->submit(false);
 
         $this->assertNull($form->getData());
-        $this->assertNull($form->getViewData());
+        $this->assertSame('', $form->getViewData(), 'View data should always be a string');
-        $this->assertEmpty($form->getExtraData());
+        $this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
         $this->assertTrue($form->isSynchronized());
     }
 
@@ -1247,7 +1279,7 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
 
         $form->submit('');
 
-        $this->assertNull($form->getData());
+        $this->assertSame('', $form->getData());
         $this->assertTrue($form->isSynchronized());
 
         $this->assertTrue($form[0]->getData());