bug #29695 [Form] Do not ignore the choice groups for caching (vudalt…

…sov)

This PR was merged into the 3.4 branch.

Discussion
----------

[Form] Do not ignore the choice groups for caching

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

While working on a different issue I suddenly came over a strange behaviour.

```php
$builder
    ->add('choice1', ChoiceType::class, [
        'choices' => [
            'a' => 'a',
            'b' => 'b',
            'c' => 'c',
        ],
        'multiple' => true,
    ])
    ->add('choice2', ChoiceType::class, [
        'choices' => [
            'ab' => [
                'a' => 'a',
                'b' => 'b',
            ],
            'c' => 'c',
        ],
        'multiple' => true,
    ]);
```
The code above will result in two identical selects:

![image](https://user-images.githubusercontent.com/2552865/50459865-b3e36980-0980-11e9-8f3d-17f9cfa9a7f8.png)

The reason for this is hash generation in `Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator::createListFromChoices()` — it does not take array structure into account. See [the comment and the code below it](https://github.com/symfony/symfony/blob/7f46dfb1c4ce5e45a5f1918ad6223a3bbdd52a0b/src/Symfony/Component/Form/ChoiceList/Factory/CachingFactoryDecorator.php#L116).

The comment says that the same choice list should be returned for the same collection of choices no matter the structure. This is wrong, because `ChoiceListInterface` has a method `getStructuredValues()` and thus a list instance cannot identified by a hash which does not take structure into account.

I propose a simple change that fixes this and allows for similar choice fields with different groupings.

ping @HeahDude

Commits
-------

9007911 Do not ignore the choice groups for caching

src/Symfony/Component/Form/ChoiceList/Factory/CachingFactoryDecorator.php

@@ -62,30 +62,6 @@ public static function generateHash($value, $namespace = '')
         return hash('sha256', $namespace.':'.serialize($value));
     }
 
-    /**
-     * Flattens an array into the given output variable.
-     *
-     * @param array $array  The array to flatten
-     * @param array $output The flattened output
-     *
-     * @internal
-     */
-    private static function flatten(array $array, &$output)
-    {
-        if (null === $output) {
-            $output = array();
-        }
-
-        foreach ($array as $key => $value) {
-            if (\is_array($value)) {
-                self::flatten($value, $output);
-                continue;
-            }
-
-            $output[$key] = $value;
-        }
-    }
-
     public function __construct(ChoiceListFactoryInterface $decoratedFactory)
     {
         $this->decoratedFactory = $decoratedFactory;
@@ -113,12 +89,7 @@ public function createListFromChoices($choices, $value = null)
         // The value is not validated on purpose. The decorated factory may
         // decide which values to accept and which not.
 
-        // We ignore the choice groups for caching. If two choice lists are
-        // requested with the same choices, but a different grouping, the same
-        // choice list is returned.
-        self::flatten($choices, $flatChoices);
-
-        $hash = self::generateHash(array($flatChoices, $value), 'fromChoices');
+        $hash = self::generateHash(array($choices, $value), 'fromChoices');
 
         if (!isset($this->lists[$hash])) {
             $this->lists[$hash] = $this->decoratedFactory->createListFromChoices($choices, $value);

src/Symfony/Component/Form/Tests/ChoiceList/Factory/CachingFactoryDecoratorTest.php

@@ -64,19 +64,24 @@ public function testCreateFromChoicesComparesTraversableChoicesAsArray()
         $this->assertSame($list, $this->factory->createListFromChoices($choices2));
     }
 
-    public function testCreateFromChoicesFlattensChoices()
+    public function testCreateFromChoicesGroupedChoices()
     {
         $choices1 = array('key' => array('A' => 'a'));
         $choices2 = array('A' => 'a');
-        $list = new \stdClass();
+        $list1 = new \stdClass();
+        $list2 = new \stdClass();
 
-        $this->decoratedFactory->expects($this->once())
+        $this->decoratedFactory->expects($this->at(0))
             ->method('createListFromChoices')
             ->with($choices1)
-            ->will($this->returnValue($list));
+            ->will($this->returnValue($list1));
+        $this->decoratedFactory->expects($this->at(1))
+            ->method('createListFromChoices')
+            ->with($choices2)
+            ->will($this->returnValue($list2));
 
-        $this->assertSame($list, $this->factory->createListFromChoices($choices1));
-        $this->assertSame($list, $this->factory->createListFromChoices($choices2));
+        $this->assertSame($list1, $this->factory->createListFromChoices($choices1));
+        $this->assertSame($list2, $this->factory->createListFromChoices($choices2));
     }
 
     /**