From fb71964adc69690844fca1dd2a035dac24eaa8a9 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Fri, 14 Dec 2012 16:05:05 +0100 Subject: [PATCH 1/2] [Form] Added an alternative signature Form::add($name, $type, $options) --- UPGRADE-2.2.md | 9 ++- .../EventListener/TranslationFormListener.php | 7 +- .../Propel1/Form/Type/TranslationType.php | 5 +- src/Symfony/Component/Form/CHANGELOG.md | 2 + .../Core/EventListener/ResizeFormListener.php | 17 ++--- .../Extension/Core/Type/CollectionType.php | 1 - src/Symfony/Component/Form/Form.php | 19 +++++- src/Symfony/Component/Form/FormBuilder.php | 21 +----- .../Component/Form/FormBuilderInterface.php | 23 +++++-- .../Component/Form/FormConfigBuilder.php | 27 ++++++++ .../Form/FormConfigBuilderInterface.php | 7 ++ .../Component/Form/FormConfigInterface.php | 7 ++ src/Symfony/Component/Form/FormInterface.php | 11 ++-- .../Component/Form/Tests/CompoundFormTest.php | 65 ++++++++++++++++++- .../EventListener/ResizeFormListenerTest.php | 34 +++++----- 15 files changed, 184 insertions(+), 71 deletions(-) diff --git a/UPGRADE-2.2.md b/UPGRADE-2.2.md index 39fb89bc0b2b9..864cf983f9617 100644 --- a/UPGRADE-2.2.md +++ b/UPGRADE-2.2.md @@ -37,7 +37,14 @@ ### Form - * The PasswordType is now not trimmed by default. + * The PasswordType is now not trimmed by default. + +#### Deprecations + + * The methods `getParent()`, `setParent()` and `hasParent()` in + `FormBuilderInterface` were deprecated and will be removed in Symfony 2.3. + You should not rely on these methods in your form type because the parent + of a form can change after building it. ### Routing diff --git a/src/Symfony/Bridge/Propel1/Form/EventListener/TranslationFormListener.php b/src/Symfony/Bridge/Propel1/Form/EventListener/TranslationFormListener.php index 27dc7e65d2af6..3f20102c7c382 100644 --- a/src/Symfony/Bridge/Propel1/Form/EventListener/TranslationFormListener.php +++ b/src/Symfony/Bridge/Propel1/Form/EventListener/TranslationFormListener.php @@ -11,7 +11,6 @@ namespace Symfony\Bridge\Propel1\Form\EventListener; use Symfony\Component\EventDispatcher\EventSubscriberInterface; -use Symfony\Component\Form\FormFactoryInterface; use Symfony\Component\Form\FormEvent; use Symfony\Component\Form\FormEvents; @@ -24,13 +23,11 @@ class TranslationFormListener implements EventSubscriberInterface { private $columns; private $dataClass; - private $formFactory; - public function __construct(FormFactoryInterface $formFactory, $columns, $dataClass) + public function __construct($columns, $dataClass) { $this->columns = $columns; $this->dataClass = $dataClass; - $this->formFactory = $formFactory; } public static function getSubscribedEvents() @@ -78,7 +75,7 @@ public function preSetData(FormEvent $event) $options = array_merge($options, $customOptions); - $form->add($this->formFactory->createNamed($column, $type, null, $options)); + $form->add($column, $type, $options); } } } diff --git a/src/Symfony/Bridge/Propel1/Form/Type/TranslationType.php b/src/Symfony/Bridge/Propel1/Form/Type/TranslationType.php index 19e777babda5f..1bd94a484ce3a 100644 --- a/src/Symfony/Bridge/Propel1/Form/Type/TranslationType.php +++ b/src/Symfony/Bridge/Propel1/Form/Type/TranslationType.php @@ -28,8 +28,9 @@ class TranslationType extends AbstractType */ public function buildForm(FormBuilderInterface $builder, array $options) { - $listener = new TranslationFormListener($builder->getFormFactory(), $options['columns'], $options['data_class']); - $builder->addEventSubscriber($listener); + $builder->addEventSubscriber( + new TranslationFormListener($options['columns'], $options['data_class']) + ); } /** diff --git a/src/Symfony/Component/Form/CHANGELOG.md b/src/Symfony/Component/Form/CHANGELOG.md index bdbb2b3546c3d..3f26b8169c104 100644 --- a/src/Symfony/Component/Form/CHANGELOG.md +++ b/src/Symfony/Component/Form/CHANGELOG.md @@ -5,6 +5,8 @@ CHANGELOG ----- * TrimListener now removes unicode whitespaces + * deprecated getParent(), setParent() and hasParent() in FormBuilderInterface + * FormInterface::add() now accepts a FormInterface instance OR a field's name, type and options 2.1.0 ----- diff --git a/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php b/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php index 7c34a9916188c..a3719f56f4ee6 100644 --- a/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php +++ b/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php @@ -13,7 +13,6 @@ use Symfony\Component\Form\FormEvents; use Symfony\Component\Form\FormEvent; -use Symfony\Component\Form\FormFactoryInterface; use Symfony\Component\Form\Exception\UnexpectedTypeException; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -24,11 +23,6 @@ */ class ResizeFormListener implements EventSubscriberInterface { - /** - * @var FormFactoryInterface - */ - protected $factory; - /** * @var string */ @@ -51,9 +45,8 @@ class ResizeFormListener implements EventSubscriberInterface */ protected $allowDelete; - public function __construct(FormFactoryInterface $factory, $type, array $options = array(), $allowAdd = false, $allowDelete = false) + public function __construct($type, array $options = array(), $allowAdd = false, $allowDelete = false) { - $this->factory = $factory; $this->type = $type; $this->allowAdd = $allowAdd; $this->allowDelete = $allowDelete; @@ -90,9 +83,9 @@ public function preSetData(FormEvent $event) // Then add all rows again in the correct order foreach ($data as $name => $value) { - $form->add($this->factory->createNamed($name, $this->type, null, array_replace(array( + $form->add((string) $name, $this->type, array_replace(array( 'property_path' => '['.$name.']', - ), $this->options))); + ), $this->options)); } } @@ -122,9 +115,9 @@ public function preBind(FormEvent $event) if ($this->allowAdd) { foreach ($data as $name => $value) { if (!$form->has($name)) { - $form->add($this->factory->createNamed($name, $this->type, null, array_replace(array( + $form->add((string) $name, $this->type, array_replace(array( 'property_path' => '['.$name.']', - ), $this->options))); + ), $this->options)); } } } diff --git a/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php b/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php index 353a89bc4bb21..3ff36b97277d5 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php @@ -34,7 +34,6 @@ public function buildForm(FormBuilderInterface $builder, array $options) } $resizeListener = new ResizeFormListener( - $builder->getFormFactory(), $options['type'], $options['options'], $options['allow_add'], diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 307db6798f855..af01ebc06dfde 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Form; use Symfony\Component\Form\Exception\FormException; +use Symfony\Component\Form\Exception\UnexpectedTypeException; use Symfony\Component\Form\Exception\AlreadyBoundException; use Symfony\Component\Form\Exception\TransformationFailedException; use Symfony\Component\Form\Util\FormUtil; @@ -859,7 +860,7 @@ public function hasChildren() /** * {@inheritdoc} */ - public function add(FormInterface $child) + public function add($child, $type = null, array $options = array()) { if ($this->bound) { throw new AlreadyBoundException('You cannot add children to a bound form'); @@ -888,6 +889,22 @@ public function add(FormInterface $child) $viewData = $this->getViewData(); } + if (!$child instanceof FormInterface) { + if (!is_string($child)) { + throw new UnexpectedTypeException($child, 'string or Symfony\Component\Form\FormInterface'); + } + + if (null !== $type && !is_string($type) && !$type instanceof FormTypeInterface) { + throw new UnexpectedTypeException($type, 'string or Symfony\Component\Form\FormTypeInterface'); + } + + if (null === $type) { + $child = $this->config->getFormFactory()->createForProperty($this->config->getDataClass(), $child, null, $options); + } else { + $child = $this->config->getFormFactory()->createNamed($child, $type, null, $options); + } + } + $this->children[$child->getName()] = $child; $child->setParent($this); diff --git a/src/Symfony/Component/Form/FormBuilder.php b/src/Symfony/Component/Form/FormBuilder.php index 86e5587eff17c..e49bf6250dd81 100644 --- a/src/Symfony/Component/Form/FormBuilder.php +++ b/src/Symfony/Component/Form/FormBuilder.php @@ -22,13 +22,6 @@ */ class FormBuilder extends FormConfigBuilder implements \IteratorAggregate, FormBuilderInterface { - /** - * The form factory. - * - * @var FormFactoryInterface - */ - private $factory; - /** * The children of the form builder. * @@ -63,15 +56,7 @@ public function __construct($name, $dataClass, EventDispatcherInterface $dispatc { parent::__construct($name, $dataClass, $dispatcher, $options); - $this->factory = $factory; - } - - /** - * {@inheritdoc} - */ - public function getFormFactory() - { - return $this->factory; + $this->setFormFactory($factory); } /** @@ -125,10 +110,10 @@ public function create($name, $type = null, array $options = array()) } if (null !== $type) { - return $this->factory->createNamedBuilder($name, $type, null, $options, $this); + return $this->getFormFactory()->createNamedBuilder($name, $type, null, $options, $this); } - return $this->factory->createBuilderForProperty($this->getDataClass(), $name, null, $options, $this); + return $this->getFormFactory()->createBuilderForProperty($this->getDataClass(), $name, null, $options, $this); } /** diff --git a/src/Symfony/Component/Form/FormBuilderInterface.php b/src/Symfony/Component/Form/FormBuilderInterface.php index 034a65a5a251f..d2a29fc8215c2 100644 --- a/src/Symfony/Component/Form/FormBuilderInterface.php +++ b/src/Symfony/Component/Form/FormBuilderInterface.php @@ -52,6 +52,7 @@ public function create($name, $type = null, array $options = array()); * @throws Exception\FormException if the given child does not exist */ public function get($name); + /** * Removes the field with the given name. * @@ -77,13 +78,6 @@ public function has($name); */ public function all(); - /** - * Returns the associated form factory. - * - * @return FormFactoryInterface The factory - */ - public function getFormFactory(); - /** * Creates the form. * @@ -97,6 +91,11 @@ public function getForm(); * @param FormBuilderInterface $parent The parent builder * * @return FormBuilderInterface The builder object. + * + * @deprecated Deprecated since version 2.2, to be removed in 2.3. You + * should not rely on the parent of a builder, because it is + * likely that the parent is only set after turning the builder + * into a form. */ public function setParent(FormBuilderInterface $parent = null); @@ -104,6 +103,11 @@ public function setParent(FormBuilderInterface $parent = null); * Returns the parent builder. * * @return FormBuilderInterface The parent builder + * + * @deprecated Deprecated since version 2.2, to be removed in 2.3. You + * should not rely on the parent of a builder, because it is + * likely that the parent is only set after turning the builder + * into a form. */ public function getParent(); @@ -111,6 +115,11 @@ public function getParent(); * Returns whether the builder has a parent. * * @return Boolean + * + * @deprecated Deprecated since version 2.2, to be removed in 2.3. You + * should not rely on the parent of a builder, because it is + * likely that the parent is only set after turning the builder + * into a form. */ public function hasParent(); } diff --git a/src/Symfony/Component/Form/FormConfigBuilder.php b/src/Symfony/Component/Form/FormConfigBuilder.php index 529fb891792d9..51077298a600d 100644 --- a/src/Symfony/Component/Form/FormConfigBuilder.php +++ b/src/Symfony/Component/Form/FormConfigBuilder.php @@ -131,6 +131,11 @@ class FormConfigBuilder implements FormConfigBuilderInterface */ private $dataLocked; + /** + * @var FormFactoryInterface + */ + private $formFactory; + /** * @var array */ @@ -611,6 +616,14 @@ public function getDataLocked() return $this->dataLocked; } + /** + * {@inheritdoc} + */ + public function getFormFactory() + { + return $this->formFactory; + } + /** * {@inheritdoc} */ @@ -849,6 +862,20 @@ public function setDataLocked($locked) return $this; } + /** + * {@inheritdoc} + */ + public function setFormFactory(FormFactoryInterface $formFactory) + { + if ($this->locked) { + throw new FormException('The config builder cannot be modified anymore.'); + } + + $this->formFactory = $formFactory; + + return $this; + } + /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/Form/FormConfigBuilderInterface.php b/src/Symfony/Component/Form/FormConfigBuilderInterface.php index 35eff8790bd29..496fc38c66aef 100644 --- a/src/Symfony/Component/Form/FormConfigBuilderInterface.php +++ b/src/Symfony/Component/Form/FormConfigBuilderInterface.php @@ -241,6 +241,13 @@ public function setData($data); */ public function setDataLocked($locked); + /** + * Sets the form factory used for creating new forms. + * + * @param FormFactoryInterface $formFactory The form factory. + */ + public function setFormFactory(FormFactoryInterface $formFactory); + /** * Builds and returns the form configuration. * diff --git a/src/Symfony/Component/Form/FormConfigInterface.php b/src/Symfony/Component/Form/FormConfigInterface.php index 5710f6e2adb8d..b5d914f6d3689 100644 --- a/src/Symfony/Component/Form/FormConfigInterface.php +++ b/src/Symfony/Component/Form/FormConfigInterface.php @@ -192,6 +192,13 @@ public function getDataClass(); */ public function getDataLocked(); + /** + * Returns the form factory used for creating new forms. + * + * @return FormFactoryInterface The form factory. + */ + public function getFormFactory(); + /** * Returns all options passed during the construction of the form. * diff --git a/src/Symfony/Component/Form/FormInterface.php b/src/Symfony/Component/Form/FormInterface.php index 63ac2e2eb0a30..b42e42dab9753 100644 --- a/src/Symfony/Component/Form/FormInterface.php +++ b/src/Symfony/Component/Form/FormInterface.php @@ -41,14 +41,17 @@ public function getParent(); /** * Adds a child to the form. * - * @param FormInterface $child The FormInterface to add as a child + * @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. * * @return FormInterface The form instance * - * @throws Exception\AlreadyBoundException If the form has already been bound. - * @throws Exception\FormException When trying to add a child to a non-compound form. + * @throws Exception\AlreadyBoundException If the form has already been bound. + * @throws Exception\FormException When trying to add a child to a non-compound form. + * @throws Exception\UnexpectedTypeException If $child or $type has an unexpected type. */ - public function add(FormInterface $child); + public function add($child, $type = null, array $options = array()); /** * Returns the child with the given name. diff --git a/src/Symfony/Component/Form/Tests/CompoundFormTest.php b/src/Symfony/Component/Form/Tests/CompoundFormTest.php index 05483f6496dd4..f40fa2b6d7957 100644 --- a/src/Symfony/Component/Form/Tests/CompoundFormTest.php +++ b/src/Symfony/Component/Form/Tests/CompoundFormTest.php @@ -33,7 +33,7 @@ public function testValidIfAllChildrenAreValid() $this->assertTrue($this->form->isValid()); } - public function testInvalidIfChildrenIsInvalid() + public function testInvalidIfChildIsInvalid() { $this->form->add($this->getValidForm('firstName')); $this->form->add($this->getInvalidForm('lastName')); @@ -135,12 +135,71 @@ public function testAdd() $child = $this->getBuilder('foo')->getForm(); $this->form->add($child); + $this->assertTrue($this->form->has('foo')); + $this->assertSame($this->form, $child->getParent()); + $this->assertSame(array('foo' => $child), $this->form->all()); + } + + public function testAddUsingNameAndType() + { + $child = $this->getBuilder('foo')->getForm(); + + $this->factory->expects($this->once()) + ->method('createNamed') + ->with('foo', 'text', null, array('bar' => 'baz')) + ->will($this->returnValue($child)); + + $this->form->add('foo', 'text', array('bar' => 'baz')); + + $this->assertTrue($this->form->has('foo')); + $this->assertSame($this->form, $child->getParent()); + $this->assertSame(array('foo' => $child), $this->form->all()); + } + + public function testAddUsingNameButNoType() + { + $this->form = $this->getBuilder('name', null, '\stdClass') + ->setCompound(true) + ->setDataMapper($this->getDataMapper()) + ->getForm(); + + $child = $this->getBuilder('foo')->getForm(); + + $this->factory->expects($this->once()) + ->method('createForProperty') + ->with('\stdClass', 'foo') + ->will($this->returnValue($child)); + + $this->form->add('foo'); + + $this->assertTrue($this->form->has('foo')); + $this->assertSame($this->form, $child->getParent()); + $this->assertSame(array('foo' => $child), $this->form->all()); + } + + public function testAddUsingNameButNoTypeAndOptions() + { + $this->form = $this->getBuilder('name', null, '\stdClass') + ->setCompound(true) + ->setDataMapper($this->getDataMapper()) + ->getForm(); + + $child = $this->getBuilder('foo')->getForm(); + + $this->factory->expects($this->once()) + ->method('createForProperty') + ->with('\stdClass', 'foo', null, array('bar' => 'baz')) + ->will($this->returnValue($child)); + + $this->form->add('foo', null, array('bar' => 'baz')); + + $this->assertTrue($this->form->has('foo')); $this->assertSame($this->form, $child->getParent()); $this->assertSame(array('foo' => $child), $this->form->all()); } /** - * @expectedException Symfony\Component\Form\Exception\AlreadyBoundException + * @expectedException \Symfony\Component\Form\Exception\AlreadyBoundException */ public function testAddThrowsExceptionIfAlreadyBound() { @@ -161,7 +220,7 @@ public function testRemove() } /** - * @expectedException Symfony\Component\Form\Exception\AlreadyBoundException + * @expectedException \Symfony\Component\Form\Exception\AlreadyBoundException */ public function testRemoveThrowsExceptionIfAlreadyBound() { diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/ResizeFormListenerTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/ResizeFormListenerTest.php index e271ad020aecb..201882e88819e 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/ResizeFormListenerTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/ResizeFormListenerTest.php @@ -81,7 +81,7 @@ public function testPreSetDataResizesForm() $data = array(1 => 'string', 2 => 'string'); $event = DeprecationErrorHandler::getFormEvent($this->form, $data); - $listener = new ResizeFormListener($this->factory, 'text', array('max_length' => '10'), false, false); + $listener = new ResizeFormListener('text', array('max_length' => '10'), false, false); $listener->preSetData($event); $this->assertFalse($this->form->has('0')); @@ -90,13 +90,13 @@ public function testPreSetDataResizesForm() } /** - * @expectedException Symfony\Component\Form\Exception\UnexpectedTypeException + * @expectedException \Symfony\Component\Form\Exception\UnexpectedTypeException */ public function testPreSetDataRequiresArrayOrTraversable() { $data = 'no array or traversable'; $event = DeprecationErrorHandler::getFormEvent($this->form, $data); - $listener = new ResizeFormListener($this->factory, 'text', array(), false, false); + $listener = new ResizeFormListener('text', array(), false, false); $listener->preSetData($event); } @@ -106,7 +106,7 @@ public function testPreSetDataDealsWithNullData() $data = null; $event = DeprecationErrorHandler::getFormEvent($this->form, $data); - $listener = new ResizeFormListener($this->factory, 'text', array(), false, false); + $listener = new ResizeFormListener('text', array(), false, false); $listener->preSetData($event); } @@ -121,7 +121,7 @@ public function testPreBindResizesUpIfAllowAdd() $data = array(0 => 'string', 1 => 'string'); $event = DeprecationErrorHandler::getFormEvent($this->form, $data); - $listener = new ResizeFormListener($this->factory, 'text', array('max_length' => 10), true, false); + $listener = new ResizeFormListener('text', array('max_length' => 10), true, false); $listener->preBind($event); $this->assertTrue($this->form->has('0')); @@ -135,7 +135,7 @@ public function testPreBindResizesDownIfAllowDelete() $data = array(0 => 'string'); $event = DeprecationErrorHandler::getFormEvent($this->form, $data); - $listener = new ResizeFormListener($this->factory, 'text', array(), false, true); + $listener = new ResizeFormListener('text', array(), false, true); $listener->preBind($event); $this->assertTrue($this->form->has('0')); @@ -149,7 +149,7 @@ public function testPreBindRemovesZeroKeys() $data = array(); $event = DeprecationErrorHandler::getFormEvent($this->form, $data); - $listener = new ResizeFormListener($this->factory, 'text', array(), false, true); + $listener = new ResizeFormListener('text', array(), false, true); $listener->preBind($event); $this->assertFalse($this->form->has('0')); @@ -162,7 +162,7 @@ public function testPreBindDoesNothingIfNotAllowAddNorAllowDelete() $data = array(0 => 'string', 2 => 'string'); $event = DeprecationErrorHandler::getFormEvent($this->form, $data); - $listener = new ResizeFormListener($this->factory, 'text', array(), false, false); + $listener = new ResizeFormListener('text', array(), false, false); $listener->preBind($event); $this->assertTrue($this->form->has('0')); @@ -171,13 +171,13 @@ public function testPreBindDoesNothingIfNotAllowAddNorAllowDelete() } /** - * @expectedException Symfony\Component\Form\Exception\UnexpectedTypeException + * @expectedException \Symfony\Component\Form\Exception\UnexpectedTypeException */ public function testPreBindRequiresArrayOrTraversable() { $data = 'no array or traversable'; $event = DeprecationErrorHandler::getFormEvent($this->form, $data); - $listener = new ResizeFormListener($this->factory, 'text', array(), false, false); + $listener = new ResizeFormListener('text', array(), false, false); $listener->preBind($event); } @@ -187,7 +187,7 @@ public function testPreBindDealsWithNullData() $data = null; $event = DeprecationErrorHandler::getFormEvent($this->form, $data); - $listener = new ResizeFormListener($this->factory, 'text', array(), false, true); + $listener = new ResizeFormListener('text', array(), false, true); $listener->preBind($event); $this->assertFalse($this->form->has('1')); @@ -200,7 +200,7 @@ public function testPreBindDealsWithEmptyData() $data = ''; $event = DeprecationErrorHandler::getFormEvent($this->form, $data); - $listener = new ResizeFormListener($this->factory, 'text', array(), false, true); + $listener = new ResizeFormListener('text', array(), false, true); $listener->preBind($event); $this->assertFalse($this->form->has('1')); @@ -212,7 +212,7 @@ public function testOnBindNormDataRemovesEntriesMissingInTheFormIfAllowDelete() $data = array(0 => 'first', 1 => 'second', 2 => 'third'); $event = DeprecationErrorHandler::getFormEvent($this->form, $data); - $listener = new ResizeFormListener($this->factory, 'text', array(), false, true); + $listener = new ResizeFormListener('text', array(), false, true); $listener->onBind($event); $this->assertEquals(array(1 => 'second'), $event->getData()); @@ -224,20 +224,20 @@ public function testOnBindNormDataDoesNothingIfNotAllowDelete() $data = array(0 => 'first', 1 => 'second', 2 => 'third'); $event = DeprecationErrorHandler::getFormEvent($this->form, $data); - $listener = new ResizeFormListener($this->factory, 'text', array(), false, false); + $listener = new ResizeFormListener('text', array(), false, false); $listener->onBind($event); $this->assertEquals($data, $event->getData()); } /** - * @expectedException Symfony\Component\Form\Exception\UnexpectedTypeException + * @expectedException \Symfony\Component\Form\Exception\UnexpectedTypeException */ public function testOnBindNormDataRequiresArrayOrTraversable() { $data = 'no array or traversable'; $event = DeprecationErrorHandler::getFormEvent($this->form, $data); - $listener = new ResizeFormListener($this->factory, 'text', array(), false, false); + $listener = new ResizeFormListener('text', array(), false, false); $listener->onBind($event); } @@ -247,7 +247,7 @@ public function testOnBindNormDataDealsWithNullData() $data = null; $event = DeprecationErrorHandler::getFormEvent($this->form, $data); - $listener = new ResizeFormListener($this->factory, 'text', array(), false, true); + $listener = new ResizeFormListener('text', array(), false, true); $listener->onBind($event); $this->assertEquals(array(), $event->getData()); From 19d8510288c8bcd0724027e7842599916b796890 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Fri, 14 Dec 2012 19:27:28 +0100 Subject: [PATCH 2/2] [Form] Improved Form::add() and FormBuilder::add() to accept integers as field names --- .../Core/EventListener/ResizeFormListener.php | 4 ++-- .../Form/Extension/Core/Type/ChoiceType.php | 2 +- src/Symfony/Component/Form/Form.php | 4 ++-- src/Symfony/Component/Form/FormBuilder.php | 4 ++-- .../Component/Form/FormBuilderInterface.php | 6 +++--- .../Component/Form/FormConfigBuilder.php | 16 +++++++--------- .../Component/Form/FormFactoryInterface.php | 4 ++-- src/Symfony/Component/Form/FormInterface.php | 6 +++--- .../Component/Form/Tests/CompoundFormTest.php | 17 +++++++++++++++++ .../Component/Form/Tests/FormBuilderTest.php | 11 +++++++++-- .../Component/Form/Tests/FormConfigTest.php | 19 +++++++++++++++++-- 11 files changed, 65 insertions(+), 28 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php b/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php index a3719f56f4ee6..12d1cba505b38 100644 --- 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 index 356cc2322c21a..a33a01b9cbe02 100644 --- 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 index af01ebc06dfde..b1895aa588438 100644 --- 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 index e49bf6250dd81..da193be94c306 100644 --- 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'); } 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 index d2a29fc8215c2..002bd43fc3ebf 100644 --- 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 index 51077298a600d..a3f16ba72023c 100644 --- 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 index fc90068479851..98c53d9b530e0 100644 --- 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 index b42e42dab9753..d217a07dd15df 100644 --- 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 index f40fa2b6d7957..4e4a48e1fcc7b 100644 --- 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 index 42d4317459d49..4ccb4ae76ef39 100644 --- 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 index e11561a47f8f6..a7626f86a688c 100644 --- 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 */ @@ -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) {