From 79a3ee21642339e4c84fab045e6fd4cc7efc29b6 Mon Sep 17 00:00:00 2001 From: Valentin Date: Fri, 9 Aug 2019 05:58:01 +0300 Subject: [PATCH] Deprecate passing false as $label to ChoiceListFactoryInterface --- UPGRADE-4.4.md | 1 + UPGRADE-5.0.md | 1 + src/Symfony/Component/Form/CHANGELOG.md | 1 + .../Factory/CachingFactoryDecorator.php | 12 +++++- .../Factory/DefaultChoiceListFactory.php | 40 +++++++++++++------ .../Factory/PropertyAccessDecorator.php | 12 +++++- .../Form/Extension/Core/Type/ChoiceType.php | 3 +- .../Factory/CachingFactoryDecoratorTest.php | 14 +++++++ .../Factory/DefaultChoiceListFactoryTest.php | 13 ++++++ .../Factory/PropertyAccessDecoratorTest.php | 14 +++++++ 10 files changed, 95 insertions(+), 16 deletions(-) diff --git a/UPGRADE-4.4.md b/UPGRADE-4.4.md index d6a38cda0e7c1..7a274efbcb810 100644 --- a/UPGRADE-4.4.md +++ b/UPGRADE-4.4.md @@ -85,6 +85,7 @@ Form reference date is deprecated. * Using `int` or `float` as data for the `NumberType` when the `input` option is set to `string` is deprecated. * Overriding the methods `FormIntegrationTestCase::setUp()`, `TypeTestCase::setUp()` and `TypeTestCase::tearDown()` without the `void` return-type is deprecated. + * Deprecated passing `false` as `$label` to `ChoiceListFactoryInterface::createView`, pass a callable that returns false instead. FrameworkBundle --------------- diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md index a3afcc44f920f..2312e295c6458 100644 --- a/UPGRADE-5.0.md +++ b/UPGRADE-5.0.md @@ -215,6 +215,7 @@ Form * The `regions` option was removed from the `TimezoneType`. * Added support for PHPUnit 8. A `void` return-type was added to the `FormIntegrationTestCase::setUp()`, `TypeTestCase::setUp()` and `TypeTestCase::tearDown()` methods. + * Passing `false` as `$label` to `ChoiceListFactoryInterface::createView` now throws a `TypeError`, pass a callable that returns false instead. FrameworkBundle --------------- diff --git a/src/Symfony/Component/Form/CHANGELOG.md b/src/Symfony/Component/Form/CHANGELOG.md index c76a15928f432..1c466ab4196fd 100644 --- a/src/Symfony/Component/Form/CHANGELOG.md +++ b/src/Symfony/Component/Form/CHANGELOG.md @@ -10,6 +10,7 @@ CHANGELOG * deprecated using `int` or `float` as data for the `NumberType` when the `input` option is set to `string` * The type guesser guesses the HTML accept attribute when a mime type is configured in the File or Image constraint. * Overriding the methods `FormIntegrationTestCase::setUp()`, `TypeTestCase::setUp()` and `TypeTestCase::tearDown()` without the `void` return-type is deprecated. + * deprecated passing `false` as `$label` to `ChoiceListFactoryInterface::createView`, pass a callable that returns false instead 4.3.0 ----- diff --git a/src/Symfony/Component/Form/ChoiceList/Factory/CachingFactoryDecorator.php b/src/Symfony/Component/Form/ChoiceList/Factory/CachingFactoryDecorator.php index 65d17afbdf0e7..c658525ab5c77 100644 --- a/src/Symfony/Component/Form/ChoiceList/Factory/CachingFactoryDecorator.php +++ b/src/Symfony/Component/Form/ChoiceList/Factory/CachingFactoryDecorator.php @@ -116,8 +116,18 @@ public function createListFromLoader(ChoiceLoaderInterface $loader, $value = nul /** * {@inheritdoc} */ - public function createView(ChoiceListInterface $list, $preferredChoices = null, $label = null, $index = null, $groupBy = null, $attr = null) + public function createView(ChoiceListInterface $list, $preferredChoices = null, $label = null, $index = null, $groupBy = null, $attr = null/* , bool $triggerDeprecation = true */) { + $triggerDeprecation = 6 >= \func_num_args() || func_get_arg(6); + + if (false === $label) { + if ($triggerDeprecation) { + @trigger_error(sprintf('Passing false as $label to %s is deprecated in Symfony 4.4 and will trigger a TypeError in 5.0, pass a callable that returns false instead.', __METHOD__), E_USER_DEPRECATED); + } + + $label = static function () { return false; }; + } + // The input is not validated on purpose. This way, the decorated // factory may decide which input to accept and which not. $hash = self::generateHash([$list, $preferredChoices, $label, $index, $groupBy, $attr]); diff --git a/src/Symfony/Component/Form/ChoiceList/Factory/DefaultChoiceListFactory.php b/src/Symfony/Component/Form/ChoiceList/Factory/DefaultChoiceListFactory.php index 7fe1f46247c4a..084e2ca5f9aec 100644 --- a/src/Symfony/Component/Form/ChoiceList/Factory/DefaultChoiceListFactory.php +++ b/src/Symfony/Component/Form/ChoiceList/Factory/DefaultChoiceListFactory.php @@ -45,20 +45,34 @@ public function createListFromLoader(ChoiceLoaderInterface $loader, $value = nul /** * {@inheritdoc} */ - public function createView(ChoiceListInterface $list, $preferredChoices = null, $label = null, $index = null, $groupBy = null, $attr = null) + public function createView(ChoiceListInterface $list, $preferredChoices = null, $label = null, $index = null, $groupBy = null, $attr = null/* , bool $triggerDeprecation = true */) { + $triggerDeprecation = 6 >= \func_num_args() || func_get_arg(6); + + if (false === $label) { + if ($triggerDeprecation) { + @trigger_error(sprintf('Passing false as $label to %s is deprecated in Symfony 4.4 and will trigger a TypeError in 5.0, pass a callable that returns false instead.', __METHOD__), E_USER_DEPRECATED); + } + + $label = static function () { return false; }; + } + $preferredViews = []; $preferredViewsOrder = []; $otherViews = []; $choices = $list->getChoices(); $keys = $list->getOriginalKeys(); - if (!\is_callable($preferredChoices) && !empty($preferredChoices)) { - // make sure we have keys that reflect order - $preferredChoices = array_values($preferredChoices); - $preferredChoices = static function ($choice) use ($preferredChoices) { - return array_search($choice, $preferredChoices, true); - }; + if (!\is_callable($preferredChoices)) { + if (empty($preferredChoices)) { + $preferredChoices = null; + } else { + // make sure we have keys that reflect order + $preferredChoices = array_values($preferredChoices); + $preferredChoices = static function ($choice) use ($preferredChoices) { + return array_search($choice, $preferredChoices, true); + }; + } } // The names are generated from an incrementing integer by default @@ -76,7 +90,7 @@ public function createView(ChoiceListInterface $list, $preferredChoices = null, self::addChoiceViewsGroupedByCallable( $groupBy, $choice, - (string) $value, + $value, $label, $keys, $index, @@ -126,7 +140,7 @@ public function createView(ChoiceListInterface $list, $preferredChoices = null, return new ChoiceListView($otherViews, $preferredViews); } - private static function addChoiceView($choice, $value, $label, $keys, &$index, $attr, $isPreferred, &$preferredViews, &$preferredViewsOrder, &$otherViews) + private static function addChoiceView($choice, string $value, ?callable $label, array $keys, &$index, $attr, ?callable $isPreferred, array &$preferredViews, array &$preferredViewsOrder, array &$otherViews) { // $value may be an integer or a string, since it's stored in the array // keys. We want to guarantee it's a string though. @@ -137,7 +151,7 @@ private static function addChoiceView($choice, $value, $label, $keys, &$index, $ if (null === $label) { // If the labels are null, use the original choice key by default $label = (string) $key; - } elseif (false !== $label) { + } else { // If "choice_label" is set to false and "expanded" is true, the value false // should be passed on to the "label" option of the checkboxes/radio buttons $dynamicLabel = $label($choice, $key, $value); @@ -154,7 +168,7 @@ private static function addChoiceView($choice, $value, $label, $keys, &$index, $ ); // $isPreferred may be null if no choices are preferred - if ($isPreferred && false !== $preferredKey = $isPreferred($choice, $key, $value)) { + if (null !== $isPreferred && false !== $preferredKey = $isPreferred($choice, $key, $value)) { $preferredViews[$nextIndex] = $view; $preferredViewsOrder[$nextIndex] = $preferredKey; } @@ -162,7 +176,7 @@ private static function addChoiceView($choice, $value, $label, $keys, &$index, $ $otherViews[$nextIndex] = $view; } - private static function addChoiceViewsFromStructuredValues($values, $label, $choices, $keys, &$index, $attr, $isPreferred, &$preferredViews, &$preferredViewsOrder, &$otherViews) + private static function addChoiceViewsFromStructuredValues(array $values, ?callable $label, array $choices, array $keys, &$index, $attr, ?callable $isPreferred, array &$preferredViews, array &$preferredViewsOrder, array &$otherViews) { foreach ($values as $key => $value) { if (null === $value) { @@ -214,7 +228,7 @@ private static function addChoiceViewsFromStructuredValues($values, $label, $cho } } - private static function addChoiceViewsGroupedByCallable($groupBy, $choice, $value, $label, $keys, &$index, $attr, $isPreferred, &$preferredViews, &$preferredViewsOrder, &$otherViews) + private static function addChoiceViewsGroupedByCallable(callable $groupBy, $choice, string $value, ?callable $label, array $keys, &$index, $attr, ?callable $isPreferred, array &$preferredViews, array &$preferredViewsOrder, array &$otherViews) { $groupLabels = $groupBy($choice, $keys[$value], $value); diff --git a/src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php b/src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php index 14be2a7a2ef40..c79f5bc3aea6a 100644 --- a/src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php +++ b/src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php @@ -128,8 +128,18 @@ public function createListFromLoader(ChoiceLoaderInterface $loader, $value = nul * * @return ChoiceListView The choice list view */ - public function createView(ChoiceListInterface $list, $preferredChoices = null, $label = null, $index = null, $groupBy = null, $attr = null) + public function createView(ChoiceListInterface $list, $preferredChoices = null, $label = null, $index = null, $groupBy = null, $attr = null/* , bool $triggerDeprecation = true */) { + $triggerDeprecation = 6 >= \func_num_args() || func_get_arg(6); + + if (false === $label) { + if ($triggerDeprecation) { + @trigger_error(sprintf('Passing false as $label to %s is deprecated in Symfony 4.4 and will trigger a TypeError in 5.0, pass a callable that returns false instead.', __METHOD__), E_USER_DEPRECATED); + } + + $label = static function () { return false; }; + } + $accessor = $this->propertyAccessor; if (\is_string($label)) { diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index 5a0986d89ff93..fc037795ec83d 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -410,7 +410,8 @@ private function createChoiceListView(ChoiceListInterface $choiceList, array $op $options['choice_label'], $options['choice_name'], $options['group_by'], - $options['choice_attr'] + $options['choice_attr'], + false ); } } diff --git a/src/Symfony/Component/Form/Tests/ChoiceList/Factory/CachingFactoryDecoratorTest.php b/src/Symfony/Component/Form/Tests/ChoiceList/Factory/CachingFactoryDecoratorTest.php index dcddc0298d6f7..6d1689689fd97 100644 --- a/src/Symfony/Component/Form/Tests/ChoiceList/Factory/CachingFactoryDecoratorTest.php +++ b/src/Symfony/Component/Form/Tests/ChoiceList/Factory/CachingFactoryDecoratorTest.php @@ -13,6 +13,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Symfony\Component\Form\ChoiceList\ChoiceListInterface; use Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator; /** @@ -484,6 +485,19 @@ public function testCreateViewDifferentAttributesClosure() $this->assertSame($view2, $this->factory->createView($list, null, null, null, null, $attr2)); } + /** + * @group legacy + * @expectedDeprecation Passing false as $label to Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator::createView is deprecated in Symfony 4.4 and will trigger a TypeError in 5.0, pass a callable that returns false instead. + */ + public function testCreateViewLabelFalseDeprecation() + { + $this->factory->createView( + $this->createMock(ChoiceListInterface::class), + null, // preferred choices + false // label + ); + } + public function provideSameChoices() { $object = (object) ['foo' => 'bar']; diff --git a/src/Symfony/Component/Form/Tests/ChoiceList/Factory/DefaultChoiceListFactoryTest.php b/src/Symfony/Component/Form/Tests/ChoiceList/Factory/DefaultChoiceListFactoryTest.php index b39f67acb7522..ca04fec0b1ba8 100644 --- a/src/Symfony/Component/Form/Tests/ChoiceList/Factory/DefaultChoiceListFactoryTest.php +++ b/src/Symfony/Component/Form/Tests/ChoiceList/Factory/DefaultChoiceListFactoryTest.php @@ -658,6 +658,19 @@ function ($object, $key, $value) { $this->assertFlatViewWithAttr($view); } + /** + * @group legacy + * @expectedDeprecation Passing false as $label to Symfony\Component\Form\ChoiceList\Factory\DefaultChoiceListFactory::createView is deprecated in Symfony 4.4 and will trigger a TypeError in 5.0, pass a callable that returns false instead. + */ + public function testCreateViewLabelFalseDeprecation() + { + $this->factory->createView( + $this->list, + null, // preferred choices + false // label + ); + } + private function assertScalarListWithChoiceValues(ChoiceListInterface $list) { $this->assertSame(['a', 'b', 'c', 'd'], $list->getValues()); diff --git a/src/Symfony/Component/Form/Tests/ChoiceList/Factory/PropertyAccessDecoratorTest.php b/src/Symfony/Component/Form/Tests/ChoiceList/Factory/PropertyAccessDecoratorTest.php index 87b8c87fc58e6..07a924ebaffec 100644 --- a/src/Symfony/Component/Form/Tests/ChoiceList/Factory/PropertyAccessDecoratorTest.php +++ b/src/Symfony/Component/Form/Tests/ChoiceList/Factory/PropertyAccessDecoratorTest.php @@ -13,6 +13,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Symfony\Component\Form\ChoiceList\ChoiceListInterface; use Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator; use Symfony\Component\PropertyAccess\PropertyPath; @@ -351,4 +352,17 @@ public function testCreateViewAttrAsPropertyPathInstance() new PropertyPath('property') )); } + + /** + * @group legacy + * @expectedDeprecation Passing false as $label to Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator::createView is deprecated in Symfony 4.4 and will trigger a TypeError in 5.0, pass a callable that returns false instead. + */ + public function testCreateViewLabelFalseDeprecation() + { + $this->factory->createView( + $this->createMock(ChoiceListInterface::class), + null, // preferred choices + false // label + ); + } }