[Form] Fixed PR comments

symfony · webmozart · Apr 1, 2015 · Sep 26, 2014 · Mar 25, 2015 · Mar 25, 2015
commit d6179c830be7f2245ad56b6a800a33275a802689
diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php
@@ -97,25 +97,12 @@ public function loadChoiceList($value = null)
     }
 
     /**
-     * Loads the values corresponding to the given objects.
-     *
-     * The values are returned with the same keys and in the same order as the
-     * corresponding objects in the given array.
-     *
-     * Optionally, a callable can be passed for generating the choice values.
-     * The callable receives the object as first and the array key as the second
-     * argument.
-     *
-     * @param array $objects       An array of objects. Non-existing objects in
-     *                             this array are ignored
-     * @param null|callable $value The callable generating the choice values
-     *
-     * @return string[] An array of choice values
+     * {@inheritdoc}
      */
-    public function loadValuesForChoices(array $objects, $value = null)
+    public function loadValuesForChoices(array $choices, $value = null)
     {
         // Performance optimization
-        if (empty($objects)) {
+        if (empty($choices)) {
             return array();
         }
    
@@ -127,7 +114,7 @@ public function loadValuesForChoices(array $objects, $value = null)
             $values = array();
 
             // Maintain order and indices of the given objects
-            foreach ($objects as $i => $object) {
+            foreach ($choices as $i => $object) {
                 if ($object instanceof $this->class) {
                     // Make sure to convert to the right format
                     $values[$i] = (string) $this->idReader->getIdValue($object);
@@ -137,24 +124,11 @@ public function loadValuesForChoices(array $objects, $value = null)
             return $values;
         }
 
-        return $this->loadChoiceList($value)->getValuesForChoices($objects);
+        return $this->loadChoiceList($value)->getValuesForChoices($choices);
     }
 
     /**
-     * Loads the objects corresponding to the given values.
-     *
-     * The objects are returned with the same keys and in the same order as the
-     * corresponding values in the given array.
-     *
-     * Optionally, a callable can be passed for generating the choice values.
-     * The callable receives the object as first and the array key as the second
-     * argument.
-     *
-     * @param string[] $values     An array of choice values. Non-existing
-     *                             values in this array are ignored
-     * @param null|callable $value The callable generating the choice values
-     *
-     * @return array An array of objects
+     * {@inheritdoc}
      */
     public function loadChoicesForValues(array $values, $value = null)
     {

diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php
@@ -129,6 +129,8 @@ public function __construct(ObjectManager $manager, $class, $labelPath = null, E
 628C
        }
 
         parent::__construct($entities, $labelPath, $preferredEntities, $groupPath, null, $propertyAccessor);
+
+        trigger_error('The '.__CLASS__.' class is deprecated since version 2.7 and will be removed in 3.0. Use Symfony\Bridge\Doctrine\Form\ChoiceList\DoctrineChoiceLoader instead.', E_USER_DEPRECATED);
 trigger_error('The '.__NAMESPACE__.'\LegacyValidator class is deprecated since version 2.5 and will be removed in 3.0.', E_USER_DEPRECATED); 
 trigger_error('The '.__NAMESPACE__.'\LegacyValidator class is deprecated since version 2.5 and will be removed in 3.0.', E_USER_DEPRECATED); 
     }
 
     /**

diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php
@@ -37,9 +37,14 @@ class ORMQueryBuilderLoader implements EntityLoaderInterface
     /**
      * Construct an ORM Query Builder Loader.
      *
-     * @param QueryBuilder|\Closure $queryBuilder
-     * @param EntityManager         $manager
-     * @param string                $class
+     * @param QueryBuilder|\Closure $queryBuilder The query builder or a closure
+     *                                            for creating the query builder.
+     *                                            Passing a closure is
+     *                                            deprecated and will not be
+     *                                            supported anymore as of
+     *                                            Symfony 3.0.
+     * @param EntityManager         $manager      Deprecated.
+     * @param string                $class        Deprecated.
      *
      * @throws UnexpectedTypeException
      */
@@ -51,13 +56,16 @@ public function __construct($queryBuilder, $manager = null, $class = null)
             throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder or \Closure');
         }
 
-        // This block is not executed anymore since Symfony 2.7. The query
-        // builder closure is already invoked in DoctrineType
         if ($queryBuilder instanceof \Closure) {
+            trigger_error('Passing a QueryBuilder closure to '.__CLASS__.'::__construct() is deprecated since version 2.7 and will be removed in 3.0.', E_USER_DEPRECATED);
+
             if (!$manager instanceof EntityManager) {
                 throw new UnexpectedTypeException($manager, 'Doctrine\ORM\EntityManager');
             }
 
+            trigger_error('Passing an EntityManager to '.__CLASS__.'::__construct() is deprecated since version 2.7 and will be removed in 3.0.', E_USER_DEPRECATED);
+            trigger_error('Passing a class to '.__CLASS__.'::__construct() is deprecated since version 2.7 and will be removed in 3.0.', E_USER_DEPRECATED);
+
             $queryBuilder = $queryBuilder($manager->getRepository($class));
 
             if (!$queryBuilder instanceof QueryBuilder) {

diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php b/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php
@@ -116,27 +116,10 @@ public function configureOptions(OptionsResolver $resolver)
         $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) {
+            // This closure and the "query_builder" options should be pushed to
+            // EntityType in Symfony 3.0 as they are specific to the ORM
+
             // Unless the choices are given explicitly, load them on demand
             if (null === $options['choices']) {
                 // We consider two query builders with an equal SQL string and
@@ -243,6 +226,13 @@ public function configureOptions(OptionsResolver $resolver)
             return $em;
         };
 
+        // deprecation note
+        $propertyNormalizer = function (Options $options, $propertyName) {
+            trigger_error('The "property" option is deprecated since version 2.7 and will be removed in 3.0. Use "choice_label" instead.', E_USER_DEPRECATED);
+
+            return $propertyName;
+        };
+
         // Invoke the query builder closure so that we can cache choice lists
         // for equal query builders
         $queryBuilderNormalizer = function (Options $options, $queryBuilder) {
@@ -257,6 +247,35 @@ public function configureOptions(OptionsResolver $resolver)
             return $queryBuilder;
         };
 
+        // deprecation note
+        $loaderNormalizer = function (Options $options, $loader) {
+            trigger_error('The "loader" option is deprecated since version 2.7 and will be removed in 3.0. Override getLoader() instead.', E_USER_DEPRECATED);
+
+            return $loader;
+        };
+
+        // Set the "id_reader" option via the normalizer. This option is not
+        // supposed to be set by the user.
+        $idReaderNormalizer = 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];
+        };
+
         $resolver->setDefaults(array(
             'em' => null,
             'property' => null, // deprecated, use "choice_label"
@@ -268,17 +287,19 @@ public function configureOptions(OptionsResolver $resolver)
             'choice_label' => $choiceLabel,
             'choice_name' => $choiceName,
             'choice_value' => $choiceValue,
-            'id_reader' => $idReader,
+            'id_reader' => null, // internal
         ));
 
         $resolver->setRequired(array('class'));
 
         $resolver->setNormalizer('em', $emNormalizer);
+        $resolver->setNormalizer('property', $propertyNormalizer);
         $resolver->setNormalizer('query_builder', $queryBuilderNormalizer);
+        $resolver->setNormalizer('loader', $loaderNormalizer);
+        $resolver->setNormalizer('id_reader', $idReaderNormalizer);
 
         $resolver->setAllowedTypes('em', array('null', 'string', 'Doctrine\Common\Persistence\ObjectManager'));
         $resolver->setAllowedTypes('loader', array('null', 'Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface'));
-        $resolver->setAllowedTypes('query_builder', array('null', 'callable', 'Doctrine\ORM\QueryBuilder'));
     }
 
     /**

diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php b/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php
@@ -20,9 +20,9 @@ class EntityType extends DoctrineType
     /**
      * Return the default loader object.
      *
-     * @param ObjectManager         $manager
-     * @param QueryBuilder|\Closure $queryBuilder
-     * @param string                $class
+     * @param ObjectManager $manager
+     * @param QueryBuilder  $queryBuilder
+     * @param string        $class
      *
      * @return ORMQueryBuilderLoader
      */

diff --git a/src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php b/src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php
@@ -51,10 +51,10 @@ 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 callable $value   The callable for creating the value for a
-     *                          choice. If `null` is passed, incrementing
-     *                          integers are used as values
+     * @param array         $choices The selectable choices
+     * @param callable|null $value   The callable for creating the value for a
+     *                               choice. If `null` is passed, incrementing
+     *                               integers are used as values
      */
     public function __construct(array $choices, $value = null)
     {

diff --git a/src/Symfony/Component/Form/ChoiceList/ArrayKeyChoiceList.php b/src/Symfony/Component/Form/ChoiceList/ArrayKeyChoiceList.php
@@ -41,7 +41,7 @@
  * @author Bernhard Schussek <bschussek@gmail.com>
  *
  * @deprecated Added for backwards compatibility in Symfony 2.7, to be removed
- *             in Symfony 3.0.
+ *             in Symfony 3.0. Use {@link ArrayChoiceList} instead.
  */
 class ArrayKeyChoiceList extends ArrayChoiceList
 {
@@ -113,6 +113,8 @@ public function __construct(array $choices, $value = null)
         }
 
         parent::__construct($choices, $value);
+
+        trigger_error('The '.__CLASS__.' class was added for backwards compatibility in version 2.7 and will be removed in 3.0. Use Symfony\Component\Form\ChoiceList\ArrayChoiceList instead.', E_USER_DEPRECATED);
     }
 
     /**

diff --git a/src/Symfony/Component/Form/ChoiceList/Factory/CachingFactoryDecorator.php b/src/Symfony/Component/Form/ChoiceList/Factory/CachingFactoryDecorator.php
@@ -91,10 +91,6 @@ public function getDecoratedFactory()
      */
     public function createListFromChoices($choices, $value = null)
     {
-        if (!is_array($choices) && !$choices instanceof \Traversable) {
-            throw new UnexpectedTypeException($choices, 'array or \Traversable');
-        }
-
         if ($choices instanceof \Traversable) {
             $choices = iterator_to_array($choices);
         }
@@ -124,10 +120,6 @@ public function createListFromChoices($choices, $value = null)
      */
     public function createListFromFlippedChoices($choices, $value = null)
     {
-        if (!is_array($choices) && !$choices instanceof \Traversable) {
-            throw new UnexpectedTypeException($choices, 'array or \Traversable');
-        }
-
         if ($choices instanceof \Traversable) {
             $choices = iterator_to_array($choices);
         }

diff --git a/src/Symfony/Component/Form/ChoiceList/Factory/DefaultChoiceListFactory.php b/src/Symfony/Component/Form/ChoiceList/Factory/DefaultChoiceListFactory.php
@@ -85,10 +85,6 @@ public static function flattenFlipped(array $array, &$output)
      */
     public function createListFromChoices($choices, $value = null)
     {
-        if (!is_array($choices) && !$choices instanceof \Traversable) {
-            throw new UnexpectedTypeException($choices, 'array or \Traversable');
-        }
-
         if ($choices instanceof \Traversable) {
             $choices = iterator_to_array($choices);
         }
@@ -109,10 +105,6 @@ public function createListFromChoices($choices, $value = null)
      */
     public function createListFromFlippedChoices($choices, $value = null)
     {
-        if (!is_array($choices) && !$choices instanceof \Traversable) {
-            throw new UnexpectedTypeException($choices, 'array or \Traversable');
-        }
-
         if ($choices instanceof \Traversable) {
             $choices = iterator_to_array($choices);
         }
@@ -140,10 +132,6 @@ public function createListFromFlippedChoices($choices, $value = null)
      */
     public function createListFromLoader(ChoiceLoaderInterface $loader, $value = null)
     {
-        if (null !== $value && !is_callable($value)) {
-            throw new UnexpectedTypeException($value, 'null or callable');
-        }
-
         return new LazyChoiceList($loader, $value);
     }
 
@@ -152,26 +140,6 @@ public function createListFromLoader(ChoiceLoaderInterface $loader, $value = nul
      */
     public function createView(ChoiceListInterface $list, $preferredChoices = null, $label = null, $index = null, $groupBy = null, $attr = null)
     {
-        if (null !== $preferredChoices && !is_array($preferredChoices) && !is_callable($preferredChoices)) {
-            throw new UnexpectedTypeException($preferredChoices, 'null, array or callable');
-        }
-
-        if (null !== $label && !is_callable($label)) {
-            throw new UnexpectedTypeException($label, 'null or callable');
-        }
-
-        if (null !== $index && !is_callable($index)) {
-            throw new UnexpectedTypeException($index, 'null or callable');
-        }
-
-        if (null !== $groupBy && !is_array($groupBy) && !$groupBy instanceof \Traversable && !is_callable($groupBy)) {
-            throw new UnexpectedTypeException($groupBy, 'null, array, \Traversable or callable');
-        }
-
-        if (null !== $attr && !is_array($attr) && !is_callable($attr)) {
-            throw new UnexpectedTypeException($attr, 'null, array or callable');
-        }
-
         // Backwards compatibility
         if ($list instanceof LegacyChoiceListInterface && null === $preferredChoices
             && null === $label && null === $index && null === $groupBy && null === $attr) {
@@ -247,7 +215,7 @@ public function createView(ChoiceListInterface $list, $preferredChoices = null,
             );
         }
 
-        // Remove any empty group views that may have been created by
+        // Remove any empty group view that may have been created by
         // addChoiceViewGroupedBy()
         foreach ($preferredViews as $key => $view) {
             if ($view instanceof ChoiceGroupView && 0 === count($view->choices)) {

diff --git a/src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php b/src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php
@@ -153,16 +153,12 @@ public function createListFromLoader(ChoiceLoaderInterface $loader, $value = nul
     /**
      * {@inheritdoc}
      *
-     * @param ChoiceListInterface                           $list             The choice list
-     * @param null|array|callable|PropertyPath              $preferredChoices The preferred choices
-     * @param null|callable|PropertyPath                    $label            The callable or path
-     *                                                                        generating the choice labels
-     * @param null|callable|PropertyPath                    $index            The callable or path
-     *                                                                        generating the view indices
-     * @param null|array|\Traversable|callable|PropertyPath $groupBy          The callable or path
-     *                                                                        generating the group names
-     * @param null|array|callable|PropertyPath              $attr             The callable or path
-     *                                                                        generating the HTML attributes
+     * @param ChoiceListInterface                                  $list             The choice list
+     * @param null|array|callable|string|PropertyPath              $preferredChoices The preferred choices
+     * @param null|callable|string|PropertyPath                    $label            The callable or path generating the choice labels
+     * @param null|callable|string|PropertyPath                    $index            The callable or path generating the view indices
+     * @param null|array|\Traversable|callable|string|PropertyPath $groupBy          The callable or path generating the group names
+     * @param null|array|callable|string|PropertyPath              $attr             The callable or path generating the HTML attributes
      *
      * @return ChoiceListView The choice list view
      */

diff --git a/src/Symfony/Component/Form/ChoiceList/LazyChoiceList.php b/src/Symfony/Component/Form/ChoiceList/LazyChoiceList.php
@@ -51,7 +51,7 @@ class LazyChoiceList implements ChoiceListInterface
     private $compareByValue;
 
     /**
-     * @var ChoiceListInterface
+     * @var ChoiceListInterface|null
      */
     private $loadedList;