From 2bf4d6cff4522a5e3a905a0049f2a1ba1dd47fe1 Mon Sep 17 00:00:00 2001
From: Bernhard Schussek <bschussek@gmail.com>
Date: Fri, 13 Jul 2012 12:12:25 +0200
Subject: [PATCH] [Form] Fixed FormFactory not to set "data" option if not
 explicitely given

---
 .../Form/Extension/Core/Type/FormType.php     |  2 +-
 src/Symfony/Component/Form/FormFactory.php    |  2 +-
 .../Component/Form/Tests/Fixtures/FooType.php | 26 ++++++++++++-------
 .../Component/Form/Tests/FormFactoryTest.php  | 13 ++++++++++
 4 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php
index 12747fd175..f2c6b89d78 100644
--- a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php
+++ b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php
@@ -149,7 +149,7 @@ class FormType extends AbstractType
     {
         // Derive "data_class" option from passed "data" object
         $dataClass = function (Options $options) {
-            return is_object($options['data']) ? get_class($options['data']) : null;
+            return isset($options['data']) && is_object($options['data']) ? get_class($options['data']) : null;
         };
 
         // Derive "empty_data" closure from "data_class" option
diff --git a/src/Symfony/Component/Form/FormFactory.php b/src/Symfony/Component/Form/FormFactory.php
index 046804f039..d0de5c92cf 100644
--- a/src/Symfony/Component/Form/FormFactory.php
+++ b/src/Symfony/Component/Form/FormFactory.php
@@ -145,7 +145,7 @@ class FormFactory implements FormFactoryInterface
      */
     public function createNamedBuilder($name, $type, $data = null, array $options = array(), FormBuilderInterface $parent = null)
     {
-        if (!array_key_exists('data', $options)) {
+        if (null !== $data && !array_key_exists('data', $options)) {
             $options['data'] = $data;
         }
 
diff --git a/src/Symfony/Component/Form/Tests/Fixtures/FooType.php b/src/Symfony/Component/Form/Tests/Fixtures/FooType.php
index feae68900f..05cbf1dd46 100644
--- a/src/Symfony/Component/Form/Tests/Fixtures/FooType.php
+++ b/src/Symfony/Component/Form/Tests/Fixtures/FooType.php
@@ -16,13 +16,19 @@ use Symfony\Component\Form\FormBuilder;
 use Symfony\Component\Form\FormBuilderInterface;
 use Symfony\Component\Form\FormFactoryInterface;
 use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\OptionsResolver\OptionsResolverInterface;
 
 class FooType extends AbstractType
 {
     public function buildForm(FormBuilderInterface $builder, array $options)
     {
         $builder->setAttribute('foo', 'x');
-        $builder->setAttribute('data_option', $options['data']);
+        $builder->setAttribute('data_option', isset($options['data']) ? $options['data'] : null);
+        // Important: array_key_exists(), not isset()
+        // -> The "data" option is optional in FormType
+        //    If it is given, the form's data will be locked to the value of the option
+        //    Thus "data" must not be set in the array unless explicitely specified
+        $builder->setAttribute('data_option_set', array_key_exists('data', $options));
     }
 
     public function getName()
@@ -35,21 +41,21 @@ class FooType extends AbstractType
         return new FormBuilder($name, null, new EventDispatcher(), $factory);
     }
 
-    public function getDefaultOptions()
+    public function setDefaultOptions(OptionsResolverInterface $resolver)
     {
-        return array(
-            'data' => null,
+        $resolver->setDefaults(array(
             'required' => false,
             'max_length' => null,
             'a_or_b' => 'a',
-        );
-    }
+        ));
 
-    public function getAllowedOptionValues()
-    {
-        return array(
+        $resolver->setOptional(array(
+            'data',
+        ));
+
+        $resolver->setAllowedValues(array(
             'a_or_b' => array('a', 'b'),
-        );
+        ));
     }
 
     public function getParent()
diff --git a/src/Symfony/Component/Form/Tests/FormFactoryTest.php b/src/Symfony/Component/Form/Tests/FormFactoryTest.php
index 189cdeec8c..cdb967f98c 100644
--- a/src/Symfony/Component/Form/Tests/FormFactoryTest.php
+++ b/src/Symfony/Component/Form/Tests/FormFactoryTest.php
@@ -150,9 +150,21 @@ class FormFactoryTest extends \PHPUnit_Framework_TestCase
         $builder = $this->factory->createNamedBuilder('bar', 'foo', 'xyz');
 
         // see FooType::buildForm()
+        $this->assertTrue($builder->getAttribute('data_option_set'));
         $this->assertEquals('xyz', $builder->getAttribute('data_option'));
     }
 
+    public function testCreateNamedBuilderDoesNotSetDataOptionIfNull()
+    {
+        $type = new FooType();
+        $this->extension1->addType($type);
+
+        $builder = $this->factory->createNamedBuilder('bar', 'foo', null);
+
+        // see FooType::buildForm()
+        $this->assertFalse($builder->getAttribute('data_option_set'));
+    }
+
     public function testCreateNamedBuilderDoesNotOverrideExistingDataOption()
     {
         $type = new FooType();
@@ -163,6 +175,7 @@ class FormFactoryTest extends \PHPUnit_Framework_TestCase
         ));
 
         // see FooType::buildForm()
+        $this->assertTrue($builder->getAttribute('data_option_set'));
         $this->assertEquals('abc', $builder->getAttribute('data_option'));
     }