symfony · fabpot · Feb 10, 2012 · Feb 7, 2012 · Feb 7, 2012 · Feb 7, 2012
diff --git a/CHANGELOG-2.1.md b/CHANGELOG-2.1.md
@@ -202,6 +202,8 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c
  * added options "add_method" and "remove_method" to collection and choice type
  * forms now don't create an empty object anymore if they are completely
    empty and not required. The empty value for such forms is null.
+ * FormType::getDefaultOptions() now sees default options defined by parent types
+ * [BC BREAK] FormType::getParent() does not see default options anymore
 
 ### HttpFoundation
 

diff --git a/UPGRADE-2.1.md b/UPGRADE-2.1.md
@@ -229,3 +229,22 @@ UPGRADE FROM 2.0 to 2.1
                 return false;
             }
         }
+
+* The options passed to `getParent` of the form types don't contain default
+  options anymore
+
+    You should check with `isset` if options exist before checking their value.
+
+    Before:
+
+        public function getParent(array $options)
+        {
+            return 'single_text' === $options['widget'] ? 'text' : 'choice';
+        }
+
+    After:
+
+        public function getParent(array $options)
+        {
+            return isset($options['widget']) && 'single_text' === $options['widget'] ? 'text' : 'choice';
+        }
diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php b/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php
@@ -50,7 +50,6 @@ public function getDefaultOptions(array $options)
             'property'          => null,
             'query_builder'     => null,
             'loader'            => null,
-            'choices'           => null,
             'group_by'          => null,
         );
 

diff --git a/...ymfony/Component/Form/Extension/Core/DataTransformer/ChoicesToBooleanArrayTransformer.php b/...ymfony/Component/Form/Extension/Core/DataTransformer/ChoicesToBooleanArrayTransformer.php
@@ -107,7 +107,7 @@ public function reverseTransform($values)
         }
 
         if (count($unknown) > 0) {
-            throw new TransformationFailedException('The choices "' . implode('", "', $unknown, $code, $previous) . '" where not found');
+            throw new TransformationFailedException('The choices "' . implode('", "', $unknown) . '" were not found');
         }
 
         return $result;

diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
@@ -149,7 +149,7 @@ public function getDefaultOptions(array $options)
             'multiple'          => false,
             'expanded'          => false,
             'choice_list'       => null,
-            'choices'           => array(),
+            'choices'           => null,
             'preferred_choices' => array(),
             'value_strategy'    => ChoiceList::GENERATE,
             'index_strategy'    => ChoiceList::GENERATE,
@@ -166,7 +166,7 @@ public function getDefaultOptions(array $options)
      */
     public function getParent(array $options)
     {
-        return $options['expanded'] ? 'form' : 'field';
+        return isset($options['expanded']) && $options['expanded'] ? 'form' : 'field';
     }
 
     /**

diff --git a/src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php b/src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php
@@ -158,6 +158,11 @@ public function getDefaultOptions(array $options)
             'widget'        => null,
             // This will overwrite "empty_value" child options
             'empty_value'   => null,
+            // If initialized with a \DateTime object, FieldType initializes
+            // this option to "\DateTime". Since the internal, normalized
+            // representation is not \DateTime, but an array, we need to unset
+            // this option.
+            'data_class'    => null,
         );
     }
 
@@ -200,7 +205,7 @@ public function getAllowedOptionValues(array $options)
      */
     public function getParent(array $options)
     {
-        return 'single_text' === $options['widget'] ? 'field' : 'form';
+        return isset($options['widget']) && 'single_text' === $options['widget'] ? 'field' : 'form';
     }
 
     /**

diff --git a/src/Symfony/Component/Form/Extension/Core/Type/DateType.php b/src/Symfony/Component/Form/Extension/Core/Type/DateType.php
@@ -183,6 +183,11 @@ public function getDefaultOptions(array $options)
             // them like immutable value objects
             'by_reference'   => false,
             'error_bubbling' => false,
+            // If initialized with a \DateTime object, FieldType initializes
+            // this option to "\DateTime". Since the internal, normalized
+            // representation is not \DateTime, but an array, we need to unset
+            // this option.
+            'data_class'     => null,
         );
     }
 
@@ -211,7 +216,7 @@ public function getAllowedOptionValues(array $options)
      */
     public function getParent(array $options)
     {
-        return 'single_text' === $options['widget'] ? 'field' : 'form';
+        return isset($options['widget']) && 'single_text' === $options['widget'] ? 'field' : 'form';
     }
 
     /**

diff --git a/src/Symfony/Component/Form/Extension/Core/Type/TimeType.php b/src/Symfony/Component/Form/Extension/Core/Type/TimeType.php
@@ -156,6 +156,11 @@ public function getDefaultOptions(array $options)
             // them like immutable value objects
             'by_reference'   => false,
             'error_bubbling' => false,
+            // If initialized with a \DateTime object, FieldType initializes
+            // this option to "\DateTime". Since the internal, normalized
+            // representation is not \DateTime, but an array, we need to unset
+            // this option.
+            'data_class'     => null,
         );
     }
 
@@ -184,7 +189,7 @@ public function getAllowedOptionValues(array $options)
      */
     public function getParent(array $options)
     {
-        return 'single_text' === $options['widget'] ? 'field' : 'form';
+        return isset($options['widget']) && 'single_text' === $options['widget'] ? 'field' : 'form';
     }
 
     /**

diff --git a/src/Symfony/Component/Form/Extension/Core/Type/TimezoneType.php b/src/Symfony/Component/Form/Extension/Core/Type/TimezoneType.php
@@ -31,7 +31,7 @@ public function getDefaultOptions(array $options)
             'value_strategy' => ChoiceList::COPY_CHOICE,
         );
 
-        if (!isset($options['choice_list']) && !isset($options['choices'])) {
+        if (empty($options['choice_list']) && empty($options['choices'])) {
             $defaultOptions['choices'] = self::getTimezones();
         }
 

diff --git a/src/Symfony/Component/Form/FormFactory.php b/src/Symfony/Component/Form/FormFactory.php
@@ -213,16 +213,20 @@ public function createBuilder($type, $data = null, array $options = array(), For
      */
     public function createNamedBuilder($type, $name, $data = null, array $options = array(), FormBuilder $parent = null)
     {
-        $builder = null;
-        $types = array();
-        $knownOptions = array();
-        $passedOptions = array_keys($options);
-        $optionValues = array();
-
         if (!array_key_exists('data', $options)) {
             $options['data'] = $data;
         }
 
+        $builder = null;
+        $types = array();
+        $defaultOptions = array();
+        $optionValues = array();
+        $passedOptions = $options;
+
+        // Bottom-up determination of the type hierarchy
+        // Start with the actual type and look for the parent type
+        // The complete hierarchy is saved in $types, the first entry being
+        // the root and the last entry being the leaf (the concrete type)
         while (null !== $type) {
             if ($type instanceof FormTypeInterface) {
                 if ($type->getName() == $type->getParent($options)) {
@@ -236,28 +240,47 @@ public function createNamedBuilder($type, $name, $data = null, array $options =
                 throw new UnexpectedTypeException($type, 'string or Symfony\Component\Form\FormTypeInterface');
             }
 
-            $defaultOptions = $type->getDefaultOptions($options);
+            array_unshift($types, $type);
+
+            // getParent() cannot see default options set by this type nor
+            // default options set by parent types
+            // As a result, the options always have to be checked for
+            // existence with isset() before using them in this method.
+            $type = $type->getParent($options);
+        }
+
+        // Top-down determination of the options and default options
+        foreach ($types as $type) {
+            // Merge the default options of all types to an array of default
+            // options. Default options of children override default options
+            // of parents.
+            // Default options of ancestors are already visible in the $options
+            // array passed to the following methods.
+            $defaultOptions = array_replace($defaultOptions, $type->getDefaultOptions($options));
             $optionValues = array_merge_recursive($optionValues, $type->getAllowedOptionValues($options));
 
             foreach ($type->getExtensions() as $typeExtension) {
                 $defaultOptions = array_replace($defaultOptions, $typeExtension->getDefaultOptions($options));
                 $optionValues = array_merge_recursive($optionValues, $typeExtension->getAllowedOptionValues($options));
             }
 
-            $options = array_replace($defaultOptions, $options);
-            $knownOptions = array_merge($knownOptions, array_keys($defaultOptions));
-            array_unshift($types, $type);
-            $type = $type->getParent($options);
+            // In each turn, the options are replaced by the combination of
+            // the currently known default options and the passed options.
+            // It is important to merge with $passedOptions and not with
+            // $options, otherwise default options of parents would override
+            // default options of child types.
+            $options = array_replace($defaultOptions, $passedOptions);
         }
 
         $type = end($types);
+        $knownOptions = array_keys($defaultOptions);
         $diff = array_diff(self::$requiredOptions, $knownOptions);
 
         if (count($diff) > 0) {
             throw new TypeDefinitionException(sprintf('Type "%s" should support the option(s) "%s"', $type->getName(), implode('", "', $diff)));
         }
 
-        $diff = array_diff($passedOptions, $knownOptions);
+        $diff = array_diff(array_keys($passedOptions), $knownOptions);
 
         if (count($diff) > 1) {
             throw new CreationException(sprintf('The options "%s" do not exist. Known options are: "%s"', implode('", "', $diff), implode('", "', $knownOptions)));

diff --git a/tests/Symfony/Tests/Component/Form/Extension/Core/Type/DateTimeTypeTest.php b/tests/Symfony/Tests/Component/Form/Extension/Core/Type/DateTimeTypeTest.php
@@ -258,4 +258,12 @@ public function testSubmit_invalidDateTime()
         $this->assertEquals(array(new FormError('Customized invalid message', array())), $form['date']->getErrors());
         $this->assertEquals(array(new FormError('Customized invalid message', array())), $form['time']->getErrors());
     }
+
+    // Bug fix
+    public function testInitializeWithDateTime()
+    {
+        // Throws an exception if "data_class" option is not explicitely set
+        // to null in the type
+        $this->factory->create('datetime', new \DateTime());
+    }
 }
diff --git a/tests/Symfony/Tests/Component/Form/Extension/Core/Type/DateTypeTest.php b/tests/Symfony/Tests/Component/Form/Extension/Core/Type/DateTypeTest.php
@@ -530,4 +530,12 @@ public function testPassWidgetToView()
 
         $this->assertSame('single_text', $view->get('widget'));
     }
+
+    // Bug fix
+    public function testInitializeWithDateTime()
+    {
+        // Throws an exception if "data_class" option is not explicitely set
+        // to null in the type
+        $this->factory->create('date', new \DateTime());
+    }
 }
diff --git a/tests/Symfony/Tests/Component/Form/Extension/Core/Type/FieldTypeTest.php b/tests/Symfony/Tests/Component/Form/Extension/Core/Type/FieldTypeTest.php
@@ -221,6 +221,42 @@ public function testBindWithEmptyDataCreatesObjectIfClassAvailable()
         $this->assertEquals($author, $form->getData());
     }
 
+    public function testBindWithEmptyDataCreatesObjectIfInitiallyBoundWithObject()
+    {
+        $form = $this->factory->create('form', null, array(
+            // data class is inferred from the passed object
+            'data' => new Author(),
+            'required' => false,
+        ));
+        $form->add($this->factory->createNamed('field', 'firstName'));
+        $form->add($this->factory->createNamed('field', 'lastName'));
+
+        $form->setData(null);
+        // partially empty, still an object is created
+        $form->bind(array('firstName' => 'Bernhard', 'lastName' => ''));
+
+        $author = new Author();
+        $author->firstName = 'Bernhard';
+        $author->setLastName('');
+
+        $this->assertEquals($author, $form->getData());
+    }
+
+    public function testBindWithEmptyDataDoesNotCreateObjectIfDataClassIsNull()
+    {
+        $form = $this->factory->create('form', null, array(
+            'data' => new Author(),
+            'data_class' => null,
+            'required' => false,
+        ));
+        $form->add($this->factory->createNamed('field', 'firstName'));
+
+        $form->setData(null);
+        $form->bind(array('firstName' => 'Bernhard'));
+
+        $this->assertSame(array('firstName' => 'Bernhard'), $form->getData());
+    }
+
     public function testBindEmptyWithEmptyDataCreatesNoObjectIfNotRequired()
     {
         $form = $this->factory->create('form', null, array(

diff --git a/tests/Symfony/Tests/Component/Form/Extension/Core/Type/TimeTypeTest.php b/tests/Symfony/Tests/Component/Form/Extension/Core/Type/TimeTypeTest.php
@@ -401,4 +401,12 @@ public function testIsPartiallyFilled_returnsTrueIfChoiceAndSecondsEmpty()
 
         $this->assertTrue($form->isPartiallyFilled());
     }
+
+    // Bug fix
+    public function testInitializeWithDateTime()
+    {
+        // Throws an exception if "data_class" option is not explicitely set
+        // to null in the type
+        $this->factory->create('time', new \DateTime());
+    }
 }