[Form] Moved option 'empty_value' to ChoiceField. An empty value is displayed if the field is not required.

src/Symfony/Component/Form/ChoiceField.php

@@ -21,6 +21,9 @@ use Symfony\Component\Form\Exception\InvalidOptionsException;
  *  * choices:              An array of key-value pairs that will represent the choices
  *  * preferred_choices:    An array of choices (by key) that should be displayed
  *                          above all other options in the field
+ *  * empty_value:          If set to a non-false value, an "empty" option will
+ *                          be added to the top of the countries choices. A
+ *                          common value might be "Choose a country". Default: false.
  *
  * The multiple and expanded options control exactly which HTML element
  * that should be used to render this field:
@@ -42,10 +45,10 @@ class ChoiceField extends HybridField
 
     /**
      * Stores the choices
-     * You should only access this property through getInitializedChoices()
+     * You should only access this property through getChoices()
      * @var array
      */
-    protected $initializedChoices = array();
+    private $choices = array();
 
     protected function configure()
     {
@@ -53,6 +56,7 @@ class ChoiceField extends HybridField
         $this->addOption('preferred_choices', array());
         $this->addOption('multiple', false);
         $this->addOption('expanded', false);
+        $this->addOption('empty_value', '');
 
         parent::configure();
 
@@ -73,7 +77,7 @@ class ChoiceField extends HybridField
         if ($this->isExpanded()) {
             $this->setFieldMode(self::GROUP);
 
-            $choices = $this->getInitializedChoices();
+            $choices = $this->getChoices();
 
             foreach ($this->preferredChoices as $choice => $_) {
                 $this->add($this->newChoiceField($choice, $choices[$choice]));
@@ -113,11 +117,29 @@ class ChoiceField extends HybridField
      */
     protected function initializeChoices()
     {
-        $this->initializedChoices = $this->getOption('choices');
+        if (!$this->choices) {
+            $this->choices = $this->getInitializedChoices();
+        }
+    }
+
+    protected function getInitializedChoices()
+    {
+        $choices = $this->getOption('choices');
+
+        if ($choices instanceof \Closure) {
+            $choices = $choices->__invoke();
+        }
 
-        if ($this->initializedChoices instanceof \Closure) {
-            $this->initializedChoices = $this->initializedChoices->__invoke();
+        // TESTME
+        if (!is_array($choices)) {
+            throw new InvalidOptionsException('The "choices" option must be an array or a closure returning an array', array('choices'));
+        }
+
+        if (!$this->isRequired()) {
+            $choices = array_merge(array('' => $this->getOption('empty_value')), $choices);
         }
+
+        return $choices;
     }
 
     /**
@@ -128,28 +150,26 @@ class ChoiceField extends HybridField
      *
      * @return array
      */
-    protected function getInitializedChoices()
+    protected function getChoices()
     {
-        if (!$this->initializedChoices) {
-            $this->initializeChoices();
-        }
+        $this->initializeChoices();
 
-        return $this->initializedChoices;
+        return $this->choices;
     }
 
     public function getPreferredChoices()
     {
-        return array_intersect_key($this->getInitializedChoices(), $this->preferredChoices);
+        return array_intersect_key($this->getChoices(), $this->preferredChoices);
     }
 
     public function getOtherChoices()
     {
-        return array_diff_key($this->getInitializedChoices(), $this->preferredChoices);
+        return array_diff_key($this->getChoices(), $this->preferredChoices);
     }
 
     public function getLabel($choice)
     {
-        $choices = $this->getInitializedChoices();
+        $choices = $this->getChoices();
 
         return isset($choices[$choice]) ? $choices[$choice] : null;
     }
@@ -225,7 +245,7 @@ class ChoiceField extends HybridField
     {
         if ($this->isExpanded()) {
             $value = parent::transform($value);
-            $choices = $this->getInitializedChoices();
+            $choices = $this->getChoices();
 
             foreach ($choices as $choice => $_) {
                 $choices[$choice] = $this->isMultipleChoice()
@@ -235,6 +255,7 @@ class ChoiceField extends HybridField
 
             return $choices;
         }
+
         return parent::transform($value);
     }
 
@@ -265,9 +286,10 @@ class ChoiceField extends HybridField
             if ($this->isMultipleChoice()) {
                 $value = $choices;
             } else {
-                $value =  count($choices) > 0 ? current($choices) : null;
+                $value = count($choices) > 0 ? current($choices) : null;
             }
         }
+
         return parent::reverseTransform($value);
     }
 }

src/Symfony/Component/Form/CountryField.php

@@ -16,13 +16,6 @@ use Symfony\Component\Locale\Locale;
 /**
  * A field for selecting from a list of countries.
  *
- * In addition to the ChoiceField options, this field has the following
- * options:
- *
- *  * empty_value:  If set to a non-false value, an "empty" option will
- *                  be added to the top of the countries choices. A
- *                  common value might be "Choose a country". Default: false.
- *
  * @see Symfony\Component\Form\ChoiceField
  * @author Bernhard Schussek <bernhard.schussek@symfony-project.com>
  */
@@ -30,15 +23,7 @@ class CountryField extends ChoiceField
 {
     protected function configure()
     {
-        $this->addOption('empty_value', false);
-
-        $choices = Locale::getDisplayCountries($this->locale);
-
-        if (false !== $this->getOption('empty_value')) {
-            $choices = array('' => $this->getOption('empty_value')) + $choices;
-        }
-
-        $this->addOption('choices', $choices);
+        $this->addOption('choices', Locale::getDisplayCountries($this->locale));
 
         parent::configure();
     }

src/Symfony/Component/Form/Field.php

@@ -88,7 +88,6 @@ class Field extends Configurable implements FieldInterface
 
         $this->normalizedData = $this->normalize($this->data);
         $this->transformedData = $this->transform($this->normalizedData);
-        $this->required = $this->getOption('required');
 
         $this->setPropertyPath($this->getOption('property_path'));
     }
@@ -184,9 +183,14 @@ class Field extends Configurable implements FieldInterface
      */
     public function isRequired()
     {
+        if (null === $this->required) {
+            $this->required = $this->getOption('required');
+        }
+
         if (null === $this->parent || $this->parent->isRequired()) {
             return $this->required;
         }
+
         return false;
     }
 

src/Symfony/Component/Form/LanguageField.php

@@ -16,13 +16,6 @@ use Symfony\Component\Locale\Locale;
 /**
  * A field for selecting from a list of languages.
  *
- * In addition to the ChoiceField options, this field has the following
- * options:
- *
- *  * empty_value:  If set to a non-false value, an "empty" option will
- *                  be added to the top of the languages choices. A
- *                  common value might be "Choose a language". Default: false.
- *
  * @see Symfony\Component\Form\ChoiceField
  * @author Bernhard Schussek <bernhard.schussek@symfony-project.com>
  */
@@ -33,15 +26,7 @@ class LanguageField extends ChoiceField
      */
     protected function configure()
     {
-        $this->addOption('empty_value', false);
-
-        $choices = Locale::getDisplayLanguages($this->locale);
-
-        if (false !== $this->getOption('empty_value')) {
-            $choices = array('' => $this->getOption('empty_value')) + $choices;
-        }
-
-        $this->addOption('choices', $choices);
+        $this->addOption('choices', Locale::getDisplayLanguages($this->locale));
 
         parent::configure();
     }

src/Symfony/Component/Form/LocaleField.php

@@ -16,13 +16,6 @@ use Symfony\Component\Locale\Locale;
 /**
  * A field for selecting from a list of locales.
  *
- * In addition to the ChoiceField options, this field has the following
- * options:
- *
- *  * empty_value:  If set to a non-false value, an "empty" option will
- *                  be added to the top of the locale choices. A
- *                  common value might be "Choose a locale". Default: false.
- *
  * @see Symfony\Component\Form\ChoiceField
  * @author Bernhard Schussek <bernhard.schussek@symfony-project.com>
  */
@@ -33,15 +26,7 @@ class LocaleField extends ChoiceField
      */
     protected function configure()
     {
-        $this->addOption('empty_value', false);
-
-        $choices = Locale::getDisplayLocales($this->locale);
-
-        if (false !== $this->getOption('empty_value')) {
-            $choices = array('' => $this->getOption('empty_value')) + $choices;
-        }
-
-        $this->addOption('choices', $choices);
+        $this->addOption('choices', Locale::getDisplayLocales($this->locale));
 
         parent::configure();
     }

src/Symfony/Component/Form/TimezoneField.php

@@ -14,11 +14,7 @@ namespace Symfony\Component\Form;
 /**
  * Represents a field where each timezone is broken down by continent.
  *
- * Available options:
- *
- *  * empty_value:   If set to a non-false value, an "empty" option will
- *                  be added to the top of the timezone choices. A
- *                  common value might be "Choose a timezone". Default: false.
+ * @author Bernhard Schussek <bernhard.schussek@symfony-project.com>
  */
 class TimezoneField extends ChoiceField
 {
@@ -33,15 +29,7 @@ class TimezoneField extends ChoiceField
      */
     public function configure()
     {
-        $this->addOption('empty_value', false);
-
-        $choices = self::getTimezoneChoices();
-
-        if (false !== $this->getOption('empty_value')) {
-            $choices = array('' => $this->getOption('empty_value')) + $choices;
-        }
-
-        $this->addOption('choices', $choices);
+        $this->addOption('choices', self::getTimezoneChoices());
 
         parent::configure();
     }

tests/Symfony/Tests/Component/Form/ChoiceFieldTest.php

@@ -87,6 +87,56 @@ class ChoiceFieldTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    /**
+     * @expectedException Symfony\Component\Form\Exception\InvalidOptionsException
+     */
+    public function testClosureShouldReturnArray()
+    {
+        $field = new ChoiceField('name', array(
+            'choices' => function () { return 'foobar'; },
+        ));
+
+        // trigger closure
+        $field->getOtherChoices();
+    }
+
+    public function testNonRequiredContainsEmptyField()
+    {
+        $field = new ChoiceField('name', array(
+            'multiple' => false,
+            'expanded' => false,
+            'choices' => $this->choices,
+            'required' => false,
+        ));
+
+        $this->assertEquals(array('' => '') + $this->choices, $field->getOtherChoices());
+    }
+
+    public function testRequiredContainsNoEmptyField()
+    {
+        $field = new ChoiceField('name', array(
+            'multiple' => false,
+            'expanded' => false,
+            'choices' => $this->choices,
+            'required' => true,
+        ));
+
+        $this->assertEquals($this->choices, $field->getOtherChoices());
+    }
+
+    public function testEmptyValueConfiguresLabelOfEmptyField()
+    {
+        $field = new ChoiceField('name', array(
+            'multiple' => false,
+            'expanded' => false,
+            'choices' => $this->choices,
+            'required' => false,
+            'empty_value' => 'Foobar',
+        ));
+
+        $this->assertEquals(array('' => 'Foobar') + $this->choices, $field->getOtherChoices());
+    }
+
     /**
      * @dataProvider getChoicesVariants
      */

tests/Symfony/Tests/Component/Form/CountryFieldTest.php

@@ -42,24 +42,4 @@ class CountryFieldTest extends \PHPUnit_Framework_TestCase
 
         $this->assertArrayNotHasKey('ZZ', $choices);
     }
-
-    public function testEmptyValueOption()
-    {
-        // empty_value false
-        $field = new CountryField('country', array('empty_value' => false));
-        $choices = $field->getOtherChoices();
-        $this->assertArrayNotHasKey('', $choices);
-
-        // empty_value as a blank string
-        $field = new CountryField('country', array('empty_value' => ''));
-        $choices = $field->getOtherChoices();
-        $this->assertArrayHasKey('', $choices);
-        $this->assertEquals('', $choices['']);
-
-        // empty_value as a normal string
-        $field = new CountryField('country', array('empty_value' => 'Choose a country'));
-        $choices = $field->getOtherChoices();
-        $this->assertArrayHasKey('', $choices);
-        $this->assertEquals('Choose a country', $choices['']);
-    }
 }

tests/Symfony/Tests/Component/Form/LanguageFieldTest.php

@@ -42,24 +42,4 @@ class LanguageFieldTest extends \PHPUnit_Framework_TestCase
 
         $this->assertArrayNotHasKey('mul', $choices);
     }
-
-    public function testEmptyValueOption()
-    {
-        // empty_value false
-        $field = new LanguageField('language', array('empty_value' => false));
-        $choices = $field->getOtherChoices();
-        $this->assertArrayNotHasKey('', $choices);
-
-        // empty_value as a blank string
-        $field = new LanguageField('language', array('empty_value' => ''));
-        $choices = $field->getOtherChoices();
-        $this->assertArrayHasKey('', $choices);
-        $this->assertEquals('', $choices['']);
-
-        // empty_value as a normal string
-        $field = new LanguageField('language', array('empty_value' => 'Choose a language'));
-        $choices = $field->getOtherChoices();
-        $this->assertArrayHasKey('', $choices);
-        $this->assertEquals('Choose a language', $choices['']);
-    }
 }

tests/Symfony/Tests/Component/Form/LocaleFieldTest.php

@@ -30,24 +30,4 @@ class LocaleFieldTest extends \PHPUnit_Framework_TestCase
         $this->assertArrayHasKey('zh_Hans_MO', $choices);
         $this->assertEquals('Chinesisch (vereinfacht, Sonderverwaltungszone Macao)', $choices['zh_Hans_MO']);
     }
-
-    public function testEmptyValueOption()
-    {
-        // empty_value false
-        $field = new LocaleField('language', array('empty_value' => false));
-        $choices = $field->getOtherChoices();
-        $this->assertArrayNotHasKey('', $choices);
-
-        // empty_value as a blank string
-        $field = new LocaleField('language', array('empty_value' => ''));
-        $choices = $field->getOtherChoices();
-        $this->assertArrayHasKey('', $choices);
-        $this->assertEquals('', $choices['']);
-
-        // empty_value as a normal string
-        $field = new LocaleField('language', array('empty_value' => 'Choose a locale'));
-        $choices = $field->getOtherChoices();
-        $this->assertArrayHasKey('', $choices);
-        $this->assertEquals('Choose a locale', $choices['']);
-    }
 }

tests/Symfony/Tests/Component/Form/TimezoneFieldTest.php

@@ -28,24 +28,4 @@ class TimezoneFieldTest extends \PHPUnit_Framework_TestCase
         $this->assertArrayHasKey('America/New_York', $choices['America']);
         $this->assertEquals('New York', $choices['America']['America/New_York']);
     }
-
-    public function testEmptyValueOption()
-    {
-        // empty_value false
-        $field = new TimezoneField('timezone', array('empty_value' => false));
-        $choices = $field->getOtherChoices();
-        $this->assertArrayNotHasKey('', $choices);
-
-        // empty_value as a blank string
-        $field = new TimezoneField('timezone', array('empty_value' => ''));
-        $choices = $field->getOtherChoices();
-        $this->assertArrayHasKey('', $choices);
-        $this->assertEquals('', $choices['']);
-
-        // empty_value as a normal string
-        $field = new TimezoneField('timezone', array('empty_value' => 'Choose your timezone'));
-        $choices = $field->getOtherChoices();
-        $this->assertArrayHasKey('', $choices);
-        $this->assertEquals('Choose your timezone', $choices['']);
-    }
 }