merged branch bamarni/preloaded-extension (PR #5479)

This PR was merged into the 2.1 branch.

Commits
-------

84635bd [Form] allowed no type guesser to be registered

Discussion
----------

[Form] made the factory builder pass null when no type guesser registered

reopened #5422 against 2.1 as it's a bug fix

---------------------------------------------------------------------------

by stof at 2012-10-13T21:23:34Z

@fabpot anything left for this PR ?

---------------------------------------------------------------------------

by fabpot at 2012-10-14T09:41:29Z

@bamarni Can you add some unit tests and also update the FormExtensionInterface interface phpdoc as `getTypeGuesser` can now return `null`? Thanks. ping @bschussek

---------------------------------------------------------------------------

by bamarni at 2012-10-14T17:10:27Z

I've added a few tests covering this.

@fabpot : the phpdoc is already correct, it currently can return null, this only occurs with this convenient class.

---------------------------------------------------------------------------

by bschussek at 2012-10-16T07:43:41Z

This PR breaks FormFactory::createBuilderForProperty(), which expects a guesser to be present. Can you check the component for other uses of the guesser and add a null-check there?

---------------------------------------------------------------------------

by bamarni at 2012-10-16T10:57:54Z

I cannot find other places than the factory (searching for 'getTypeGuesser').

---------------------------------------------------------------------------

by bschussek at 2012-11-08T16:58:37Z

You should also adapt `FormRegistry::getTypeGuesser()` not to build a `FormTypeGuesserChain` if the array of guessers is empty. In that case it will return now `null` (adapt the doc block). We also need a different was of checking if the type guessers have already been parsed in FormRegistry. Otherwise the first if condition in `FormRegistry::getTypeGuesser()` will never become false. You could for example initialize the property `$guesser` to `false` and only set it to `null` after the first run of `getTypeGuesser()`.

---------------------------------------------------------------------------

by bamarni at 2012-11-08T18:40:00Z

good catch I had missed it! I've applied your suggestion in the latest commit. Do you see anything else before I squash?

---------------------------------------------------------------------------

by bschussek at 2012-11-08T18:45:15Z

A test for `FormRegistry::getTypeGuesser()` would of course be awesome.

---------------------------------------------------------------------------

by bamarni at 2012-11-08T18:52:13Z

Then it was already awesome! (see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Tests/FormRegistryTest.php#L252)

I've also added one for the null case if it's what you meant.

src/Symfony/Component/Form/FormFactory.php

@@ -92,7 +92,10 @@ class FormFactory implements FormFactoryInterface
      */
     public function createBuilderForProperty($class, $property, $data = null, array $options = array(), FormBuilderInterface $parent = null)
     {
-        $guesser = $this->registry->getTypeGuesser();
+        if (null === $guesser = $this->registry->getTypeGuesser()) {
+            return $this->createNamedBuilder($property, 'text', $data, $options, $parent);
+        }
+
         $typeGuess = $guesser->guessType($class, $property);
         $maxLengthGuess = $guesser->guessMaxLength($class, $property);
         // Keep $minLengthGuess for BC until Symfony 2.3

src/Symfony/Component/Form/FormFactoryBuilder.php

@@ -145,9 +145,11 @@ class FormFactoryBuilder implements FormFactoryBuilderInterface
         $extensions = $this->extensions;
 
         if (count($this->types) > 0 || count($this->typeExtensions) > 0 || count($this->typeGuessers) > 0) {
-            $typeGuesser = count($this->typeGuessers) > 1
-                ? new FormTypeGuesserChain($this->typeGuessers)
-                : $this->typeGuessers[0];
+            if (count($this->typeGuessers) > 1) {
+                $typeGuesser = new FormTypeGuesserChain($this->typeGuessers);
+            } else {
+                $typeGuesser = isset($this->typeGuessers[0]) ? $this->typeGuessers[0] : null;
+            }
 
             $extensions[] = new PreloadedExtension($this->types, $this->typeExtensions, $typeGuesser);
         }

src/Symfony/Component/Form/FormRegistry.php

@@ -33,9 +33,9 @@ class FormRegistry implements FormRegistryInterface
     private $types = array();
 
     /**
-     * @var FormTypeGuesserInterface
+     * @var FormTypeGuesserInterface|false|null
      */
-    private $guesser;
+    private $guesser = false;
 
     /**
      * @var ResolvedFormTypeFactoryInterface
@@ -158,7 +158,7 @@ class FormRegistry implements FormRegistryInterface
      */
     public function getTypeGuesser()
     {
-        if (null === $this->guesser) {
+        if (false === $this->guesser) {
             $guessers = array();
 
             foreach ($this->extensions as $extension) {
@@ -170,7 +170,7 @@ class FormRegistry implements FormRegistryInterface
                 }
             }
 
-            $this->guesser = new FormTypeGuesserChain($guessers);
+            $this->guesser = !empty($guessers) ? new FormTypeGuesserChain($guessers) : null;
         }
 
         return $this->guesser;

src/Symfony/Component/Form/FormRegistryInterface.php

@@ -55,7 +55,7 @@ interface FormRegistryInterface
     /**
      * Returns the guesser responsible for guessing types.
      *
-     * @return FormTypeGuesserInterface
+     * @return FormTypeGuesserInterface|null
      */
     public function getTypeGuesser();
 

src/Symfony/Component/Form/PreloadedExtension.php

@@ -38,11 +38,11 @@ class PreloadedExtension implements FormExtensionInterface
     /**
      * Creates a new preloaded extension.
      *
-     * @param array                    $types          The types that the extension should support.
-     * @param array                    $typeExtensions The type extensions that the extension should support.
-     * @param FormTypeGuesserInterface $typeGuesser    The guesser that the extension should support.
+     * @param array                         $types          The types that the extension should support.
+     * @param array                         $typeExtensions The type extensions that the extension should support.
+     * @param FormTypeGuesserInterface|null $typeGuesser    The guesser that the extension should support.
      */
-    public function __construct(array $types, array $typeExtensions, FormTypeGuesserInterface $typeGuesser)
+    public function __construct(array $types, array $typeExtensions, FormTypeGuesserInterface $typeGuesser = null)
     {
         $this->types = $types;
         $this->typeExtensions = $typeExtensions;

src/Symfony/Component/Form/Tests/FormFactoryBuilderTest.php

@@ -0,0 +1,59 @@
+<?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\Tests;
+
+use Symfony\Component\Form\FormFactoryBuilder;
+use Symfony\Component\Form\Tests\Fixtures\FooType;
+
+class FormFactoryBuilderTest extends \PHPUnit_Framework_TestCase
+{
+    private $registry;
+    private $guesser;
+    private $type;
+
+    protected function setUp()
+    {
+        $factory = new \ReflectionClass('Symfony\Component\Form\FormFactory');
+        $this->registry = $factory->getProperty('registry');
+        $this->registry->setAccessible(true);
+
+        $this->guesser = $this->getMock('Symfony\Component\Form\FormTypeGuesserInterface');
+        $this->type = new FooType;
+    }
+
+    public function testAddType()
+    {
+        $factoryBuilder = new FormFactoryBuilder;
+        $factoryBuilder->addType($this->type);
+
+        $factory = $factoryBuilder->getFormFactory();
+        $registry = $this->registry->getValue($factory);
+        $extensions = $registry->getExtensions();
+
+        $this->assertCount(1, $extensions);
+        $this->assertTrue($extensions[0]->hasType($this->type->getName()));
+        $this->assertNull($extensions[0]->getTypeGuesser());
+    }
+
+    public function testAddTypeGuesser()
+    {
+        $factoryBuilder = new FormFactoryBuilder;
+        $factoryBuilder->addTypeGuesser($this->guesser);
+
+        $factory = $factoryBuilder->getFormFactory();
+        $registry = $this->registry->getValue($factory);
+        $extensions = $registry->getExtensions();
+
+        $this->assertCount(1, $extensions);
+        $this->assertNotNull($extensions[0]->getTypeGuesser());
+    }
+}

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

@@ -348,6 +348,24 @@ class FormFactoryTest extends \PHPUnit_Framework_TestCase
         $this->assertSame('FORM', $this->factory->createNamed('name', 'type', null, $options));
     }
 
+    public function testCreateBuilderForPropertyWithoutTypeGuesser()
+    {
+        $registry = $this->getMock('Symfony\Component\Form\FormRegistryInterface');
+        $factory = $this->getMockBuilder('Symfony\Component\Form\FormFactory')
+            ->setMethods(array('createNamedBuilder'))
+            ->setConstructorArgs(array($registry, $this->resolvedTypeFactory))
+            ->getMock();
+
+        $factory->expects($this->once())
+            ->method('createNamedBuilder')
+            ->with('firstName', 'text', null, array())
+            ->will($this->returnValue('builderInstance'));
+
+        $builder = $factory->createBuilderForProperty('Application\Author', 'firstName');
+
+        $this->assertEquals('builderInstance', $builder);
+    }
+
     public function testCreateBuilderForPropertyCreatesFormWithHighestConfidence()
     {
         $this->guesser1->expects($this->once())

src/Symfony/Component/Form/Tests/FormRegistryTest.php

@@ -252,6 +252,12 @@ class FormRegistryTest extends \PHPUnit_Framework_TestCase
         $expectedGuesser = new FormTypeGuesserChain(array($this->guesser1, $this->guesser2));
 
         $this->assertEquals($expectedGuesser, $this->registry->getTypeGuesser());
+
+        $registry = new FormRegistry(
+            array($this->getMock('Symfony\Component\Form\FormExtensionInterface')),
+            $this->resolvedTypeFactory);
+
+        $this->assertNull($registry->getTypeGuesser());
     }
 
     public function testGetExtensions()