[Form] Fixed new ArrayChoiceList to compare choices by their values, if enabled

src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php

@@ -11,7 +11,7 @@
 
 namespace Symfony\Component\Form\ChoiceList;
 
-use Symfony\Component\Form\Exception\InvalidArgumentException;
+use Symfony\Component\Form\Exception\UnexpectedTypeException;
 
 /**
  * A list of choices with arbitrary data types.
@@ -39,33 +39,46 @@ class ArrayChoiceList implements ChoiceListInterface
      */
     protected $values = array();
 
+    /**
+     * The callback for creating the value for a choice.
+     *
+     * @var callable
+     */
+    protected $valueCallback;
+
     /**
      * Creates a list with the given choices and values.
      *
      * The given choice array must have the same array keys as the value array.
      *
-     * @param array    $choices The selectable choices
+     * @param array    $choices        The selectable choices
-     * @param string[] $values  The string values of the choices
+     * @param callable $value          The callable for creating the value for a
-     *
+     *                                 choice. If `null` is passed, incrementing
-     * @throws InvalidArgumentException If the keys of the choices don't match
+     *                                 integers are used as values
-     *                                  the keys of the values
+     * @param bool     $compareByValue Whether to use the value callback to
+     *                                 compare choices. If `null`, choices are
+     *                                 compared by identity
      */
-    public function __construct(array $choices, array $values)
+    public function __construct(array $choices, $value = null, $compareByValue = false)
     {
-        $choiceKeys = array_keys($choices);
+        if (null !== $value && !is_callable($value)) {
-        $valueKeys = array_keys($values);
+            throw new UnexpectedTypeException($value, 'null or callable');
-
-        if ($choiceKeys !== $valueKeys) {
-            throw new InvalidArgumentException(sprintf(
-                'The keys of the choices and the values must match. The choice '.
-                'keys are: "%s". The value keys are: "%s".',
-                implode('", "', $choiceKeys),
-                implode('", "', $valueKeys)
-            ));
         }
 
         $this->choices = $choices;
-        $this->values = array_map('strval', $values);
+        $this->values = array();
+        $this->valueCallback = $compareByValue ? $value : null;
+
+        if (null === $value) {
+            $i = 0;
+            foreach ($this->choices as $key => $choice) {
+                $this->values[$key] = (string) $i++;
+            }
+        } else {
+            foreach ($choices as $key => $choice) {
+                $this->values[$key] = (string) call_user_func($value, $choice, $key);
+            }
+        }
     }
 
     /**
@@ -116,6 +129,17 @@ class ArrayChoiceList implements ChoiceListInterface
     {
         $values = array();
 
+        // Use the value callback to compare choices by their values, if present
+        if ($this->valueCallback) {
+            $givenValues = array();
+            foreach ($choices as $key => $choice) {
+                $givenValues[$key] = (string) call_user_func($this->valueCallback, $choice, $key);
+            }
+
+            return array_intersect($givenValues, $this->values);
+        }
+
+        // Otherwise compare choices by identity
         foreach ($choices as $i => $givenChoice) {
             foreach ($this->choices as $j => $choice) {
                 if ($choice !== $givenChoice) {

src/Symfony/Component/Form/ChoiceList/ArrayKeyChoiceList.php

@@ -43,21 +43,14 @@ use Symfony\Component\Form\Exception\InvalidArgumentException;
  * @deprecated Added for backwards compatibility in Symfony 2.7, to be removed
  *             in Symfony 3.0.
  */
-class ArrayKeyChoiceList implements ChoiceListInterface
+class ArrayKeyChoiceList extends ArrayChoiceList
 {
     /**
-     * The selectable choices.
+     * Whether the choices are used as values.
      *
-     * @var array
+     * @var bool
      */
-    private $choices = array();
+    private $useChoicesAsValues = false;
-
-    /**
-     * The values of the choices.
-     *
-     * @var string[]
-     */
-    private $values = array();
 
     /**
      * Casts the given choice to an array key.
@@ -100,51 +93,26 @@ class ArrayKeyChoiceList implements ChoiceListInterface
      * values.
      *
      * @param array    $choices The selectable choices
-     * @param string[] $values  Optional. The string values of the choices
+     * @param callable $value   The callable for creating the value for a
+     *                          choice. If `null` is passed, the choices are
+     *                          cast to strings and used as values
      *
      * @throws InvalidArgumentException If the keys of the choices don't match
      *                                  the keys of the values or if any of the
      *                                  choices is not scalar
      */
-    public function __construct(array $choices, array $values = array())
+    public function __construct(array $choices, $value = null)
     {
-        if (empty($values)) {
+        $choices = array_map(array(__CLASS__, 'toArrayKey'), $choices);
-            // The cast to strings happens later
-            $values = $choices;
-        } else {
-            $choiceKeys = array_keys($choices);
-            $valueKeys = array_keys($values);
 
-            if ($choiceKeys !== $valueKeys) {
+        if (null === $value) {
-                throw new InvalidArgumentException(
+            $value = function ($choice) {
-                    sprintf(
+                return (string) $choice;
-                        'The keys of the choices and the values must match. The choice '.
+            };
-                        'keys are: "%s". The value keys are: "%s".',
+            $this->useChoicesAsValues = true;
-                        implode('", "', $choiceKeys),
-                        implode('", "', $valueKeys)
-                    )
-                );
-            }
         }
 
-        $this->choices = array_map(array(__CLASS__, 'toArrayKey'), $choices);
+        parent::__construct($choices, $value);
-        $this->values = array_map('strval', $values);
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function getChoices()
-    {
-        return $this->choices;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function getValues()
-    {
-        return $this->values;
     }
 
     /**
@@ -152,11 +120,15 @@ class ArrayKeyChoiceList implements ChoiceListInterface
      */
     public function getChoicesForValues(array $values)
     {
-        $values = array_map('strval', $values);
+        if ($this->useChoicesAsValues) {
+            $values = array_map('strval', $values);
 
-        // The values are identical to the choices, so we can just return them
+            // If the values are identical to the choices, so we can just return
-        // to improve performance a little bit
+            // them to improve performance a little bit
-        return array_map(array(__CLASS__, 'toArrayKey'), array_intersect($values, $this->values));
+            return array_map(array(__CLASS__, 'toArrayKey'), array_intersect($values, $this->values));
+        }
+
+        return parent::getChoicesForValues($values);
     }
 
     /**
@@ -166,8 +138,12 @@ class ArrayKeyChoiceList implements ChoiceListInterface
     {
         $choices = array_map(array(__CLASS__, 'toArrayKey'), $choices);
 
-        // The choices are identical to the values, so we can just return them
+        if ($this->useChoicesAsValues) {
-        // to improve performance a little bit
+            // If the choices are identical to the values, we can just return
-        return array_map('strval', array_intersect($choices, $this->choices));
+            // them to improve performance a little bit
+            return array_map('strval', array_intersect($choices, $this->choices));
+        }
+
+        return parent::getValuesForChoices($choices);
     }
 }

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

@@ -89,10 +89,6 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface
             throw new UnexpectedTypeException($choices, 'array or \Traversable');
         }
 
-        if (null !== $value && !is_callable($value)) {
-            throw new UnexpectedTypeException($value, 'null or callable');
-        }
-
         if ($choices instanceof \Traversable) {
             $choices = iterator_to_array($choices);
         }
@@ -102,29 +98,7 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface
         // in the view only.
         self::flatten($choices, $flatChoices);
 
-        // If no values are given, use incrementing integers as values
+        return new ArrayChoiceList($flatChoices, $value);
-        // We can not use the choices themselves, because we don't know whether
-        // choices can be converted to (duplicate-free) strings
-        if (null === $value) {
-            $values = $flatChoices;
-            $i = 0;
-
-            foreach ($values as $key => $value) {
-                $values[$key] = (string) $i++;
-            }
-
-            return new ArrayChoiceList($flatChoices, $values);
-        }
-
-        // Can't use array_map(), because array_map() doesn't pass the key
-        // Can't use array_walk(), which ignores the return value of the
-        // closure
-        $values = array();
-        foreach ($flatChoices as $key => $choice) {
-            $values[$key] = call_user_func($value, $choice, $key);
-        }
-
-        return new ArrayChoiceList($flatChoices, $values);
     }
 
     /**
@@ -139,10 +113,6 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface
             throw new UnexpectedTypeException($choices, 'array or \Traversable');
         }
 
-        if (null !== $value && !is_callable($value)) {
-            throw new UnexpectedTypeException($value, 'null or callable');
-        }
-
         if ($choices instanceof \Traversable) {
             $choices = iterator_to_array($choices);
         }
@@ -157,20 +127,12 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface
         // strings or integers, we are guaranteed to be able to convert them
         // to strings
         if (null === $value) {
-            $values = array_map('strval', $flatChoices);
+            $value = function ($choice) {
-
+                return (string) $choice;
-            return new ArrayKeyChoiceList($flatChoices, $values);
+            };
         }
 
-        // Can't use array_map(), because array_map() doesn't pass the key
+        return new ArrayKeyChoiceList($flatChoices, $value);
-        // Can't use array_walk(), which ignores the return value of the
-        // closure
-        $values = array();
-        foreach ($flatChoices as $key => $choice) {
-            $values[$key] = call_user_func($value, $choice, $key);
-        }
-
-        return new ArrayKeyChoiceList($flatChoices, $values);
     }
 
     /**

src/Symfony/Component/Form/Tests/ChoiceList/ArrayChoiceListTest.php

@@ -29,7 +29,9 @@ class ArrayChoiceListTest extends AbstractChoiceListTest
 
     protected function createChoiceList()
     {
-        return new ArrayChoiceList($this->getChoices(), $this->getValues());
+        $i = 0;
+
+        return new ArrayChoiceList($this->getChoices());
     }
 
     protected function getChoices()
@@ -49,4 +51,45 @@ class ArrayChoiceListTest extends AbstractChoiceListTest
     {
         new ArrayChoiceList(array(0 => 'a', 1 => 'b'), array(1 => 'a', 2 => 'b'));
     }
+
+    public function testCreateChoiceListWithValueCallback()
+    {
+        $callback = function ($choice, $key) {
+            return $key.':'.$choice;
+        };
+
+        $choiceList = new ArrayChoiceList(array(2 => 'foo', 7 => 'bar', 10 => 'baz'), $callback);
+
+        $this->assertSame(array(2 => '2:foo', 7 => '7:bar', 10 => '10:baz'), $choiceList->getValues());
+        $this->assertSame(array(1 => 'foo', 2 => 'baz'), $choiceList->getChoicesForValues(array(1 => '2:foo', 2 => '10:baz')));
+        $this->assertSame(array(1 => '2:foo', 2 => '10:baz'), $choiceList->getValuesForChoices(array(1 => 'foo', 2 => 'baz')));
+    }
+
+    public function testCompareChoicesByIdentityByDefault()
+    {
+        $callback = function ($choice) {
+            return $choice->value;
+        };
+
+        $obj1 = (object) array('value' => 'value1');
+        $obj2 = (object) array('value' => 'value2');
+
+        $choiceList = new ArrayChoiceList(array($obj1, $obj2), $callback);
+        $this->assertSame(array(2 => 'value2'), $choiceList->getValuesForChoices(array(2 => $obj2)));
+        $this->assertSame(array(), $choiceList->getValuesForChoices(array(2 => (object) array('value' => 'value2'))));
+    }
+
+    public function testCompareChoicesWithValueCallbackIfCompareByValue()
+    {
+        $callback = function ($choice) {
+            return $choice->value;
+        };
+
+        $obj1 = (object) array('value' => 'value1');
+        $obj2 = (object) array('value' => 'value2');
+
+        $choiceList = new ArrayChoiceList(array($obj1, $obj2), $callback, true);
+        $this->assertSame(array(2 => 'value2'), $choiceList->getValuesForChoices(array(2 => $obj2)));
+        $this->assertSame(array(2 => 'value2'), $choiceList->getValuesForChoices(array(2 => (object) array('value' => 'value2'))));
+    }
 }

src/Symfony/Component/Form/Tests/ChoiceList/ArrayKeyChoiceListTest.php

@@ -29,7 +29,7 @@ class ArrayKeyChoiceListTest extends AbstractChoiceListTest
 
     protected function createChoiceList()
     {
-        return new ArrayKeyChoiceList($this->getChoices(), $this->getValues());
+        return new ArrayKeyChoiceList($this->getChoices());
     }
 
     protected function getChoices()
@@ -42,14 +42,6 @@ class ArrayKeyChoiceListTest extends AbstractChoiceListTest
         return array('0', '1', 'a', 'b', '');
     }
 
-    /**
-     * @expectedException \Symfony\Component\Form\Exception\InvalidArgumentException
-     */
-    public function testFailIfKeyMismatch()
-    {
-        new ArrayKeyChoiceList(array(0 => 'a', 1 => 'b'), array(1 => 'a', 2 => 'b'));
-    }
-
     public function testUseChoicesAsValuesByDefault()
     {
         $list = new ArrayKeyChoiceList(array(1 => '', 3 => 0, 7 => '1', 10 => 1.23));
@@ -102,7 +94,7 @@ class ArrayKeyChoiceListTest extends AbstractChoiceListTest
      */
     public function testConvertChoicesIfNecessary(array $choices, array $converted)
     {
-        $list = new ArrayKeyChoiceList($choices, range(0, count($choices) - 1));
+        $list = new ArrayKeyChoiceList($choices);
 
         $this->assertSame($converted, $list->getChoices());
     }
@@ -134,7 +126,7 @@ class ArrayKeyChoiceListTest extends AbstractChoiceListTest
      */
     public function testFailIfInvalidChoices(array $choices)
     {
-        new ArrayKeyChoiceList($choices, range(0, count($choices) - 1));
+        new ArrayKeyChoiceList($choices);
     }
 
     /**
@@ -157,31 +149,48 @@ class ArrayKeyChoiceListTest extends AbstractChoiceListTest
     /**
      * @dataProvider provideConvertibleValues
      */
-    public function testConvertValuesToStrings(array $values, array $converted)
+    public function testConvertValuesToStrings($value, $converted)
     {
-        $list = new ArrayKeyChoiceList(range(0, count($values) - 1), $values);
+        $callback = function () use ($value) {
+            return $value;
+        };
 
-        $this->assertSame($converted, $list->getValues());
+        $list = new ArrayKeyChoiceList(array('choice'), $callback);
+
+        $this->assertSame(array($converted), $list->getValues());
     }
 
     public function provideConvertibleValues()
     {
         return array(
-            array(array(0), array('0')),
+            array(0, '0'),
-            array(array(1), array('1')),
+            array(1, '1'),
-            array(array('0'), array('0')),
+            array('0', '0'),
-            array(array('1'), array('1')),
+            array('1', '1'),
-            array(array('1.23'), array('1.23')),
+            array('1.23', '1.23'),
-            array(array('foobar'), array('foobar')),
+            array('foobar', 'foobar'),
             // The default value of choice fields is NULL. It should be treated
             // like the empty value for this choice list type
-            array(array(null), array('')),
+            array(null, ''),
-            array(array(1.23), array('1.23')),
+            array(1.23, '1.23'),
             // Always cast booleans to 0 and 1, because:
             // array(true => 'Yes', false => 'No') === array(1 => 'Yes', 0 => 'No')
             // see ChoiceTypeTest::testSetDataSingleNonExpandedAcceptsBoolean
-            array(array(true), array('1')),
+            array(true, '1'),
-            array(array(false), array('')),
+            array(false, ''),
         );
     }
+
+    public function testCreateChoiceListWithValueCallback()
+    {
+        $callback = function ($choice, $key) {
+            return $key.':'.$choice;
+        };
+
+        $choiceList = new ArrayKeyChoiceList(array(2 => 'foo', 7 => 'bar', 10 => 'baz'), $callback);
+
+        $this->assertSame(array(2 => '2:foo', 7 => '7:bar', 10 => '10:baz'), $choiceList->getValues());
+        $this->assertSame(array(1 => 'foo', 2 => 'baz'), $choiceList->getChoicesForValues(array(1 => '2:foo', 2 => '10:baz')));
+        $this->assertSame(array(1 => '2:foo', 2 => '10:baz'), $choiceList->getValuesForChoices(array(1 => 'foo', 2 => 'baz')));
+    }
 }

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

@@ -83,8 +83,7 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase
         $this->obj3 = (object) array('label' => 'C', 'index' => 'y', 'value' => 1, 'preferred' => true, 'group' => 'Group 2', 'attr' => array('attr2' => 'value2'));
         $this->obj4 = (object) array('label' => 'D', 'index' => 'z', 'value' => 2, 'preferred' => false, 'group' => 'Group 2', 'attr' => array());
         $this->list = new ArrayChoiceList(
-            array('A' => $this->obj1, 'B' => $this->obj2, 'C' => $this->obj3, 'D' => $this->obj4),
+            array('A' => $this->obj1, 'B' => $this->obj2, 'C' => $this->obj3, 'D' => $this->obj4)
-            array('A' => '0', 'B' => '1', 'C' => '2', 'D' => '3')
         );
         $this->factory = new DefaultChoiceListFactory();
     }