[Form] Improved test coverage of FormFactory and improved error handling

src/Symfony/Component/Form/Exception/CreationException.php

@@ -0,0 +1,21 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Form\Exception;
+
+/**
+ * Thrown when a form could not be constructed by a FormFactory
+ *
+ * @author Bernhard Schussek <bernhard.schussek@symfony.com>
+ */
+class CreationException extends FormException
+{
+}

src/Symfony/Component/Form/Exception/TypeDefinitionException.php

@@ -0,0 +1,21 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Form\Exception;
+
+/**
+ * Thrown when a form type is configured incorrectly
+ *
+ * @author Bernhard Schussek <bernhard.schussek@symfony.com>
+ */
+class TypeDefinitionException extends FormException
+{
+}

src/Symfony/Component/Form/FormFactory.php

@@ -13,9 +13,17 @@ namespace Symfony\Component\Form;
 
 use Symfony\Component\Form\Exception\FormException;
 use Symfony\Component\Form\Exception\UnexpectedTypeException;
+use Symfony\Component\Form\Exception\TypeDefinitionException;
+use Symfony\Component\Form\Exception\CreationException;
 
 class FormFactory implements FormFactoryInterface
 {
+    private static $requiredOptions = array(
+        'data',
+        'required',
+        'max_length',
+    );
+
     private $extensions = array();
 
     private $types = array();
@@ -40,41 +48,36 @@ class FormFactory implements FormFactoryInterface
         $this->extensions = $extensions;
     }
 
+    public function hasType($name)
+    {
+        if (isset($this->types[$name])) {
+            return true;
+        }
+
+        try {
+            $this->loadType($name);
+        } catch (FormException $e) {
+            return false;
+        }
+
+        return true;
+    }
+
+    public function addType(FormTypeInterface $type)
+    {
+        $this->loadTypeExtensions($type);
+
+        $this->types[$type->getName()] = $type;
+    }
+
     public function getType($name)
     {
-        $type = null;
-
-        if ($name instanceof FormTypeInterface) {
-            $type = $name;
-            $name = $type->getName();
+        if (!is_string($name)) {
+            throw new UnexpectedTypeException($name, 'string');
         }
 
         if (!isset($this->types[$name])) {
-            if (!$type) {
-                foreach ($this->extensions as $extension) {
-                    if ($extension->hasType($name)) {
-                        $type = $extension->getType($name);
-                        break;
-                    }
-                }
-
-                if (!$type) {
-                    throw new FormException(sprintf('Could not load type "%s"', $name));
-                }
-            }
-
-            $typeExtensions = array();
-
-            foreach ($this->extensions as $extension) {
-                $typeExtensions = array_merge(
-                    $typeExtensions,
-                    $extension->getTypeExtensions($name)
-                );
-            }
-
-            $type->setExtensions($typeExtensions);
-
-            $this->types[$name] = $type;
+            $this->loadType($name);
         }
 
         return $this->types[$name];
@@ -117,7 +120,11 @@ class FormFactory implements FormFactoryInterface
         }
 
         while (null !== $type) {
-            $type = $this->getType($type);
+            if ($type instanceof FormTypeInterface) {
+                $this->addType($type);
+            } else {
+                $type = $this->getType($type);
+            }
 
             $defaultOptions = $type->getDefaultOptions($options);
 
@@ -131,17 +138,30 @@ class FormFactory implements FormFactoryInterface
             $type = $type->getParent($options);
         }
 
-        $diff = array_diff($passedOptions, $knownOptions);
+        $type = end($types);
+        $diff = array_diff(self::$requiredOptions, $knownOptions);
 
         if (count($diff) > 0) {
-            throw new FormException(sprintf('The options "%s" do not exist', implode('", "', $diff)));
+            throw new TypeDefinitionException(sprintf('Type "%s" should support the option(s) "%s"', $type->getName(), implode('", "', $diff)));
+        }
+
+        $diff = array_diff($passedOptions, $knownOptions);
+
+        if (count($diff) > 1) {
+            throw new CreationException(sprintf('The options "%s" do not exist', implode('", "', $diff)));
+        }
+
+        if (count($diff) > 0) {
+            throw new CreationException(sprintf('The option "%s" does not exist', $diff[0]));
         }
 
         for ($i = 0, $l = count($types); $i < $l && !$builder; ++$i) {
             $builder = $types[$i]->createBuilder($name, $this, $options);
         }
 
-        // TODO check if instance exists
+        if (!$builder) {
+            throw new TypeDefinitionException(sprintf('Type "%s" or any of its parents should return a FormBuilder instance from createBuilder()', $type->getName()));
+        }
 
         $builder->setTypes($types);
 
@@ -198,4 +218,38 @@ class FormFactory implements FormFactoryInterface
 
         $this->guesser = new FormTypeGuesserChain($guessers);
     }
+
+    private function loadType($name)
+    {
+        $type = null;
+
+        foreach ($this->extensions as $extension) {
+            if ($extension->hasType($name)) {
+                $type = $extension->getType($name);
+                break;
+            }
+        }
+
+        if (!$type) {
+            throw new FormException(sprintf('Could not load type "%s"', $name));
+        }
+
+        $this->loadTypeExtensions($type);
+
+        $this->types[$name] = $type;
+    }
+
+    private function loadTypeExtensions(FormTypeInterface $type)
+    {
+        $typeExtensions = array();
+
+        foreach ($this->extensions as $extension) {
+            $typeExtensions = array_merge(
+                $typeExtensions,
+                $extension->getTypeExtensions($type->getName())
+            );
+        }
+
+        $type->setExtensions($typeExtensions);
+    }
 }

src/Symfony/Component/Form/FormFactoryInterface.php

@@ -26,4 +26,8 @@ interface FormFactoryInterface
     function createBuilderForProperty($class, $property, $data = null, array $options = array());
 
     function getType($name);
+
+    function hasType($name);
+
+    function addType(FormTypeInterface $type);
 }

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

@@ -17,53 +17,29 @@ use Symfony\Component\Form\FormInterface;
 use Symfony\Component\Form\FormView;
 use Symfony\Component\Form\FormFactoryInterface;
 use Symfony\Component\Form\FormBuilder;
+use Symfony\Tests\Component\Form\Fixtures\FooType;
 
 class AbstractExtensionTest extends \PHPUnit_Framework_TestCase
 {
     public function testHasType()
     {
-        $loader = new TestExtension();
+        $loader = new ConcreteExtension();
         $this->assertTrue($loader->hasType('foo'));
         $this->assertFalse($loader->hasType('bar'));
     }
 
     public function testGetType()
     {
-        $loader = new TestExtension(array($type));
-        $this->assertInstanceOf(__NAMESPACE__.'\TestType', $loader->getType('foo'));
-        $this->assertSame($loader->getType('foo'), $loader->getType('foo'));
+        $loader = new ConcreteExtension();
+        $this->assertTrue($loader->getType('foo') instanceof FooType);
     }
 }
 
-class TestType implements FormTypeInterface
-{
-    public function getName()
-    {
-        return 'foo';
-    }
-
-    function buildForm(FormBuilder $builder, array $options) {}
-
-    function buildView(FormView $view, FormInterface $form) {}
-
-    function buildViewBottomUp(FormView $view, FormInterface $form) {}
-
-    function createBuilder($name, FormFactoryInterface $factory, array $options) {}
-
-    function getDefaultOptions(array $options) {}
-
-    function getParent(array $options) {}
-
-    function setExtensions(array $extensions) {}
-
-    function getExtensions() {}
-}
-
-class TestExtension extends AbstractExtension
+class ConcreteExtension extends AbstractExtension
 {
     protected function loadTypes()
     {
-        return array(new TestType());
+        return array(new FooType());
     }
 
     protected function loadTypeGuesser()

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

@@ -0,0 +1,41 @@
+<?php
+
+namespace Symfony\Tests\Component\Form\Fixtures;
+
+use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\FormBuilder;
+use Symfony\Component\Form\FormFactoryInterface;
+use Symfony\Component\EventDispatcher\EventDispatcher;
+
+class FooType extends AbstractType
+{
+    public function buildForm(FormBuilder $builder, array $options)
+    {
+        $builder->setAttribute('foo', 'x');
+        $builder->setAttribute('data_option', $options['data']);
+    }
+
+    public function getName()
+    {
+        return 'foo';
+    }
+
+    public function createBuilder($name, FormFactoryInterface $factory, array $options)
+    {
+        return new FormBuilder($name, $factory, new EventDispatcher());
+    }
+
+    public function getDefaultOptions(array $options)
+    {
+        return array(
+            'data' => null,
+            'required' => false,
+            'max_length' => null,
+        );
+    }
+
+    public function getParent(array $options)
+    {
+        return null;
+    }
+}

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

@@ -0,0 +1,19 @@
+<?php
+
+namespace Symfony\Tests\Component\Form\Fixtures;
+
+use Symfony\Component\Form\AbstractTypeExtension;
+use Symfony\Component\Form\FormBuilder;
+
+class FooTypeBarExtension extends AbstractTypeExtension
+{
+    public function buildForm(FormBuilder $builder, array $options)
+    {
+        $builder->setAttribute('bar', 'x');
+    }
+
+    public function getExtendedType()
+    {
+        return 'foo';
+    }
+}

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

@@ -0,0 +1,19 @@
+<?php
+
+namespace Symfony\Tests\Component\Form\Fixtures;
+
+use Symfony\Component\Form\AbstractTypeExtension;
+use Symfony\Component\Form\FormBuilder;
+
+class FooTypeBazExtension extends AbstractTypeExtension
+{
+    public function buildForm(FormBuilder $builder, array $options)
+    {
+        $builder->setAttribute('baz', 'x');
+    }
+
+    public function getExtendedType()
+    {
+        return 'foo';
+    }
+}

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

@@ -0,0 +1,63 @@
+<?php
+
+namespace Symfony\Tests\Component\Form\Fixtures;
+
+use Symfony\Component\Form\FormTypeInterface;
+use Symfony\Component\Form\FormTypeExtensionInterface;
+use Symfony\Component\Form\FormTypeGuesserInterface;
+use Symfony\Component\Form\FormExtensionInterface;
+
+class TestExtension implements FormExtensionInterface
+{
+    private $types = array();
+
+    private $extensions = array();
+
+    private $guesser;
+
+    public function __construct(FormTypeGuesserInterface $guesser)
+    {
+        $this->guesser = $guesser;
+    }
+
+    public function addType(FormTypeInterface $type)
+    {
+        $this->types[$type->getName()] = $type;
+    }
+
+    public function getType($name)
+    {
+        return isset($this->types[$name]) ? $this->types[$name] : null;
+    }
+
+    public function hasType($name)
+    {
+        return isset($this->types[$name]);
+    }
+
+    public function addTypeExtension(FormTypeExtensionInterface $extension)
+    {
+        $type = $extension->getExtendedType();
+
+        if (!isset($this->extensions[$type])) {
+            $this->extensions[$type] = array();
+        }
+
+        $this->extensions[$type][] = $extension;
+    }
+
+    public function getTypeExtensions($name)
+    {
+        return isset($this->extensions[$name]) ? $this->extensions[$name] : array();
+    }
+
+    public function hasTypeExtensions($name)
+    {
+        return isset($this->extensions[$name]);
+    }
+
+    public function getTypeGuesser()
+    {
+        return $this->guesser;
+    }
+}

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

@@ -11,10 +11,15 @@
 
 namespace Symfony\Tests\Component\Form;
 
+use Symfony\Component\Form\FormBuilder;
 use Symfony\Component\Form\FormFactory;
 use Symfony\Component\Form\Guess\Guess;
 use Symfony\Component\Form\Guess\ValueGuess;
 use Symfony\Component\Form\Guess\TypeGuess;
+use Symfony\Tests\Component\Form\Fixtures\TestExtension;
+use Symfony\Tests\Component\Form\Fixtures\FooType;
+use Symfony\Tests\Component\Form\Fixtures\FooTypeBarExtension;
+use Symfony\Tests\Component\Form\Fixtures\FooTypeBazExtension;
 
 class FormFactoryTest extends \PHPUnit_Framework_TestCase
 {
@@ -32,17 +37,253 @@ class FormFactoryTest extends \PHPUnit_Framework_TestCase
     {
         $this->guesser1 = $this->getMock('Symfony\Component\Form\FormTypeGuesserInterface');
         $this->guesser2 = $this->getMock('Symfony\Component\Form\FormTypeGuesserInterface');
-        $this->extension1 = $this->getMock('Symfony\Component\Form\FormExtensionInterface');
-        $this->extension1->expects($this->any())
-            ->method('getTypeGuesser')
-            ->will($this->returnValue($this->guesser1));
-        $this->extension2 = $this->getMock('Symfony\Component\Form\FormExtensionInterface');
-        $this->extension2->expects($this->any())
-            ->method('getTypeGuesser')
-            ->will($this->returnValue($this->guesser2));
+        $this->extension1 = new TestExtension($this->guesser1);
+        $this->extension2 = new TestExtension($this->guesser2);
         $this->factory = new FormFactory(array($this->extension1, $this->extension2));
     }
 
+    public function testAddType()
+    {
+        $this->assertFalse($this->factory->hasType('foo'));
+
+        $type = new FooType();
+        $this->factory->addType($type);
+
+        $this->assertTrue($this->factory->hasType('foo'));
+        $this->assertSame($type, $this->factory->getType('foo'));
+    }
+
+    public function testAddTypeAddsExtensions()
+    {
+        $type = new FooType();
+        $ext1 = new FooTypeBarExtension();
+        $ext2 = new FooTypeBazExtension();
+
+        $this->extension1->addTypeExtension($ext1);
+        $this->extension2->addTypeExtension($ext2);
+
+        $this->factory->addType($type);
+
+        $this->assertEquals(array($ext1, $ext2), $type->getExtensions());
+    }
+
+    public function testGetTypeFromExtension()
+    {
+        $type = new FooType();
+        $this->extension2->addType($type);
+
+        $this->assertSame($type, $this->factory->getType('foo'));
+    }
+
+    public function testGetTypeAddsExtensions()
+    {
+        $type = new FooType();
+        $ext1 = new FooTypeBarExtension();
+        $ext2 = new FooTypeBazExtension();
+
+        $this->extension1->addTypeExtension($ext1);
+        $this->extension2->addTypeExtension($ext2);
+        $this->extension2->addType($type);
+
+        $type = $this->factory->getType('foo');
+
+        $this->assertEquals(array($ext1, $ext2), $type->getExtensions());
+    }
+
+    /**
+     * @expectedException Symfony\Component\Form\Exception\FormException
+     */
+    public function testGetTypeExpectsExistingType()
+    {
+        $this->factory->getType('bar');
+    }
+
+    public function testCreateNamedBuilder()
+    {
+        $type = new FooType();
+        $this->extension1->addType($type);
+
+        $builder = $this->factory->createNamedBuilder('foo', 'bar');
+
+        $this->assertTrue($builder instanceof FormBuilder);
+        $this->assertEquals('bar', $builder->getName());
+    }
+
+    public function testCreateNamedBuilderCallsBuildFormMethods()
+    {
+        $type = new FooType();
+        $ext1 = new FooTypeBarExtension();
+        $ext2 = new FooTypeBazExtension();
+
+        $this->extension1->addTypeExtension($ext1);
+        $this->extension2->addTypeExtension($ext2);
+        $this->extension2->addType($type);
+
+        $builder = $this->factory->createNamedBuilder('foo', 'bar');
+
+        $this->assertTrue($builder->hasAttribute('foo'));
+        $this->assertTrue($builder->hasAttribute('bar'));
+        $this->assertTrue($builder->hasAttribute('baz'));
+    }
+
+    public function testCreateNamedBuilderFillsDataOption()
+    {
+        $type = new FooType();
+        $this->extension1->addType($type);
+
+        $builder = $this->factory->createNamedBuilder('foo', 'bar', 'xyz');
+
+        // see FooType::buildForm()
+        $this->assertEquals('xyz', $builder->getAttribute('data_option'));
+    }
+
+    public function testCreateNamedBuilderDoesNotOverrideExistingDataOption()
+    {
+        $type = new FooType();
+        $this->extension1->addType($type);
+
+        $builder = $this->factory->createNamedBuilder('foo', 'bar', 'xyz', array(
+            'data' => 'abc',
+        ));
+
+        // see FooType::buildForm()
+        $this->assertEquals('abc', $builder->getAttribute('data_option'));
+    }
+
+    /**
+     * @expectedException Symfony\Component\Form\Exception\TypeDefinitionException
+     */
+    public function testCreateNamedBuilderExpectsDataOptionToBeSupported()
+    {
+        $type = $this->getMock('Symfony\Component\Form\FormTypeInterface');
+        $type->expects($this->any())
+            ->method('getName')
+            ->will($this->returnValue('foo'));
+        $type->expects($this->any())
+            ->method('getExtensions')
+            ->will($this->returnValue(array()));
+        $type->expects($this->any())
+            ->method('getDefaultOptions')
+            ->will($this->returnValue(array(
+                'required' => false,
+                'max_length' => null,
+            )));
+
+        $this->extension1->addType($type);
+
+        $this->factory->createNamedBuilder('foo', 'bar');
+    }
+
+    /**
+     * @expectedException Symfony\Component\Form\Exception\TypeDefinitionException
+     */
+    public function testCreateNamedBuilderExpectsRequiredOptionToBeSupported()
+    {
+        $type = $this->getMock('Symfony\Component\Form\FormTypeInterface');
+        $type->expects($this->any())
+            ->method('getName')
+            ->will($this->returnValue('foo'));
+        $type->expects($this->any())
+            ->method('getExtensions')
+            ->will($this->returnValue(array()));
+        $type->expects($this->any())
+            ->method('getDefaultOptions')
+            ->will($this->returnValue(array(
+                'data' => null,
+                'max_length' => null,
+            )));
+
+        $this->extension1->addType($type);
+
+        $this->factory->createNamedBuilder('foo', 'bar');
+    }
+
+    /**
+     * @expectedException Symfony\Component\Form\Exception\TypeDefinitionException
+     */
+    public function testCreateNamedBuilderExpectsMaxLengthOptionToBeSupported()
+    {
+        $type = $this->getMock('Symfony\Component\Form\FormTypeInterface');
+        $type->expects($this->any())
+            ->method('getName')
+            ->will($this->returnValue('foo'));
+        $type->expects($this->any())
+            ->method('getExtensions')
+            ->will($this->returnValue(array()));
+        $type->expects($this->any())
+            ->method('getDefaultOptions')
+            ->will($this->returnValue(array(
+                'data' => null,
+                'required' => false,
+            )));
+
+        $this->extension1->addType($type);
+
+        $this->factory->createNamedBuilder('foo', 'bar');
+    }
+
+    /**
+     * @expectedException Symfony\Component\Form\Exception\TypeDefinitionException
+     */
+    public function testCreateNamedBuilderExpectsBuilderToBeReturned()
+    {
+        $type = $this->getMock('Symfony\Component\Form\FormTypeInterface');
+        $type->expects($this->any())
+            ->method('getName')
+            ->will($this->returnValue('foo'));
+        $type->expects($this->any())
+            ->method('getExtensions')
+            ->will($this->returnValue(array()));
+        $type->expects($this->any())
+            ->method('getDefaultOptions')
+            ->will($this->returnValue(array(
+                'data' => null,
+                'required' => false,
+                'max_length' => null,
+            )));
+        $type->expects($this->any())
+            ->method('createBuilder')
+            ->will($this->returnValue(null));
+
+        $this->extension1->addType($type);
+
+        $this->factory->createNamedBuilder('foo', 'bar');
+    }
+
+    /**
+     * @expectedException Symfony\Component\Form\Exception\CreationException
+     */
+    public function testCreateNamedBuilderExpectsOptionsToExist()
+    {
+        $type = new FooType();
+        $this->extension1->addType($type);
+
+        $this->factory->createNamedBuilder('foo', 'bar', null, array(
+            'invalid' => 'xyz',
+        ));
+    }
+
+    public function testCreateNamedBuilderAddsTypeInstances()
+    {
+        $type = new FooType();
+        $this->assertFalse($this->factory->hasType('foo'));
+
+        $builder = $this->factory->createNamedBuilder($type, 'bar');
+
+        $this->assertTrue($builder instanceof FormBuilder);
+        $this->assertTrue($this->factory->hasType('foo'));
+    }
+
+    public function testCreateUsesTypeNameAsName()
+    {
+        $type = new FooType();
+        $this->extension1->addType($type);
+
+        $builder = $this->factory->createBuilder('foo');
+
+        $this->assertEquals('foo', $builder->getName());
+    }
+
     public function testCreateBuilderForPropertyCreatesFieldWithHighestConfidence()
     {
         $this->guesser1->expects($this->once())