merged branch bschussek/issue5899 (PR #6573)

This PR was merged into the master branch.

Discussion
----------

[2.3] [Form] Renamed option "virtual" to "inherit_data" and improved handling of such forms

Bug fix: yes
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #5899, #5720, #5578
Todo: -
License of the code: MIT
Documentation PR: symfony/symfony-docs#2107

This PR renames the option "virtual" to "inherit_data" for more clarity (the old option is deprecated and usable until 2.3). It also fixes the behavior of forms having that option set.

Forms with that option set will now correctly return their parents' data from `getData()`, `getNormData()` and `getViewData()`. Furthermore, `getPropertyPath()` was fixed for forms that inherit their parent data.

Commits
-------

1290b80 [Form] Fixed the deprecation notes for the "virtual" option
ac2ca44 [Form] Moved parent data inheritance from data mappers to Form
8ea5e1a [Form] Renamed option "virtual" to "inherit_data"

UPGRADE-2.3.md

@@ -1,4 +1,4 @@
-UPGRADE FROM 2.2 to 2.3
+UPGRADE FROM 2.2 to 2.3
 =======================
 
 ### Form
@@ -35,6 +35,79 @@ UPGRADE FROM 2.2 to 2.3
    "validation_groups" => false
    "validation_groups" => array()
    ```
+ * The array type hint from DataMapperInterface was removed. You should adapt
+   implementations of that interface accordingly.
+
+   Before:
+
+   ```
+   use Symfony\Component\Form\DataMapperInterface;
+
+   class MyDataMapper
+   {
+       public function mapFormsToData(array $forms, $data)
+       {
+           // ...
+       }
+
+       public function mapDataToForms($data, array $forms)
+       {
+           // ...
+       }
+   }
+   ```
+
+   After:
+
+   ```
+   use Symfony\Component\Form\DataMapperInterface;
+
+   class MyDataMapper
+   {
+       public function mapFormsToData($forms, $data)
+       {
+           // ...
+       }
+
+       public function mapDataToForms($data, $forms)
+       {
+           // ...
+       }
+   }
+   ```
+
+   Instead of an array, the methods here are now passed a
+   RecursiveIteratorIterator containing an InheritDataAwareIterator by default,
+   so you don't need to handle forms inheriting their parent data (former
+   "virtual forms") in the data mapper anymore.
+
+   Before:
+
+   ```
+   use Symfony\Component\Form\Util\VirtualFormAwareIterator;
+
+   public function mapFormsToData(array $forms, $data)
+   {
+       $iterator = new \RecursiveIteratorIterator(
+           new VirtualFormAwareIterator($forms)
+       );
+
+       foreach ($iterator as $form) {
+           // ...
+       }
+   }
+   ```
+
+   After:
+
+   ```
+   public function mapFormsToData($forms, $data)
+   {
+       foreach ($forms as $form) {
+           // ...
+       }
+   }
+   ```
 
 ### PropertyAccess
 

UPGRADE-3.0.md

@@ -57,6 +57,42 @@ UPGRADE FROM 2.x to 3.0
    }
    ```
 
+ * The option "virtual" was renamed to "inherit_data".
+
+   Before:
+
+   ```
+   $builder->add('address', 'form', array(
+       'virtual' => true,
+   ));
+   ```
+
+   After:
+
+   ```
+   $builder->add('address', 'form', array(
+       'inherit_data' => true,
+   ));
+   ```
+
+ * The class VirtualFormAwareIterator was renamed to InheritDataAwareIterator.
+
+   Before:
+
+   ```
+   use Symfony\Component\Form\Util\VirtualFormAwareIterator;
+
+   $iterator = new VirtualFormAwareIterator($forms);
+   ```
+
+   After:
+
+   ```
+   use Symfony\Component\Form\Util\InheritDataAwareIterator;
+
+   $iterator = new InheritDataAwareIterator($forms);
+   ```
+
 ### FrameworkBundle
 
  * The `enctype` method of the `form` helper was removed. You should use the

src/Symfony/Component/Form/ButtonBuilder.php

@@ -492,6 +492,18 @@ public function setFormProcessor(FormProcessorInterface $formProcessor)
         throw new \BadMethodCallException('Buttons do not support form processors.');
     }
 
+    /**
+     * Unsupported method.
+     *
+     * @param Boolean $inheritData
+     *
+     * @throws \BadMethodCallException
+     */
+    public function setInheritData($inheritData)
+    {
+        throw new \BadMethodCallException('Buttons do not support data inheritance.');
+    }
+
     /**
      * Builds and returns the button configuration.
      *
@@ -759,6 +771,16 @@ public function getFormProcessor()
         return null;
     }
 
+    /**
+     * Unsupported method.
+     *
+     * @return null Always returns null.
+     */
+    public function getInheritData()
+    {
+        return null;
+    }
+
     /**
      * Returns all options passed during the construction of the button.
      *

src/Symfony/Component/Form/CHANGELOG.md

@@ -11,6 +11,10 @@ CHANGELOG
  * added FormProcessorInterface and FormInterface::process()
  * deprecated passing a Request instance to FormInterface::bind()
  * added options "method" and "action" to FormType
+ * deprecated option "virtual" in favor "inherit_data"
+ * deprecated VirtualFormAwareIterator in favor of InheritDataAwareIterator
+ * [BC BREAK] removed the "array" type hint from DataMapperInterface
+ * improved forms inheriting their parent data to actually return that data from getData(), getNormData() and getViewData()
 
 2.2.0
 -----

src/Symfony/Component/Form/DataMapperInterface.php

@@ -19,20 +19,20 @@ interface DataMapperInterface
     /**
      * Maps properties of some data to a list of forms.
      *
-     * @param mixed $data  Structured data.
-     * @param array $forms A list of {@link FormInterface} instances.
+     * @param mixed           $data  Structured data.
+     * @param FormInterface[] $forms A list of {@link FormInterface} instances.
      *
      * @throws Exception\UnexpectedTypeException if the type of the data parameter is not supported.
      */
-    public function mapDataToForms($data, array $forms);
+    public function mapDataToForms($data, $forms);
 
     /**
      * Maps the data of a list of forms into the properties of some data.
      *
-     * @param array $forms A list of {@link FormInterface} instances.
-     * @param mixed $data  Structured data.
+     * @param FormInterface[] $forms A list of {@link FormInterface} instances.
+     * @param mixed           $data  Structured data.
      *
      * @throws Exception\UnexpectedTypeException if the type of the data parameter is not supported.
      */
-    public function mapFormsToData(array $forms, &$data);
+    public function mapFormsToData($forms, &$data);
 }

src/Symfony/Component/Form/Exception/RuntimeException.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;
+
+/**
+ * Base RuntimeException for the Form component.
+ *
+ * @author Bernhard Schussek <bschussek@gmail.com>
+ */
+class RuntimeException extends \RuntimeException implements ExceptionInterface
+{
+}

src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php

@@ -12,7 +12,6 @@
 namespace Symfony\Component\Form\Extension\Core\DataMapper;
 
 use Symfony\Component\Form\DataMapperInterface;
-use Symfony\Component\Form\Util\VirtualFormAwareIterator;
 use Symfony\Component\Form\Exception\UnexpectedTypeException;
 use Symfony\Component\PropertyAccess\PropertyAccess;
 use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
@@ -42,7 +41,7 @@ public function __construct(PropertyAccessorInterface $propertyAccessor = null)
     /**
      * {@inheritdoc}
      */
-    public function mapDataToForms($data, array $forms)
+    public function mapDataToForms($data, $forms)
     {
         if (null === $data || array() === $data) {
             return;
@@ -52,11 +51,7 @@ public function mapDataToForms($data, array $forms)
             throw new UnexpectedTypeException($data, 'object, array or empty');
         }
 
-        $iterator = new VirtualFormAwareIterator($forms);
-        $iterator = new \RecursiveIteratorIterator($iterator);
-
-        foreach ($iterator as $form) {
-            /* @var FormInterface $form */
+        foreach ($forms as $form) {
             $propertyPath = $form->getPropertyPath();
             $config = $form->getConfig();
 
@@ -69,7 +64,7 @@ public function mapDataToForms($data, array $forms)
     /**
      * {@inheritdoc}
      */
-    public function mapFormsToData(array $forms, &$data)
+    public function mapFormsToData($forms, &$data)
     {
         if (null === $data) {
             return;
@@ -79,11 +74,7 @@ public function mapFormsToData(array $forms, &$data)
             throw new UnexpectedTypeException($data, 'object, array or empty');
         }
 
-        $iterator = new VirtualFormAwareIterator($forms);
-        $iterator = new \RecursiveIteratorIterator($iterator);
-
-        foreach ($iterator as $form) {
-            /* @var FormInterface $form */
+        foreach ($forms as $form) {
             $propertyPath = $form->getPropertyPath();
             $config = $form->getConfig();
 

src/Symfony/Component/Form/Extension/Core/Type/FormType.php

@@ -48,7 +48,7 @@ public function buildForm(FormBuilderInterface $builder, array $options)
             ->setPropertyPath($options['property_path'])
             ->setMapped($options['mapped'])
             ->setByReference($options['by_reference'])
-            ->setVirtual($options['virtual'])
+            ->setInheritData($options['inherit_data'])
             ->setCompound($options['compound'])
             ->setData(isset($options['data']) ? $options['data'] : null)
             ->setDataLocked(isset($options['data']))
@@ -95,8 +95,8 @@ public function buildView(FormView $view, FormInterface $form, array $options)
             'size'       => null,
             'label_attr' => $options['label_attr'],
             'compound'   => $form->getConfig()->getCompound(),
-            'method'              => $form->getConfig()->getMethod(),
-            'action'              => $form->getConfig()->getAction(),
+            'method'     => $form->getConfig()->getMethod(),
+            'action'     => $form->getConfig()->getAction(),
         ));
     }
 
@@ -150,6 +150,18 @@ public function setDefaultOptions(OptionsResolverInterface $resolver)
             return $options['compound'];
         };
 
+        // BC with old "virtual" option
+        $inheritData = function (Options $options) {
+            if (null !== $options['virtual']) {
+                // Uncomment this as soon as the deprecation note should be shown
+                // trigger_error('The form option "virtual" is deprecated since version 2.3 and will be removed in 3.0. Use "inherit_data" instead.', E_USER_DEPRECATED);
+
+                return $options['virtual'];
+            }
+
+            return false;
+        };
+
         // If data is given, the form is locked to that data
         // (independent of its value)
         $resolver->setOptional(array(
@@ -169,7 +181,8 @@ public function setDefaultOptions(OptionsResolverInterface $resolver)
             'by_reference'       => true,
             'error_bubbling'     => $errorBubbling,
             'label_attr'         => array(),
-            'virtual'            => false,
+            'virtual'            => null,
+            'inherit_data'       => $inheritData,
             'compound'           => true,
             'method'             => 'POST',
             // According to RFC 2396 (http://www.ietf.org/rfc/rfc2396.txt)

src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php

@@ -12,7 +12,7 @@
 namespace Symfony\Component\Form\Extension\Validator\ViolationMapper;
 
 use Symfony\Component\Form\FormInterface;
-use Symfony\Component\Form\Util\VirtualFormAwareIterator;
+use Symfony\Component\Form\Util\InheritDataAwareIterator;
 use Symfony\Component\PropertyAccess\PropertyPathIterator;
 use Symfony\Component\PropertyAccess\PropertyPathBuilder;
 use Symfony\Component\PropertyAccess\PropertyPathIteratorInterface;
@@ -88,8 +88,8 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form
         }
 
         // This case happens if an error happened in the data under a
-        // virtual form that does not match any of the children of
-        // the virtual form.
+        // form inheriting its parent data that does not match any of the
+        // children of that form.
         if (null !== $violationPath && !$match) {
             // If we could not map the error to anything more specific
             // than the root element, map it to the innermost directly
@@ -100,6 +100,9 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form
             $scope = $form;
             $it = new ViolationPathIterator($violationPath);
 
+            // Note: acceptsErrors() will always return true for forms inheriting
+            // their parent data, because these forms can never be non-synchronized
+            // (they don't do any data transformation on their own)
             while ($this->acceptsErrors($scope) && $it->valid() && $it->mapsForm()) {
                 if (!$scope->has($it->current())) {
                     // Break if we find a reference to a non-existing child
@@ -162,9 +165,9 @@ private function matchChild(FormInterface $form, PropertyPathIteratorInterface $
             }
         }
 
-        // Ignore virtual forms when iterating the children
+        // Skip forms inheriting their parent data when iterating the children
         $childIterator = new \RecursiveIteratorIterator(
-            new VirtualFormAwareIterator($form->all())
+            new InheritDataAwareIterator($form->all())
         );
 
         // Make the path longer until we find a matching child
@@ -253,8 +256,8 @@ private function reconstructPath(ViolationPath $violationPath, FormInterface $or
             // Process child form
             $scope = $scope->get($it->current());
 
-            if ($scope->getConfig()->getVirtual()) {
-                // Form is virtual
+            if ($scope->getConfig()->getInheritData()) {
+                // Form inherits its parent data
                 // Cut the piece out of the property path and proceed
                 $propertyPathBuilder->remove($i);
             } elseif (!$scope->getConfig()->getMapped()) {