[Form] Fixed regression: Choices are compared by their values if a value callback is given

src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php

@@ -11,12 +11,10 @@
 
 namespace Symfony\Bridge\Doctrine\Form\ChoiceList;
 
-use Doctrine\Common\Persistence\Mapping\ClassMetadata;
 use Doctrine\Common\Persistence\ObjectManager;
 use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
 use Symfony\Component\Form\ChoiceList\Factory\ChoiceListFactoryInterface;
 use Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface;
-use Symfony\Component\Form\Exception\RuntimeException;
 
 /**
  * Loads choices using a Doctrine object manager.
@@ -41,67 +39,20 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
     private $class;
 
     /**
-     * @var ClassMetadata
+     * @var IdReader
      */
-    private $classMetadata;
+    private $idReader;
 
     /**
      * @var null|EntityLoaderInterface
      */
     private $objectLoader;
 
-    /**
-     * The identifier field, unless the identifier is composite
-     *
-     * @var null|string
-     */
-    private $idField = null;
-
-    /**
-     * Whether to use the identifier for value generation
-     *
-     * @var bool
-     */
-    private $compositeId = true;
-
     /**
      * @var ChoiceListInterface
      */
     private $choiceList;
 
-    /**
-     * Returns the value of the identifier field of an object.
-     *
-     * Doctrine must know about this object, that is, the object must already
-     * be persisted or added to the identity map before. Otherwise an
-     * exception is thrown.
-     *
-     * This method assumes that the object has a single-column identifier and
-     * will return a single value instead of an array.
-     *
-     * @param object $object The object for which to get the identifier
-     *
-     * @return int|string The identifier value
-     *
-     * @throws RuntimeException If the object does not exist in Doctrine's identity map
-     *
-     * @internal Should not be accessed by user-land code. This method is public
-     *           only to be usable as callback.
-     */
-    public static function getIdValue(ObjectManager $om, ClassMetadata $classMetadata, $object)
-    {
-        if (!$om->contains($object)) {
-            throw new RuntimeException(
-                'Entities passed to the choice field must be managed. Maybe '.
-                'persist them in the entity manager?'
-            );
-        }
-
-        $om->initializeObject($object);
-
-        return current($classMetadata->getIdentifierValues($object));
-    }
-
     /**
      * Creates a new choice loader.
      *
@@ -114,22 +65,17 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
      * @param ObjectManager              $manager      The object manager
      * @param string                     $class        The class name of the
      *                                                 loaded objects
+     * @param IdReader                   $idReader     The reader for the object
+     *                                                 IDs.
      * @param null|EntityLoaderInterface $objectLoader The objects loader
      */
-    public function __construct(ChoiceListFactoryInterface $factory, ObjectManager $manager, $class, EntityLoaderInterface $objectLoader = null)
+    public function __construct(ChoiceListFactoryInterface $factory, ObjectManager $manager, $class, IdReader $idReader, EntityLoaderInterface $objectLoader = null)
     {
         $this->factory = $factory;
         $this->manager = $manager;
-        $this->classMetadata = $manager->getClassMetadata($class);
+        $this->class = $manager->getClassMetadata($class)->getName();
-        $this->class = $this->classMetadata->getName();
+        $this->idReader = $idReader;
         $this->objectLoader = $objectLoader;
-
-        $identifier = $this->classMetadata->getIdentifierFieldNames();
-
-        if (1 === count($identifier)) {
-            $this->idField = $identifier[0];
-            $this->compositeId = false;
-        }
     }
 
     /**
@@ -145,23 +91,7 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
             ? $this->objectLoader->getEntities()
             : $this->manager->getRepository($this->class)->findAll();
 
-        // If the class has a multi-column identifier, we cannot index the
+        $this->choiceList = $this->factory->createListFromChoices($objects, $value);
-        // objects by their IDs
-        if ($this->compositeId) {
-            $this->choiceList = $this->factory->createListFromChoices($objects, $value);
-
-            return $this->choiceList;
-        }
-
-        // Index the objects by ID
-        $objectsById = array();
-
-        foreach ($objects as $object) {
-            $id = self::getIdValue($this->manager, $this->classMetadata, $object);
-            $objectsById[$id] = $object;
-        }
-
-        $this->choiceList = $this->factory->createListFromChoices($objectsById, $value);
 
         return $this->choiceList;
     }
@@ -193,14 +123,14 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
         // know that the IDs are used as values
 
         // Attention: This optimization does not check choices for existence
-        if (!$this->choiceList && !$this->compositeId) {
+        if (!$this->choiceList && $this->idReader->isSingleId()) {
             $values = array();
 
             // Maintain order and indices of the given objects
             foreach ($objects as $i => $object) {
                 if ($object instanceof $this->class) {
                     // Make sure to convert to the right format
-                    $values[$i] = (string) self::getIdValue($this->manager, $this->classMetadata, $object);
+                    $values[$i] = (string) $this->idReader->getIdValue($object);
                 }
             }
 
@@ -240,8 +170,8 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
 
         // Optimize performance in case we have an object loader and
         // a single-field identifier
-        if (!$this->choiceList && !$this->compositeId && $this->objectLoader) {
+        if (!$this->choiceList && $this->objectLoader && $this->idReader->isSingleId()) {
-            $unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idField, $values);
+            $unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values);
             $objectsById = array();
             $objects = array();
 
@@ -250,8 +180,7 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
             // "INDEX BY" clause to the Doctrine query in the loader,
             // but I'm not sure whether that's doable in a generic fashion.
             foreach ($unorderedObjects as $object) {
-                $id = self::getIdValue($this->manager, $this->classMetadata, $object);
+                $objectsById[$this->idReader->getIdValue($object)] = $object;
-                $objectsById[$id] = $object;
             }
 
             foreach ($values as $i => $id) {

src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php

@@ -0,0 +1,125 @@
+<?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\Bridge\Doctrine\Form\ChoiceList;
+
+use Doctrine\Common\Persistence\Mapping\ClassMetadata;
+use Doctrine\Common\Persistence\ObjectManager;
+use Symfony\Component\Form\Exception\RuntimeException;
+
+/**
+ * A utility for reading object IDs.
+ *
+ * @since  1.0
+ * @author Bernhard Schussek <bschussek@gmail.com>
+ *
+ * @internal This class is meant for internal use only.
+ */
+class IdReader
+{
+    /**
+     * @var ObjectManager
+     */
+    private $om;
+
+    /**
+     * @var ClassMetadata
+     */
+    private $classMetadata;
+
+    /**
+     * @var bool
+     */
+    private $singleId;
+
+    /**
+     * @var bool
+     */
+    private $intId;
+
+    /**
+     * @var string
+     */
+    private $idField;
+
+    public function __construct(ObjectManager $om, ClassMetadata $classMetadata)
+    {
+        $ids = $classMetadata->getIdentifierFieldNames();
+        $idType = $classMetadata->getTypeOfField(current($ids));
+
+        $this->om = $om;
+        $this->classMetadata = $classMetadata;
+        $this->singleId = 1 === count($ids);
+        $this->intId = $this->singleId && 1 === count($ids) && in_array($idType, array('integer', 'smallint', 'bigint'));
+        $this->idField = current($ids);
+    }
+
+    /**
+     * Returns whether the class has a single-column ID.
+     *
+     * @return bool Returns `true` if the class has a single-column ID and
+     *              `false` otherwise.
+     */
+    public function isSingleId()
+    {
+        return $this->singleId;
+    }
+
+    /**
+     * Returns whether the class has a single-column integer ID.
+     *
+     * @return bool Returns `true` if the class has a single-column integer ID
+     *              and `false` otherwise.
+     */
+    public function isIntId()
+    {
+        return $this->intId;
+    }
+
+    /**
+     * Returns the ID value for an object.
+     *
+     * This method assumes that the object has a single-column ID.
+     *
+     * @param object $object The object.
+     *
+     * @return mixed The ID value.
+     */
+    public function getIdValue($object)
+    {
+        if (!$object) {
+            return null;
+        }
+
+        if (!$this->om->contains($object)) {
+            throw new RuntimeException(
+                'Entities passed to the choice field must be managed. Maybe '.
+                'persist them in the entity manager?'
+            );
+        }
+
+        $this->om->initializeObject($object);
+
+        return current($this->classMetadata->getIdentifierValues($object));
+    }
+
+    /**
+     * Returns the name of the ID field.
+     *
+     * This method assumes that the object has a single-column ID.
+     *
+     * @return string The name of the ID field.
+     */
+    public function getIdField()
+    {
+        return $this->idField;
+    }
+}

src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php

@@ -16,6 +16,7 @@ use Doctrine\Common\Persistence\ObjectManager;
 use Doctrine\ORM\QueryBuilder;
 use Symfony\Bridge\Doctrine\Form\ChoiceList\DoctrineChoiceLoader;
 use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface;
+use Symfony\Bridge\Doctrine\Form\ChoiceList\IdReader;
 use Symfony\Bridge\Doctrine\Form\DataTransformer\CollectionToArrayTransformer;
 use Symfony\Bridge\Doctrine\Form\EventListener\MergeDoctrineCollectionListener;
 use Symfony\Component\Form\AbstractType;
@@ -42,11 +43,55 @@ abstract class DoctrineType extends AbstractType
      */
     private $choiceListFactory;
 
+    /**
+     * @var IdReader[]
+     */
+    private $idReaders = array();
+
     /**
      * @var DoctrineChoiceLoader[]
      */
     private $choiceLoaders = array();
 
+    /**
+     * Creates the label for a choice.
+     *
+     * For backwards compatibility, objects are cast to strings by default.
+     *
+     * @param object $choice The object.
+     *
+     * @return string The string representation of the object.
+     *
+     * @internal This method is public to be usable as callback. It should not
+     *           be used in user code.
+     */
+    public static function createChoiceLabel($choice)
+    {
+        return (string) $choice;
+    }
+
+    /**
+     * Creates the field name for a choice.
+     *
+     * This method is used to generate field names if the underlying object has
+     * a single-column integer ID. In that case, the value of the field is
+     * the ID of the object. That ID is also used as field name.
+     *
+     * @param object     $choice The object.
+     * @param int|string $key    The choice key.
+     * @param string     $value  The choice value. Corresponds to the object's
+     *                           ID here.
+     *
+     * @return string The field name.
+     *
+     * @internal This method is public to be usable as callback. It should not
+     *           be used in user code.
+     */
+    public static function createChoiceName($choice, $key, $value)
+    {
+        return (string) $value;
+    }
+
     public function __construct(ManagerRegistry $registry, PropertyAccessorInterface $propertyAccessor = null, ChoiceListFactoryInterface $choiceListFactory = null)
     {
         $this->registry = $registry;
@@ -67,9 +112,30 @@ abstract class DoctrineType extends AbstractType
     {
         $registry = $this->registry;
         $choiceListFactory = $this->choiceListFactory;
+        $idReaders = &$this->idReaders;
         $choiceLoaders = &$this->choiceLoaders;
         $type = $this;
 
+        $idReader = function (Options $options) use (&$idReaders) {
+            $hash = CachingFactoryDecorator::generateHash(array(
+                $options['em'],
+                $options['class'],
+            ));
+
+            // The ID reader is a utility that is needed to read the object IDs
+            // when generating the field values. The callback generating the
+            // field values has no access to the object manager or the class
+            // of the field, so we store that information in the reader.
+            // The reader is cached so that two choice lists for the same class
+            // (and hence with the same reader) can successfully be cached.
+            if (!isset($idReaders[$hash])) {
+                $classMetadata = $options['em']->getClassMetadata($options['class']);
+                $idReaders[$hash] = new IdReader($options['em'], $classMetadata);
+            }
+
+            return $idReaders[$hash];
+        };
+
         $choiceLoader = function (Options $options) use ($choiceListFactory, &$choiceLoaders, $type) {
             // Unless the choices are given explicitly, load them on demand
             if (null === $options['choices']) {
@@ -106,6 +172,7 @@ abstract class DoctrineType extends AbstractType
                     $choiceListFactory,
                     $options['em'],
                     $options['class'],
+                    $options['id_reader'],
                     $entityLoader
                 );
 
@@ -120,24 +187,18 @@ abstract class DoctrineType extends AbstractType
             }
 
             // BC: use __toString() by default
-            return function ($entity) {
+            return array(__CLASS__, 'createChoiceLabel');
-                return (string) $entity;
-            };
         };
 
         $choiceName = function (Options $options) {
-            /** @var ObjectManager $om */
+            /** @var IdReader $idReader */
-            $om = $options['em'];
+            $idReader = $options['id_reader'];
-            $classMetadata = $om->getClassMetadata($options['class']);
-            $ids = $classMetadata->getIdentifierFieldNames();
-            $idType = $classMetadata->getTypeOfField(current($ids));
 
-            // If the entity has a single-column, numeric ID, use that ID as
+            // If the object has a single-column, numeric ID, use that ID as
-            // field name
+            // field name. We can only use numeric IDs as names, as we cannot
-            if (1 === count($ids) && in_array($idType, array('integer', 'smallint', 'bigint'))) {
+            // guarantee that a non-numeric ID contains a valid form name
-                return function ($entity, $id) {
+            if ($idReader->isIntId()) {
-                    return $id;
+                return array(__CLASS__, 'createChoiceName');
-                };
             }
 
             // Otherwise, an incrementing integer is used as name automatically
@@ -147,8 +208,16 @@ abstract class DoctrineType extends AbstractType
         // and DoctrineChoiceLoader), unless the ID is composite. Then they
         // are indexed by an incrementing integer.
         // Use the ID/incrementing integer as choice value.
-        $choiceValue = function ($entity, $key) {
+        $choiceValue = function (Options $options) {
-            return $key;
+            /** @var IdReader $idReader */
+            $idReader = $options['id_reader'];
+
+            // If the entity has a single-column ID, use that ID as value
+            if ($idReader->isSingleId()) {
+                return array($idReader, 'getIdValue');
+            }
+
+            // Otherwise, an incrementing integer is used as value automatically
         };
 
         $emNormalizer = function (Options $options, $em) use ($registry) {
@@ -174,33 +243,6 @@ abstract class DoctrineType extends AbstractType
             return $em;
         };
 
-        $choicesNormalizer = function (Options $options, $entities) {
-            if (null === $entities || 0 === count($entities)) {
-                return $entities;
-            }
-
-            // Make sure that the entities are indexed by their ID
-            /** @var ObjectManager $om */
-            $om = $options['em'];
-            $classMetadata = $om->getClassMetadata($options['class']);
-            $ids = $classMetadata->getIdentifierFieldNames();
-
-            // We cannot use composite IDs as indices. In that case, keep the
-            // given indices
-            if (count($ids) > 1) {
-                return $entities;
-            }
-
-            $entitiesById = array();
-
-            foreach ($entities as $entity) {
-                $id = DoctrineChoiceLoader::getIdValue($om, $classMetadata, $entity);
-                $entitiesById[$id] = $entity;
-            }
-
-            return $entitiesById;
-        };
-
         // Invoke the query builder closure so that we can cache choice lists
         // for equal query builders
         $queryBuilderNormalizer = function (Options $options, $queryBuilder) {
@@ -226,12 +268,12 @@ abstract class DoctrineType extends AbstractType
             'choice_label' => $choiceLabel,
             'choice_name' => $choiceName,
             'choice_value' => $choiceValue,
+            'id_reader' => $idReader,
         ));
 
         $resolver->setRequired(array('class'));
 
         $resolver->setNormalizer('em', $emNormalizer);
-        $resolver->setNormalizer('choices', $choicesNormalizer);
         $resolver->setNormalizer('query_builder', $queryBuilderNormalizer);
 
         $resolver->setAllowedTypes('em', array('null', 'string', 'Doctrine\Common\Persistence\ObjectManager'));

src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php

@@ -29,7 +29,6 @@ use Symfony\Component\Form\ChoiceList\View\ChoiceGroupView;
 use Symfony\Component\Form\ChoiceList\View\ChoiceView;
 use Symfony\Component\Form\Forms;
 use Symfony\Component\Form\Test\TypeTestCase;
-use Symfony\Component\OptionsResolver\Options;
 use Symfony\Component\PropertyAccess\PropertyAccess;
 
 class EntityTypeTest extends TypeTestCase

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

@@ -51,15 +51,12 @@ class ArrayChoiceList implements ChoiceListInterface
      *
      * 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 callable $value          The callable for creating the value for a
+     * @param callable $value   The callable for creating the value for a
-     *                                 choice. If `null` is passed, incrementing
+     *                          choice. If `null` is passed, incrementing
-     *                                 integers are used as values
+     *                          integers are used as 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, $value = null, $compareByValue = false)
+    public function __construct(array $choices, $value = null)
     {
         if (null !== $value && !is_callable($value)) {
             throw new UnexpectedTypeException($value, 'null or callable');
@@ -67,7 +64,7 @@ class ArrayChoiceList implements ChoiceListInterface
 
         $this->choices = $choices;
         $this->values = array();
-        $this->valueCallback = $compareByValue ? $value : null;
+        $this->valueCallback = $value;
 
         if (null === $value) {
             $i = 0;
@@ -76,7 +73,7 @@ class ArrayChoiceList implements ChoiceListInterface
             }
         } else {
             foreach ($choices as $key => $choice) {
-                $this->values[$key] = (string) call_user_func($value, $choice, $key);
+                $this->values[$key] = (string) call_user_func($value, $choice);
             }
         }
     }
@@ -132,8 +129,9 @@ class ArrayChoiceList implements ChoiceListInterface
         // 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);
+            foreach ($choices as $i => $givenChoice) {
+                $givenValues[$i] = (string) call_user_func($this->valueCallback, $givenChoice);
             }
 
             return array_intersect($givenValues, $this->values);

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

@@ -191,10 +191,7 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface
 
         // The names are generated from an incrementing integer by default
         if (null === $index) {
-            $i = 0;
+            $index = 0;
-            $index = function () use (&$i) {
-                return $i++;
-            };
         }
 
         // If $groupBy is not given, no grouping is done
@@ -267,27 +264,30 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface
         return new ChoiceListView($otherViews, $preferredViews);
     }
 
-    private static function addChoiceView($choice, $key, $label, $values, $index, $attr, $isPreferred, &$preferredViews, &$otherViews)
+    private static function addChoiceView($choice, $key, $label, $values, &$index, $attr, $isPreferred, &$preferredViews, &$otherViews)
     {
+        $value = $values[$key];
+        $nextIndex = is_int($index) ? $index++ : call_user_func($index, $choice, $key, $value);
+
         $view = new ChoiceView(
             // If the labels are null, use the choice key by default
-            null === $label ? (string) $key : (string) call_user_func($label, $choice, $key),
+            null === $label ? (string) $key : (string) call_user_func($label, $choice, $key, $value),
-            $values[$key],
+            $value,
             $choice,
             // The attributes may be a callable or a mapping from choice indices
             // to nested arrays
-            is_callable($attr) ? call_user_func($attr, $choice, $key) : (isset($attr[$key]) ? $attr[$key] : array())
+            is_callable($attr) ? call_user_func($attr, $choice, $key, $value) : (isset($attr[$key]) ? $attr[$key] : array())
         );
 
         // $isPreferred may be null if no choices are preferred
-        if ($isPreferred && call_user_func($isPreferred, $choice, $key)) {
+        if ($isPreferred && call_user_func($isPreferred, $choice, $key, $value)) {
-            $preferredViews[call_user_func($index, $choice, $key)] = $view;
+            $preferredViews[$nextIndex] = $view;
         } else {
-            $otherViews[call_user_func($index, $choice, $key)] = $view;
+            $otherViews[$nextIndex] = $view;
         }
     }
 
-    private static function addChoiceViewsGroupedBy($groupBy, $label, $choices, $values, $index, $attr, $isPreferred, &$preferredViews, &$otherViews)
+    private static function addChoiceViewsGroupedBy($groupBy, $label, $choices, $values, &$index, $attr, $isPreferred, &$preferredViews, &$otherViews)
     {
         foreach ($groupBy as $key => $content) {
             // Add the contents of groups to new ChoiceGroupView instances
@@ -333,9 +333,9 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface
         }
     }
 
-    private static function addChoiceViewGroupedBy($groupBy, $choice, $key, $label, $values, $index, $attr, $isPreferred, &$preferredViews, &$otherViews)
+    private static function addChoiceViewGroupedBy($groupBy, $choice, $key, $label, $values, &$index, $attr, $isPreferred, &$preferredViews, &$otherViews)
     {
-        $groupLabel = call_user_func($groupBy, $choice, $key);
+        $groupLabel = call_user_func($groupBy, $choice, $key, $values[$key]);
 
         if (null === $groupLabel) {
             // If the callable returns null, don't group the choice

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

@@ -91,7 +91,15 @@ class PropertyAccessDecorator implements ChoiceListFactoryInterface
         if ($value instanceof PropertyPath) {
             $accessor = $this->propertyAccessor;
             $value = function ($choice) use ($accessor, $value) {
-                return $accessor->getValue($choice, $value);
+                // The callable may be invoked with a non-object/array value
+                // when such values are passed to
+                // ChoiceListInterface::getValuesForChoices(). Handle this case
+                // so that the call to getValue() doesn't break.
+                if (is_object($choice) || is_array($choice)) {
+                    return $accessor->getValue($choice, $value);
+                }
+
+                return null;
             };
         }
 

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

@@ -43,6 +43,13 @@ class LazyChoiceList implements ChoiceListInterface
      */
     private $value;
 
+    /**
+     * Whether to use the value callback to compare choices.
+     *
+     * @var bool
+     */
+    private $compareByValue;
+
     /**
      * @var ChoiceListInterface
      */
@@ -59,10 +66,11 @@ class LazyChoiceList implements ChoiceListInterface
      * @param null|callable         $value  The callable generating the choice
      *                                      values
      */
-    public function __construct(ChoiceLoaderInterface $loader, $value = null)
+    public function __construct(ChoiceLoaderInterface $loader, $value = null, $compareByValue = false)
     {
         $this->loader = $loader;
         $this->value = $value;
+        $this->compareByValue = $compareByValue;
     }
 
     /**

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

@@ -54,15 +54,15 @@ class ArrayChoiceListTest extends AbstractChoiceListTest
 
     public function testCreateChoiceListWithValueCallback()
     {
-        $callback = function ($choice, $key) {
+        $callback = function ($choice) {
-            return $key.':'.$choice;
+            return ':'.$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(2 => ':foo', 7 => ':bar', 10 => ':baz'), $choiceList->getValues());
-        $this->assertSame(array(1 => 'foo', 2 => 'baz'), $choiceList->getChoicesForValues(array(1 => '2:foo', 2 => '10:baz')));
+        $this->assertSame(array(1 => 'foo', 2 => 'baz'), $choiceList->getChoicesForValues(array(1 => ':foo', 2 => ':baz')));
-        $this->assertSame(array(1 => '2:foo', 2 => '10:baz'), $choiceList->getValuesForChoices(array(1 => 'foo', 2 => 'baz')));
+        $this->assertSame(array(1 => ':foo', 2 => ':baz'), $choiceList->getValuesForChoices(array(1 => 'foo', 2 => 'baz')));
     }
 
     public function testCompareChoicesByIdentityByDefault()
@@ -76,20 +76,6 @@ class ArrayChoiceListTest extends AbstractChoiceListTest
 
         $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

@@ -183,14 +183,14 @@ class ArrayKeyChoiceListTest extends AbstractChoiceListTest
 
     public function testCreateChoiceListWithValueCallback()
     {
-        $callback = function ($choice, $key) {
+        $callback = function ($choice) {
-            return $key.':'.$choice;
+            return ':'.$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(2 => ':foo', 7 => ':bar', 10 => ':baz'), $choiceList->getValues());
-        $this->assertSame(array(1 => 'foo', 2 => 'baz'), $choiceList->getChoicesForValues(array(1 => '2:foo', 2 => '10:baz')));
+        $this->assertSame(array(1 => 'foo', 2 => 'baz'), $choiceList->getChoicesForValues(array(1 => ':foo', 2 => ':baz')));
-        $this->assertSame(array(1 => '2:foo', 2 => '10:baz'), $choiceList->getValuesForChoices(array(1 => 'foo', 2 => 'baz')));
+        $this->assertSame(array(1 => ':foo', 2 => ':baz'), $choiceList->getValuesForChoices(array(1 => 'foo', 2 => 'baz')));
     }
 }

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

@@ -150,23 +150,6 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase
         $this->assertObjectListWithCustomValues($list);
     }
 
-    public function testCreateFromChoicesFlatValuesClosureReceivesKey()
-    {
-        $list = $this->factory->createListFromChoices(
-            array('A' => $this->obj1, 'B' => $this->obj2, 'C' => $this->obj3, 'D' => $this->obj4),
-            function ($object, $key) {
-                switch ($key) {
-                    case 'A': return 'a';
-                    case 'B': return 'b';
-                    case 'C': return '1';
-                    case 'D': return '2';
-                }
-            }
-        );
-
-        $this->assertObjectListWithCustomValues($list);
-    }
-
     public function testCreateFromChoicesGrouped()
     {
         $list = $this->factory->createListFromChoices(
@@ -217,26 +200,6 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase
         $this->assertObjectListWithCustomValues($list);
     }
 
-    public function testCreateFromChoicesGroupedValuesAsClosureReceivesKey()
-    {
-        $list = $this->factory->createListFromChoices(
-            array(
-                'Group 1' => array('A' => $this->obj1, 'B' => $this->obj2),
-                'Group 2' => array('C' => $this->obj3, 'D' => $this->obj4),
-            ),
-            function ($object, $key) {
-                switch ($key) {
-                    case 'A': return 'a';
-                    case 'B': return 'b';
-                    case 'C': return '1';
-                    case 'D': return '2';
-                }
-            }
-        );
-
-        $this->assertObjectListWithCustomValues($list);
-    }
-
     /**
      * @expectedException \Symfony\Component\Form\Exception\UnexpectedTypeException
      */
@@ -306,23 +269,6 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase
         $this->assertScalarListWithCustomValues($list);
     }
 
-    public function testCreateFromFlippedChoicesFlatValuesClosureReceivesKey()
-    {
-        $list = $this->factory->createListFromFlippedChoices(
-            array('a' => 'A', 'b' => 'B', 'c' => 'C', 'd' => 'D'),
-            function ($choice, $key) {
-                switch ($key) {
-                    case 'A': return 'a';
-                    case 'B': return 'b';
-                    case 'C': return '1';
-                    case 'D': return '2';
-                }
-            }
-        );
-
-        $this->assertScalarListWithCustomValues($list);
-    }
-
     public function testCreateFromFlippedChoicesGrouped()
     {
         $list = $this->factory->createListFromFlippedChoices(
@@ -380,26 +326,6 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase
         $this->assertScalarListWithCustomValues($list);
     }
 
-    public function testCreateFromFlippedChoicesGroupedValuesAsClosureReceivesKey()
-    {
-        $list = $this->factory->createListFromFlippedChoices(
-            array(
-                'Group 1' => array('a' => 'A', 'b' => 'B'),
-                'Group 2' => array('c' => 'C', 'd' => 'D'),
-            ),
-            function ($choice, $key) {
-                switch ($key) {
-                    case 'A': return 'a';
-                    case 'B': return 'b';
-                    case 'C': return '1';
-                    case 'D': return '2';
-                }
-            }
-        );
-
-        $this->assertScalarListWithCustomValues($list);
-    }
-
     public function testCreateFromLoader()
     {
         $loader = $this->getMock('Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface');
@@ -537,12 +463,9 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase
 
     public function testCreateViewFlatPreferredChoicesClosureReceivesKey()
     {
-        $obj2 = $this->obj2;
-        $obj3 = $this->obj3;
-
         $view = $this->factory->createView(
             $this->list,
-            function ($object, $key) use ($obj2, $obj3) {
+            function ($object, $key) {
                 return 'B' === $key || 'C' === $key;
             }
         );
@@ -550,6 +473,18 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase
         $this->assertFlatView($view);
     }
 
+    public function testCreateViewFlatPreferredChoicesClosureReceivesValue()
+    {
+        $view = $this->factory->createView(
+            $this->list,
+            function ($object, $key, $value) {
+                return '1' === $value || '2' === $value;
+            }
+        );
+
+        $this->assertFlatView($view);
+    }
+
     public function testCreateViewFlatLabelAsCallable()
     {
         $view = $this->factory->createView(
@@ -587,6 +522,24 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase
         $this->assertFlatView($view);
     }
 
+    public function testCreateViewFlatLabelClosureReceivesValue()
+    {
+        $view = $this->factory->createView(
+            $this->list,
+            array($this->obj2, $this->obj3),
+            function ($object, $key, $value) {
+                switch ($value) {
+                    case '0': return 'A';
+                    case '1': return 'B';
+                    case '2': return 'C';
+                    case '3': return 'D';
+                }
+            }
+        );
+
+        $this->assertFlatView($view);
+    }
+
     public function testCreateViewFlatIndexAsCallable()
     {
         $view = $this->factory->createView(
@@ -632,6 +585,25 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase
         $this->assertFlatViewWithCustomIndices($view);
     }
 
+    public function testCreateViewFlatIndexClosureReceivesValue()
+    {
+        $view = $this->factory->createView(
+            $this->list,
+            array($this->obj2, $this->obj3),
+            null, // label
+            function ($object, $key, $value) {
+                switch ($value) {
+                    case '0': return 'w';
+                    case '1': return 'x';
+                    case '2': return 'y';
+                    case '3': return 'z';
+                }
+            }
+        );
+
+        $this->assertFlatViewWithCustomIndices($view);
+    }
+
     public function testCreateViewFlatGroupByAsArray()
     {
         $view = $this->factory->createView(
@@ -724,6 +696,21 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase
         $this->assertGroupedView($view);
     }
 
+    public function testCreateViewFlatGroupByClosureReceivesValue()
+    {
+        $view = $this->factory->createView(
+            $this->list,
+            array($this->obj2, $this->obj3),
+            null, // label
+            null, // index
+            function ($object, $key, $value) {
+                return '0' === $value || '1' === $value ? 'Group 1' : 'Group 2';
+            }
+        );
+
+        $this->assertGroupedView($view);
+    }
+
     public function testCreateViewFlatAttrAsArray()
     {
         $view = $this->factory->createView(
@@ -805,6 +792,26 @@ class DefaultChoiceListFactoryTest extends \PHPUnit_Framework_TestCase
         $this->assertFlatViewWithAttr($view);
     }
 
+    public function testCreateViewFlatAttrClosureReceivesValue()
+    {
+        $view = $this->factory->createView(
+            $this->list,
+            array($this->obj2, $this->obj3),
+            null, // label
+            null, // index
+            null, // group
+            function ($object, $key, $value) {
+                switch ($value) {
+                    case '1': return array('attr1' => 'value1');
+                    case '2': return array('attr2' => 'value2');
+                    default: return array();
+                }
+            }
+        );
+
+        $this->assertFlatViewWithAttr($view);
+    }
+
     public function testCreateViewForLegacyChoiceList()
     {
         $preferred = array(new ChoiceView('Preferred', 'x', 'x'));