[Form] Fixed: PropertyPath always requires arrays or objects. Forms now always store arrays or objects as transformed data, even when they were bound empty

src/Symfony/Component/Form/CollectionField.php

@@ -77,6 +77,10 @@ class CollectionField extends Form
 
     public function setData($collection)
     {
+        if (null === $collection) {
+            $collection = array();
+        }
+
         if (!is_array($collection) && !$collection instanceof \Traversable) {
             throw new UnexpectedTypeException($collection, 'array or \Traversable');
         }

src/Symfony/Component/Form/EntityChoiceField.php

@@ -449,7 +449,7 @@ class EntityChoiceField extends ChoiceField
         }
 
         if (count($notFound) > 0) {
-            throw new TransformationFailedException('The entities with keys "%s" could not be found', implode('", "', $notFound));
+            throw new TransformationFailedException(sprintf('The entities with keys "%s" could not be found', implode('", "', $notFound)));
         }
 
         return $result;
@@ -466,10 +466,10 @@ class EntityChoiceField extends ChoiceField
     protected function transform($collectionOrEntity)
     {
         if (null === $collectionOrEntity) {
-            return $this->getOption('multiple') ? array() : '';
+            return $this->isField() ? '' : array();
         }
 
-        if (count($this->identifier) > 1) {
+        if (count($this->getIdentifierFields()) > 1) {
             // load all choices
             $availableEntities = $this->getEntities();
 
@@ -496,7 +496,6 @@ class EntityChoiceField extends ChoiceField
             }
         }
 
-
         return parent::transform($result);
     }
 }

src/Symfony/Component/Form/Field.php

@@ -91,8 +91,7 @@ class Field extends Configurable implements FieldInterface
             $this->setNormalizationTransformer($this->getOption('normalization_transformer'));
         }
 
-        $this->normalizedData = $this->normalize($this->data);
-        $this->transformedData = $this->transform($this->normalizedData);
+        $this->setData($this->data);
 
         if (!$this->getOption('data')) {
             $this->setPropertyPath($this->getOption('property_path'));
@@ -285,9 +284,15 @@ class Field extends Configurable implements FieldInterface
      */
     public function setData($data)
     {
+        // All four transformation methods must be executed to make sure
+        // that all three data representations are synchronized
+        // Store data in between steps because processData() might use
+        // this data
         $this->data = $data;
         $this->normalizedData = $this->normalize($data);
-        $this->transformedData = $this->transform($this->normalizedData);
+        $this->transformedData = $this->transform($this->normalize($data));
+        $this->normalizedData = $this->processData($this->reverseTransform($this->transformedData));
+        $this->data = $this->denormalize($this->normalizedData);
     }
 
     /**

src/Symfony/Component/Form/Form.php

@@ -352,16 +352,6 @@ class Form extends Field implements \IteratorAggregate, FormInterface
      */
     public function setData($data)
     {
-        if (empty($data)) {
-            if ($this->dataConstructor) {
-                $constructor = $this->dataConstructor;
-                $data = $constructor();
-            } else if ($this->dataClass) {
-                $class = $this->dataClass;
-                $data = new $class();
-            }
-        }
-
         parent::setData($data);
 
         // get transformed data and pass its values to child fields
@@ -380,6 +370,30 @@ class Form extends Field implements \IteratorAggregate, FormInterface
         }
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    protected function transform($value)
+    {
+        if (null === $this->getValueTransformer()) {
+            // Empty values must be converted to objects or arrays so that
+            // they can be read by PropertyPath in the child fields
+            if (empty($value)) {
+                if ($this->dataConstructor) {
+                    $constructor = $this->dataConstructor;
+                    return $constructor();
+                } else if ($this->dataClass) {
+                    $class = $this->dataClass;
+                    return new $class();
+                } else {
+                    return array();
+                }
+            }
+        }
+
+        return parent::transform($value);
+    }
+
     /**
      * Returns the data of the field as it is displayed to the user.
      *

src/Symfony/Component/Form/HybridField.php

@@ -135,4 +135,28 @@ class HybridField extends Form
             return Field::isEmpty();
         }
     }
+
+    /**
+     * {@inheritDoc}
+     */
+    protected function transform($value)
+    {
+        if ($this->mode === self::FORM) {
+            return parent::transform($value);
+        } else {
+            return Field::transform($value);
+        }
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    protected function reverseTransform($value)
+    {
+        if ($this->mode === self::FORM) {
+            return parent::reverseTransform($value);
+        } else {
+            return Field::reverseTransform($value);
+        }
+    }
 }

src/Symfony/Component/Form/PropertyPath.php

@@ -14,6 +14,7 @@ namespace Symfony\Component\Form;
 use Symfony\Component\Form\Exception\InvalidPropertyPathException;
 use Symfony\Component\Form\Exception\InvalidPropertyException;
 use Symfony\Component\Form\Exception\PropertyAccessDeniedException;
+use Symfony\Component\Form\Exception\UnexpectedTypeException;
 
 /**
  * Allows easy traversing of a property path
@@ -228,6 +229,10 @@ class PropertyPath implements \IteratorAggregate
      */
     protected function readPropertyPath(&$objectOrArray, $currentIndex)
     {
+        if (!is_object($objectOrArray) && !is_array($objectOrArray)) {
+            throw new UnexpectedTypeException($objectOrArray, 'object or array');
+        }
+
         $property = $this->elements[$currentIndex];
 
         if (is_object($objectOrArray)) {
@@ -261,6 +266,10 @@ class PropertyPath implements \IteratorAggregate
      */
     protected function writePropertyPath(&$objectOrArray, $currentIndex, $value)
     {
+        if (!is_object($objectOrArray) && !is_array($objectOrArray)) {
+            throw new UnexpectedTypeException($objectOrArray, 'object or array');
+        }
+
         $property = $this->elements[$currentIndex];
 
         if ($currentIndex + 1 < $this->length) {

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

@@ -162,23 +162,53 @@ class EntityChoiceFieldTest extends DoctrineOrmTestCase
         $this->assertEquals('', $field->getDisplayedData());
     }
 
-    public function testSetDataMultiple_null()
+    public function testSetDataMultipleExpanded_null()
     {
         $field = new EntityChoiceField('name', array(
             'multiple' => true,
+            'expanded' => true,
             'em' => $this->em,
             'class' => self::SINGLE_IDENT_CLASS,
         ));
         $field->setData(null);
 
+        $this->assertEquals(new ArrayCollection(), $field->getData());
+        $this->assertEquals(array(), $field->getDisplayedData());
+    }
+
+    public function testSetDataMultipleNonExpanded_null()
+    {
+        $field = new EntityChoiceField('name', array(
+            'multiple' => true,
+            'expanded' => false,
+            'em' => $this->em,
+            'class' => self::SINGLE_IDENT_CLASS,
+        ));
+        $field->setData(null);
+
+        $this->assertEquals(new ArrayCollection(), $field->getData());
+        $this->assertEquals('', $field->getDisplayedData());
+    }
+
+    public function testSubmitSingleExpanded_null()
+    {
+        $field = new EntityChoiceField('name', array(
+            'multiple' => false,
+            'expanded' => true,
+            'em' => $this->em,
+            'class' => self::SINGLE_IDENT_CLASS,
+        ));
+        $field->submit(null);
+
         $this->assertEquals(null, $field->getData());
         $this->assertEquals(array(), $field->getDisplayedData());
     }
 
-    public function testSubmitSingle_null()
+    public function testSubmitSingleNonExpanded_null()
     {
         $field = new EntityChoiceField('name', array(
             'multiple' => false,
+            'expanded' => false,
             'em' => $this->em,
             'class' => self::SINGLE_IDENT_CLASS,
         ));
@@ -344,7 +374,7 @@ class EntityChoiceFieldTest extends DoctrineOrmTestCase
             'property' => 'name',
         ));
 
-        $existing = new ArrayCollection(array($entity2));
+        $existing = new ArrayCollection(array(0 => $entity2));
 
         $field->setData($existing);
         $field->submit(array('0', '2'));

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

@@ -15,6 +15,7 @@ require_once __DIR__ . '/Fixtures/Author.php';
 require_once __DIR__ . '/Fixtures/TestField.php';
 require_once __DIR__ . '/Fixtures/InvalidField.php';
 require_once __DIR__ . '/Fixtures/RequiredOptionsField.php';
+require_once __DIR__ . '/Fixtures/FixedValueTransformer.php';
 
 use Symfony\Component\Form\ValueTransformer\ValueTransformerInterface;
 use Symfony\Component\Form\PropertyPath;
@@ -25,6 +26,7 @@ use Symfony\Tests\Component\Form\Fixtures\Author;
 use Symfony\Tests\Component\Form\Fixtures\TestField;
 use Symfony\Tests\Component\Form\Fixtures\InvalidField;
 use Symfony\Tests\Component\Form\Fixtures\RequiredOptionsField;
+use Symfony\Tests\Component\Form\Fixtures\FixedValueTransformer;
 
 class FieldTest extends \PHPUnit_Framework_TestCase
 {
@@ -224,7 +226,11 @@ class FieldTest extends \PHPUnit_Framework_TestCase
     {
         $this->field->setData(123);
 
-        $this->assertSame(123, $this->field->getData());
+        // The values are synchronized
+        // Without value transformer, the field can't know that the data
+        // should be casted to an integer when the field is bound
+        // Even without binding, the data will thus be a string
+        $this->assertSame('123', $this->field->getData());
         $this->assertSame('123', $this->field->getDisplayedData());
     }
 
@@ -349,21 +355,15 @@ class FieldTest extends \PHPUnit_Framework_TestCase
 
     public function testValuesAreTransformedCorrectly()
     {
-        // The value is first passed to the normalization transformer...
-        $normTransformer = $this->createMockTransformer();
-        $normTransformer->expects($this->exactly(2))
-                                ->method('transform')
-                                // Impossible to test with PHPUnit because called twice
-                                // ->with($this->identicalTo(0))
-                                ->will($this->returnValue('norm[0]'));
+        $normTransformer = new FixedValueTransformer(array(
+            null => '',
+            0 => 'norm[0]',
+        ));
 
-        // ...and then to the value transformer
-        $valueTransformer = $this->createMockTransformer();
-        $valueTransformer->expects($this->exactly(2))
-                                ->method('transform')
-                                // Impossible to test with PHPUnit because called twice
-                                // ->with($this->identicalTo('norm[0]'))
-                                ->will($this->returnValue('transform[norm[0]]'));
+        $valueTransformer = new FixedValueTransformer(array(
+            '' => '',
+            'norm[0]' => 'transform[norm[0]]',
+        ));
 
         $field = new TestField('title', array(
             'value_transformer' => $valueTransformer,
@@ -379,18 +379,10 @@ class FieldTest extends \PHPUnit_Framework_TestCase
 
     public function testSubmittedValuesAreTrimmedBeforeTransforming()
     {
-        // The value is passed to the value transformer
-        $transformer = $this->createMockTransformer();
-        $transformer->expects($this->once())
-                                ->method('reverseTransform')
-                                ->with($this->identicalTo('a'))
-                                ->will($this->returnValue('reverse[a]'));
-
-        $transformer->expects($this->exactly(2))
-                                ->method('transform')
-                                // Impossible to test with PHPUnit because called twice
-                                // ->with($this->identicalTo('reverse[a]'))
-                                ->will($this->returnValue('a'));
+        $transformer = new FixedValueTransformer(array(
+            null => '',
+            'reverse[a]' => 'a',
+        ));
 
         $field = new TestField('title', array(
             'value_transformer' => $transformer,
@@ -404,18 +396,10 @@ class FieldTest extends \PHPUnit_Framework_TestCase
 
     public function testSubmittedValuesAreNotTrimmedBeforeTransformingIfDisabled()
     {
-        // The value is passed to the value transformer
-        $transformer = $this->createMockTransformer();
-        $transformer->expects($this->once())
-                                ->method('reverseTransform')
-                                ->with($this->identicalTo(' a '))
-                                ->will($this->returnValue('reverse[ a ]'));
-
-        $transformer->expects($this->exactly(2))
-                                ->method('transform')
-                                // Impossible to test with PHPUnit because called twice
-                                // ->with($this->identicalTo('reverse[ a ]'))
-                                ->will($this->returnValue(' a '));
+        $transformer = new FixedValueTransformer(array(
+            null => '',
+            'reverse[ a ]' => ' a ',
+        ));
 
         $field = new TestField('title', array(
             'trim' => false,
@@ -428,21 +412,6 @@ class FieldTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals('reverse[ a ]', $field->getData());
     }
 
-    /*
-     * This is important so that submit() can work even if setData() was not called
-     * before
-     */
-    public function testWritePropertyTreatsEmptyValuesAsArrays()
-    {
-        $array = null;
-
-        $field = new TestField('firstName');
-        $field->submit('Bernhard');
-        $field->writeProperty($array);
-
-        $this->assertEquals(array('firstName' => 'Bernhard'), $array);
-    }
-
     public function testWritePropertyDoesNotWritePropertyIfPropertyPathIsEmpty()
     {
         $object = new Author();
@@ -470,15 +439,16 @@ class FieldTest extends \PHPUnit_Framework_TestCase
     {
         // The value is passed to the value transformer
         $transformer = $this->createMockTransformer();
-        $transformer->expects($this->once())
-                ->method('reverseTransform')
-                ->will($this->throwException(new TransformationFailedException()));
 
         $field = new TestField('title', array(
             'trim' => false,
             'value_transformer' => $transformer,
         ));
 
+        $transformer->expects($this->once())
+                ->method('reverseTransform')
+                ->will($this->throwException(new TransformationFailedException()));
+
         $field->submit('a');
 
         $this->assertEquals('a', $field->getDisplayedData());

tests/Symfony/Tests/Component/Form/Fixtures/FixedValueTransformer.php

@@ -0,0 +1,35 @@
+<?php
+
+namespace Symfony\Tests\Component\Form\Fixtures;
+
+use Symfony\Component\Form\ValueTransformer\ValueTransformerInterface;
+
+class FixedValueTransformer implements ValueTransformerInterface
+{
+    private $mapping;
+
+    public function __construct(array $mapping)
+    {
+        $this->mapping = $mapping;
+    }
+
+    public function transform($value)
+    {
+        if (!array_key_exists($value, $this->mapping)) {
+            throw new \RuntimeException(sprintf('No mapping for value "%s"', $value));
+        }
+
+        return $this->mapping[$value];
+    }
+
+    public function reverseTransform($value)
+    {
+        $result = array_search($value, $this->mapping, true);
+
+        if ($result === false) {
+            throw new \RuntimeException(sprintf('No reverse mapping for value "%s"', $value));
+        }
+
+        return $result;
+    }
+}

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

@@ -840,7 +840,7 @@ class FormTest extends \PHPUnit_Framework_TestCase
      */
     public function testAddThrowsExceptionIfStringButNoFieldFactory()
     {
-        $form = new Form('author', array('data_class' => 'Application\Entity'));
+        $form = new Form('author');
 
         $form->add('firstName');
     }
@@ -991,14 +991,6 @@ class FormTest extends \PHPUnit_Framework_TestCase
         $form->setData(new Author());
     }
 
-    public function testSetDataToNull()
-    {
-        $form = new Form('author');
-        $form->setData(null);
-
-        $this->assertNull($form->getData());
-    }
-
     public function testSetDataToNullCreatesObjectIfClassAvailable()
     {
         $form = new Form('author', array(
@@ -1022,6 +1014,18 @@ class FormTest extends \PHPUnit_Framework_TestCase
         $this->assertSame($author, $form->getData());
     }
 
+    /*
+     * We need something to write the field values into
+     */
+    public function testSetDataToNullCreatesArrayIfNoDataClassOrConstructor()
+    {
+        $author = new Author();
+        $form = new Form('author');
+        $form->setData(null);
+
+        $this->assertSame(array(), $form->getData());
+    }
+
     public function testSubmitUpdatesTransformedDataFromAllFields()
     {
         $originalAuthor = new Author();

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

@@ -192,6 +192,33 @@ class PropertyPathTest extends \PHPUnit_Framework_TestCase
         $path->getValue(new Author());
     }
 
+    public function testGetValueThrowsExceptionIfNotObjectOrArray()
+    {
+        $path = new PropertyPath('foobar');
+
+        $this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException');
+
+        $path->getValue('baz');
+    }
+
+    public function testGetValueThrowsExceptionIfNull()
+    {
+        $path = new PropertyPath('foobar');
+
+        $this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException');
+
+        $path->getValue(null);
+    }
+
+    public function testGetValueThrowsExceptionIfEmpty()
+    {
+        $path = new PropertyPath('foobar');
+
+        $this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException');
+
+        $path->getValue('');
+    }
+
     public function testSetValueUpdatesArrays()
     {
         $array = array();
@@ -292,6 +319,36 @@ class PropertyPathTest extends \PHPUnit_Framework_TestCase
         $path->setValue(new Author(), 'foobar');
     }
 
+    public function testSetValueThrowsExceptionIfNotObjectOrArray()
+    {
+        $path = new PropertyPath('foobar');
+        $value = 'baz';
+
+        $this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException');
+
+        $path->setValue($value, 'bam');
+    }
+
+    public function testSetValueThrowsExceptionIfNull()
+    {
+        $path = new PropertyPath('foobar');
+        $value = null;
+
+        $this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException');
+
+        $path->setValue($value, 'bam');
+    }
+
+    public function testSetValueThrowsExceptionIfEmpty()
+    {
+        $path = new PropertyPath('foobar');
+        $value = '';
+
+        $this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException');
+
+        $path->setValue($value, 'bam');
+    }
+
     public function testToString()
     {
         $path = new PropertyPath('reference.traversable[index].property');