[Form] Improved Form::add() and FormBuilder::add() to accept integers…

… as field names
symfony · fabpot · Dec 18, 2012 · Dec 14, 2012 · Dec 14, 2012 · Dec 18, 2012
commit 19d8510288c8bcd0724027e7842599916b796890
diff --git a/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php b/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php
@@ -83,7 +83,7 @@ public function preSetData(FormEvent $event)
 
         // Then add all rows again in the correct order
         foreach ($data as $name => $value) {
-            $form->add((string) $name, $this->type, array_replace(array(
+            $form->add($name, $this->type, array_replace(array(
                 'property_path' => '['.$name.']',
             ), $this->options));
         }
@@ -115,7 +115,7 @@ public function preBind(FormEvent $event)
         if ($this->allowAdd) {
             foreach ($data as $name => $value) {
                 if (!$form->has($name)) {
-                    $form->add((string) $name, $this->type, array_replace(array(
+                    $form->add($name, $this->type, array_replace(array(
                         'property_path' => '['.$name.']',
                     ), $this->options));
                 }

diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
@@ -256,7 +256,7 @@ private function addSubForms(FormBuilderInterface $builder, array $choiceViews,
                     $choiceType = 'radio';
                 }
 
-                $builder->add((string) $i, $choiceType, $choiceOpts);
+                $builder->add($i, $choiceType, $choiceOpts);
             }
         }
     }

diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php
@@ -890,8 +890,8 @@ public function add($child, $type = null, array $options = array())
         }
 
         if (!$child instanceof FormInterface) {
-            if (!is_string($child)) {
-                throw new UnexpectedTypeException($child, 'string or Symfony\Component\Form\FormInterface');
+            if (!is_string($child) && !is_int($child)) {
+                throw new UnexpectedTypeException($child, 'string, integer or Symfony\Component\Form\FormInterface');
             }
 
             if (null !== $type && !is_string($type) && !$type instanceof FormTypeInterface) {

diff --git a/src/Symfony/Component/Form/FormBuilder.php b/src/Symfony/Component/Form/FormBuilder.php
@@ -78,8 +78,8 @@ public function add($child, $type = null, array $options = array())
             return $this;
         }
 
-        if (!is_string($child)) {
-            throw new UnexpectedTypeException($child, 'string or Symfony\Component\Form\FormBuilder');
+        if (!is_string($child) && !is_int($child)) {
+            throw new UnexpectedTypeException($child, 'string, integer or Symfony\Component\Form\FormBuilder');
         }
 
        
8000
 if (null !== $type && !is_string($type) && !$type instanceof FormTypeInterface) {

diff --git a/src/Symfony/Component/Form/FormBuilderInterface.php b/src/Symfony/Component/Form/FormBuilderInterface.php
@@ -23,9 +23,9 @@ interface FormBuilderInterface extends \Traversable, \Countable, FormConfigBuild
      * If you add a nested group, this group should also be represented in the
      * object hierarchy.
      *
-     * @param string|FormBuilderInterface $child
-     * @param string|FormTypeInterface    $type
-     * @param array                       $options
+     * @param string|integer|FormBuilderInterface $child
+     * @param string|FormTypeInterface            $type
+     * @param array                               $options
      *
      * @return FormBuilderInterface The builder object.
      */

diff --git a/src/Symfony/Component/Form/FormConfigBuilder.php b/src/Symfony/Component/Form/FormConfigBuilder.php
@@ -144,7 +144,7 @@ class FormConfigBuilder implements FormConfigBuilderInterface
     /**
      * Creates an empty form configuration.
      *
-     * @param string                   $name       The form name
+     * @param string|integer           $name       The form name
      * @param string                   $dataClass  The class of the form's data
      * @param EventDispatcherInterface $dispatcher The event dispatcher
      * @param array                    $options    The form options
@@ -154,15 +154,13 @@ class FormConfigBuilder implements FormConfigBuilderInterface
      */
     public function __construct($name, $dataClass, EventDispatcherInterface $dispatcher, array $options = array())
     {
-        $name = (string) $name;
-
         self::validateName($name);
 
         if (null !== $dataClass && !class_exists($dataClass)) {
             throw new \InvalidArgumentException(sprintf('The data class "%s" is not a valid class.', $dataClass));
         }
 
-        $this->name = $name;
+        $this->name = (string) $name;
         $this->dataClass = $dataClass;
         $this->dispatcher = $dispatcher;
         $this->options = $options;
@@ -895,15 +893,15 @@ public function getFormConfig()
     /**
      * Validates whether the given variable is a valid form name.
      *
-     * @param string $name The tested form name.
+     * @param string|integer $name The tested form name.
      *
-     * @throws UnexpectedTypeException   If the name is not a string.
+     * @throws UnexpectedTypeException   If the name is not a string or an integer.
      * @throws \InvalidArgumentException If the name contains invalid characters.
      */
     public static function validateName($name)
     {
-        if (!is_string($name)) {
-            throw new UnexpectedTypeException($name, 'string');
+        if (null !== $name && !is_string($name) && !is_int($name)) {
+            throw new UnexpectedTypeException($name, 'string, integer or null');
         }
 
         if (!self::isValidName($name)) {
@@ -930,6 +928,6 @@ public static function validateName($name)
      */
     public static function isValidName($name)
     {
-        return '' === $name || preg_match('/^[a-zA-Z0-9_][a-zA-Z0-9_\-:]*$/D', $name);
+        return '' === $name || null === $name || preg_match('/^[a-zA-Z0-9_][a-zA-Z0-9_\-:]*$/D', $name);
     }
 }
diff --git a/src/Symfony/Component/Form/FormFactoryInterface.php b/src/Symfony/Component/Form/FormFactoryInterface.php
@@ -37,7 +37,7 @@ public function create($type = 'form', $data = null, array $options = array(), F
      *
      * @see createNamedBuilder()
      *
-     * @param string                   $name    The name of the form
+     * @param string|integer           $name    The name of the form
      * @param string|FormTypeInterface $type    The type of the form
      * @param mixed                    $data    The initial data
      * @param array                    $options The options
@@ -83,7 +83,7 @@ public function createBuilder($type = 'form', $data = null, array $options = arr
     /**
      * Returns a form builder.
      *
-     * @param string                   $name    The name of the form
+     * @param string|integer           $name    The name of the form
      * @param string|FormTypeInterface $type    The type of the form
      * @param mixed                    $data    The initial data
      * @param array                    $options The options

diff --git a/src/Symfony/Component/Form/FormInterface.php b/src/Symfony/Component/Form/FormInterface.php
@@ -41,9 +41,9 @@ public function getParent();
     /**
      * Adds a child to the form.
      *
-     * @param FormInterface|string $child   The FormInterface instance or the name of the child.
-     * @param string|null          $type    The child's type, if a name was passed.
-     * @param array                $options The child's options, if a name was passed.
+     * @param FormInterface|string|integer $child   The FormInterface instance or the name of the child.
+     * @param string|null                  $type    The child's type, if a name was passed.
+     * @param array                        $options The child's options, if a name was passed.
      *
      * @return FormInterface The form instance
      *

diff --git a/src/Symfony/Component/Form/Tests/CompoundFormTest.php b/src/Symfony/Component/Form/Tests/CompoundFormTest.php
@@ -156,6 +156,23 @@ public function testAddUsingNameAndType()
         $this->assertSame(array('foo' => $child), $this->form->all());
     }
 
+    public function testAddUsingIntegerNameAndType()
+    {
+        $child = $this->getBuilder(0)->getForm();
+
+        $this->factory->expects($this->once())
+            ->method('createNamed')
+            ->with('0', 'text', null, array('bar' => 'baz'))
+            ->will($this->returnValue($child));
+
+        // in order to make casting unnecessary
+        $this->form->add(0, 'text', array('bar' => 'baz'));
+
+        $this->assertTrue($this->form->has(0));
+        $this->assertSame($this->form, $child->getParent());
+        $this->assertSame(array(0 => $child), $this->form->all());
+    }
+
     public function testAddUsingNameButNoType()
     {
         $this->form = $this->getBuilder('name', null, '\stdClass')

diff --git a/src/Symfony/Component/Form/Tests/FormBuilderTest.php b/src/Symfony/Component/Form/Tests/FormBuilderTest.php
@@ -50,10 +50,10 @@ public function testNoSetName()
         $this->assertFalse(method_exists($this->builder, 'setName'));
     }
 
-    public function testAddNameNoString()
+    public function testAddNameNoStringAndNoInteger()
     {
         $this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException');
-        $this->builder->add(1234);
+        $this->builder->add(true);
     }
 
     public function testAddTypeNoString()
@@ -82,6 +82,13 @@ public function testAdd()
         $this->assertTrue($this->builder->has('foo'));
     }
 
+    public function testAddIntegerName()
+    {
+        $this->assertFalse($this->builder->has(0));
+        $this->builder->add(0, 'text');
+        $this->assertTrue($this->builder->has(0));
+    }
+
     public function testAll()
     {
         $this->factory->expects($this->once())

diff --git a/src/Symfony/Component/Form/Tests/FormConfigTest.php b/src/Symfony/Component/Form/Tests/FormConfigTest.php
@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Form\Tests;
 
+use Symfony\Component\Form\Exception\UnexpectedTypeException;
+
 /**
  * @author Bernhard Schussek <bschussek@gmail.com>
  */
@@ -21,8 +23,6 @@ class FormConfigTest extends \PHPUnit_Framework_TestCase
     public function getHtml4Ids()
     {
         return array(
-            array('a0', true),
-            array('a9', true),
             array('z0', true),
             array('A0', true),
             array('A9', true),
@@ -53,6 +53,16 @@ public function getHtml4Ids()
             // For root forms, leading underscores will be stripped from the
             // "id" attribute to produce valid HTML4.
             array('_', true),
+            // Integers are allowed
+            array(0, true),
+            array(123, true),
+            // NULL is allowed
+            array(null, true),
+            // Other types are not
+            array(1.23, false),
+            array(5., false),
+            array(true, false),
+            array(new \stdClass(), false),
         );
     }
 
@@ -68,6 +78,11 @@ public function testNameAcceptsOnlyNamesValidAsIdsInHtml4($name, $accepted)
             if (!$accepted) {
                 $this->fail(sprintf('The value "%s" should not be accepted', $name));
             }
+        } catch (UnexpectedTypeException $e) {
+            // if the value was not accepted, but should be, rethrow exception
+            if ($accepted) {
+                throw $e;
+            }
         } catch (\InvalidArgumentException $e) {
             // if the value was not accepted, but should be, rethrow exception
             if ($accepted) {
-Original file line number
+Diff line change
@@ Expand Up @@
                         $choiceType = 'radio';
                     }
-                    $builder->add((string) $i, $choiceType, $choiceOpts);
+                    $builder->add($i, $choiceType, $choiceOpts);
                 }
             }
         }
@@ Expand Down @@