[Form] Remove support for ArrayObject as ChoiceField's choices option

Internally, ChoiceField expects both choices and preferred_choices to be a simple array, so I replaced incomplete bits of code that attempted to not modify a possible ArrayObject and instead added type checks in the configure() method (with unit tests for expected exceptions).
symfony · Sep 10, 2010 · 57c0ce0 · 57c0ce0
1 parent 226277f
commit 57c0ce0
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 14 deletions.
diff --git a/src/Symfony/Component/Form/ChoiceField.php b/src/Symfony/Component/Form/ChoiceField.php
@@ -2,6 +2,8 @@
 
 namespace Symfony\Component\Form;
 
+use Symfony\Component\Form\Exception\UnexpectedTypeException;
+
 /**
  * Lets the user select between different choices
  *
@@ -28,24 +30,28 @@ protected function configure()
         $this->addOption('empty_value', '');
         $this->addOption('translate_choices', false);
 
+        if (!is_array($this->getOption('choices'))) {
+            throw new UnexpectedTypeException('The choices option must be an array');
+        }
+
+        if (!is_array($this->getOption('preferred_choices'))) {
+            throw new UnexpectedTypeException('The preferred_choices option must be an array');
+        }
+
         if (count($this->getOption('preferred_choices')) > 0) {
             $this->preferredChoices = array_flip($this->getOption('preferred_choices'));
-
-            if (false && $diff = array_diff_key($this->options, $this->knownOptions)) {
-                //throw new InvalidOptionsException(sprintf('%s does not support the following options: "%s".', get_class($this), implode('", "', array_keys($diff))), array_keys($diff));
-            }
         }
 
         if ($this->getOption('expanded')) {
             $this->setFieldMode(self::GROUP);
 
             $choices = $this->getOption('choices');
 
-            foreach ($this->getOption('preferred_choices') as $choice) {
+            foreach ($this->preferredChoices as $choice => $_) {
                 $this->add($this->newChoiceField($choice, $choices[$choice]));
             }
 
-            foreach ($this->getOption('choices') as $choice => $value) {
+            foreach ($choices as $choice => $value) {
                 if (!isset($this->preferredChoices[$choice])) {
                     $this->add($this->newChoiceField($choice, $value));
                 }

diff --git a/tests/Symfony/Tests/Component/Form/ChoiceFieldTest.php b/tests/Symfony/Tests/Component/Form/ChoiceFieldTest.php
@@ -3,6 +3,7 @@
 namespace Symfony\Tests\Component\Form;
 
 use Symfony\Component\Form\ChoiceField;
+use Symfony\Component\Form\Exception\UnexpectedTypeException;
 
 class ChoiceFieldTest extends \PHPUnit_Framework_TestCase
 {
@@ -36,18 +37,25 @@ class ChoiceFieldTest extends \PHPUnit_Framework_TestCase
         4 => 'Roman',
     );
 
-    public function testConfigureChoicesWithArrayObject()
+    /**
+     * @expectedException Symfony\Component\Form\Exception\UnexpectedTypeException
+     */
+    public function testConfigureChoicesWithNonArray()
     {
-        $choices = new \ArrayObject($this->choices);
-
         $field = new ChoiceField('name', array(
-            'multiple' => false,
-            'expanded' => true,
-            'choices' => $choices,
-            'preferred_choices' => $this->preferredChoices,
+            'choices' => new \ArrayObject(),
         ));
+    }
 
-        $this->assertEquals($this->choices, $choices->getArrayCopy());
+    /**
+     * @expectedException Symfony\Component\Form\Exception\UnexpectedTypeException
+     */
+    public function testConfigurePreferredChoicesWithNonArray()
+    {
+        $field = new ChoiceField('name', array(
+            'choices' => $this->choices,
+            'preferred_choices' => new \ArrayObject(),
+        ));
     }
 
     public function testBindSingleNonExpanded()