From 43f04bb9a2737062461d1b863535cc057fa8b6d6 Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Wed, 22 Feb 2023 22:02:25 +0100 Subject: [PATCH 1/4] Change FormTypeValidatorExtension construct signature --- UPGRADE-6.3.md | 5 +++++ .../FrameworkBundle/Resources/config/form.php | 1 - src/Symfony/Component/Form/CHANGELOG.md | 5 +++++ .../Type/FormTypeValidatorExtension.php | 17 +++++++++++++++-- .../Extension/Validator/ValidatorExtension.php | 2 +- 5 files changed, 26 insertions(+), 4 deletions(-) diff --git a/UPGRADE-6.3.md b/UPGRADE-6.3.md index 1b6479a48a083..0e39769beeb05 100644 --- a/UPGRADE-6.3.md +++ b/UPGRADE-6.3.md @@ -7,6 +7,11 @@ DependencyInjection * Deprecate `PhpDumper` options `inline_factories_parameter` and `inline_class_loader_parameter`, use `inline_factories` and `inline_class_loader` instead * Deprecate undefined and numeric keys with `service_locator` config, use string aliases instead +Form +---- + + * Change the signature of `FormTypeValidatorExtension::__construct(ValidatorInterface $validator, bool $legacyErrorMessages = true, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null)` to `__construct(ValidatorInterface $validator, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null)` + FrameworkBundle --------------- diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/form.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/form.php index 3c936a284b325..2a6e1e7c3ef98 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/form.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/form.php @@ -132,7 +132,6 @@ ->set('form.type_extension.form.validator', FormTypeValidatorExtension::class) ->args([ service('validator'), - false, service('twig.form.renderer')->ignoreOnInvalid(), service('translator')->ignoreOnInvalid(), ]) diff --git a/src/Symfony/Component/Form/CHANGELOG.md b/src/Symfony/Component/Form/CHANGELOG.md index 68622612e30b3..6dce01183df2d 100644 --- a/src/Symfony/Component/Form/CHANGELOG.md +++ b/src/Symfony/Component/Form/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +6.3 +--- + + * Change the signature of `FormTypeValidatorExtension::__construct(ValidatorInterface $validator, bool $legacyErrorMessages = true, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null)` to `__construct(ValidatorInterface $validator, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null)` + 6.2 --- diff --git a/src/Symfony/Component/Form/Extension/Validator/Type/FormTypeValidatorExtension.php b/src/Symfony/Component/Form/Extension/Validator/Type/FormTypeValidatorExtension.php index 4602918c3a9b8..b3b7f24a11113 100644 --- a/src/Symfony/Component/Form/Extension/Validator/Type/FormTypeValidatorExtension.php +++ b/src/Symfony/Component/Form/Extension/Validator/Type/FormTypeValidatorExtension.php @@ -29,10 +29,23 @@ class FormTypeValidatorExtension extends BaseValidatorExtension { private ValidatorInterface $validator; private ViolationMapper $violationMapper; - private bool $legacyErrorMessages; - public function __construct(ValidatorInterface $validator, bool $legacyErrorMessages = true, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null) + /** + * @param ValidatorInterface $validator + * @param FormRendererInterface|null $formRenderer + * @param TranslatorInterface|null $translator + */ + public function __construct(ValidatorInterface $validator/* , FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null*/) { + if (\func_num_args() > 3) { + trigger_deprecation('symfony/form', '6.3', 'The signature of constructor requires 3 arguments: "ValidatorInterface $validator, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null". Passing argument $legacyErrorMessages is deprecated.', __METHOD__); + $formRenderer = func_get_arg(2); + $translator = func_get_arg(3); + } else { + $formRenderer = func_get_arg(1); + $translator = func_get_arg(2); + } + $this->validator = $validator; $this->violationMapper = new ViolationMapper($formRenderer, $translator); } diff --git a/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php b/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php index fe1bd33f5f8d5..fe696ed12e32c 100644 --- a/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php +++ b/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php @@ -60,7 +60,7 @@ public function loadTypeGuesser(): ?FormTypeGuesserInterface protected function loadTypeExtensions(): array { return [ - new Type\FormTypeValidatorExtension($this->validator, $this->legacyErrorMessages, $this->formRenderer, $this->translator), + new Type\FormTypeValidatorExtension($this->validator, $this->formRenderer, $this->translator), new Type\RepeatedTypeValidatorExtension(), new Type\SubmitTypeValidatorExtension(), ]; From a9e9b85d99ae627117bdaebf96b9720068f942fb Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Thu, 23 Feb 2023 21:45:46 +0100 Subject: [PATCH 2/4] Fix CR issues --- UPGRADE-6.3.md | 2 +- src/Symfony/Component/Form/CHANGELOG.md | 2 +- .../Type/FormTypeValidatorExtension.php | 14 +++----- .../Validator/ValidatorExtension.php | 13 ++++++-- .../Test/Traits/ValidatorExtensionTrait.php | 2 +- .../FormValidatorPerformanceTest.php | 2 +- .../Type/FormTypeValidatorExtensionTest.php | 2 +- .../Validator/ValidatorExtensionTest.php | 33 ++++++++++++++++++- 8 files changed, 52 insertions(+), 18 deletions(-) diff --git a/UPGRADE-6.3.md b/UPGRADE-6.3.md index 0e39769beeb05..eee4fe2e09dd0 100644 --- a/UPGRADE-6.3.md +++ b/UPGRADE-6.3.md @@ -10,7 +10,7 @@ DependencyInjection Form ---- - * Change the signature of `FormTypeValidatorExtension::__construct(ValidatorInterface $validator, bool $legacyErrorMessages = true, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null)` to `__construct(ValidatorInterface $validator, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null)` + * Remove argument `$legacyErrorMessages` from the constructor of `FormTypeValidatorExtension` FrameworkBundle --------------- diff --git a/src/Symfony/Component/Form/CHANGELOG.md b/src/Symfony/Component/Form/CHANGELOG.md index 6dce01183df2d..b2366eb508a82 100644 --- a/src/Symfony/Component/Form/CHANGELOG.md +++ b/src/Symfony/Component/Form/CHANGELOG.md @@ -4,7 +4,7 @@ CHANGELOG 6.3 --- - * Change the signature of `FormTypeValidatorExtension::__construct(ValidatorInterface $validator, bool $legacyErrorMessages = true, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null)` to `__construct(ValidatorInterface $validator, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null)` + * Remove argument `$legacyErrorMessages` from the constructor of `FormTypeValidatorExtension` 6.2 --- diff --git a/src/Symfony/Component/Form/Extension/Validator/Type/FormTypeValidatorExtension.php b/src/Symfony/Component/Form/Extension/Validator/Type/FormTypeValidatorExtension.php index b3b7f24a11113..710067d34459f 100644 --- a/src/Symfony/Component/Form/Extension/Validator/Type/FormTypeValidatorExtension.php +++ b/src/Symfony/Component/Form/Extension/Validator/Type/FormTypeValidatorExtension.php @@ -31,19 +31,15 @@ class FormTypeValidatorExtension extends BaseValidatorExtension private ViolationMapper $violationMapper; /** - * @param ValidatorInterface $validator * @param FormRendererInterface|null $formRenderer * @param TranslatorInterface|null $translator */ - public function __construct(ValidatorInterface $validator/* , FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null*/) + public function __construct(ValidatorInterface $validator, /* FormRendererInterface */ $formRenderer = null, /* TranslatorInterface */ $translator = null) { - if (\func_num_args() > 3) { - trigger_deprecation('symfony/form', '6.3', 'The signature of constructor requires 3 arguments: "ValidatorInterface $validator, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null". Passing argument $legacyErrorMessages is deprecated.', __METHOD__); - $formRenderer = func_get_arg(2); - $translator = func_get_arg(3); - } else { - $formRenderer = func_get_arg(1); - $translator = func_get_arg(2); + if (\is_bool($formRenderer)) { + trigger_deprecation('symfony/form', '6.3', 'The signature of "%s" constructor requires 3 arguments: "ValidatorInterface $validator, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null". Passing argument $legacyErrorMessages is deprecated.', __CLASS__); + $formRenderer = $translator; + $translator = 4 <= \func_num_args() ? func_get_arg(3) : null; } $this->validator = $validator; diff --git a/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php b/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php index fe696ed12e32c..f46a960384568 100644 --- a/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php +++ b/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php @@ -30,11 +30,18 @@ class ValidatorExtension extends AbstractExtension private ValidatorInterface $validator; private ?FormRendererInterface $formRenderer; private ?TranslatorInterface $translator; - private bool $legacyErrorMessages; - public function __construct(ValidatorInterface $validator, bool $legacyErrorMessages = true, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null) + /** + * @param FormRendererInterface|null $formRenderer + * @param TranslatorInterface|null $translator + */ + public function __construct(ValidatorInterface $validator, /* FormRendererInterface */ $formRenderer = null, /* TranslatorInterface */ $translator = null) { - $this->legacyErrorMessages = $legacyErrorMessages; + if (\is_bool($formRenderer)) { + trigger_deprecation('symfony/form', '6.3', 'The signature of "%s" constructor requires 3 arguments: "ValidatorInterface $validator, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null". Passing argument $legacyErrorMessages is deprecated.', __CLASS__); + $formRenderer = $translator; + $translator = 4 <= \func_num_args() ? func_get_arg(3) : null; + } $metadata = $validator->getMetadataFor(\Symfony\Component\Form\Form::class); diff --git a/src/Symfony/Component/Form/Test/Traits/ValidatorExtensionTrait.php b/src/Symfony/Component/Form/Test/Traits/ValidatorExtensionTrait.php index b4b35fadf9c40..80db4ae18e3db 100644 --- a/src/Symfony/Component/Form/Test/Traits/ValidatorExtensionTrait.php +++ b/src/Symfony/Component/Form/Test/Traits/ValidatorExtensionTrait.php @@ -39,6 +39,6 @@ protected function getValidatorExtension(): ValidatorExtension $this->validator->expects($this->any())->method('getMetadataFor')->will($this->returnValue($metadata)); $this->validator->expects($this->any())->method('validate')->will($this->returnValue(new ConstraintViolationList())); - return new ValidatorExtension($this->validator, false); + return new ValidatorExtension($this->validator); } } diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorPerformanceTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorPerformanceTest.php index e8bfbc64ae5a5..64b49d7e9fb12 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorPerformanceTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorPerformanceTest.php @@ -23,7 +23,7 @@ class FormValidatorPerformanceTest extends FormPerformanceTestCase protected function getExtensions() { return [ - new ValidatorExtension(Validation::createValidator(), false), + new ValidatorExtension(Validation::createValidator()), ]; } diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php index 3b4cd77396c60..3ccfc54626f0d 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php @@ -76,7 +76,7 @@ public function testInvalidConstraint() public function testGroupSequenceWithConstraintsOption() { $form = Forms::createFormFactoryBuilder() - ->addExtension(new ValidatorExtension(Validation::createValidator(), false)) + ->addExtension(new ValidatorExtension(Validation::createValidator())) ->getFormFactory() ->create(FormTypeTest::TESTED_TYPE, null, ['validation_groups' => new GroupSequence(['First', 'Second'])]) ->add('field', TextTypeTest::TESTED_TYPE, [ diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php index c92bbe6651904..7a21c801a4e19 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Form\Tests\Extension\Validator; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Form\Extension\Validator\Constraints\Form as FormConstraint; use Symfony\Component\Form\Extension\Validator\ValidatorExtension; use Symfony\Component\Form\Extension\Validator\ValidatorTypeGuesser; @@ -24,8 +25,15 @@ class ValidatorExtensionTest extends TestCase { - public function test2Dot5ValidationApi() + use ExpectDeprecationTrait; + + /** + * @group legacy + */ + public function testLegacy2Dot5ValidationApi() { + $this->expectDeprecation('Since symfony/form 6.3: The signature of "Symfony\Component\Form\Extension\Validator\ValidatorExtension" constructor requires 3 arguments: "ValidatorInterface $validator, FormRendererInterface $formRenderer = null, TranslatorInterface $translator = null". Passing argument $legacyErrorMessages is deprecated.'); + $metadata = new ClassMetadata(Form::class); $metadataFactory = new FakeMetadataFactory(); @@ -46,4 +54,27 @@ public function test2Dot5ValidationApi() $this->assertSame(TraversalStrategy::NONE, $metadata->traversalStrategy); $this->assertCount(0, $metadata->getPropertyMetadata('children')); } + + public function test2Dot5ValidationApi() + { + $metadata = new ClassMetadata(Form::class); + + $metadataFactory = new FakeMetadataFactory(); + $metadataFactory->addMetadata($metadata); + + $validator = Validation::createValidatorBuilder() + ->setMetadataFactory($metadataFactory) + ->getValidator(); + + $extension = new ValidatorExtension($validator); + + $this->assertInstanceOf(ValidatorTypeGuesser::class, $extension->loadTypeGuesser()); + + $this->assertCount(1, $metadata->getConstraints()); + $this->assertInstanceOf(FormConstraint::class, $metadata->getConstraints()[0]); + + $this->assertSame(CascadingStrategy::NONE, $metadata->cascadingStrategy); + $this->assertSame(TraversalStrategy::NONE, $metadata->traversalStrategy); + $this->assertCount(0, $metadata->getPropertyMetadata('children')); + } } From aa8dc37299646c298053a6e362ce45d2aca8103b Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Sun, 5 Mar 2023 11:29:52 +0100 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Christian Flothmann --- UPGRADE-6.3.md | 2 +- src/Symfony/Component/Form/CHANGELOG.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/UPGRADE-6.3.md b/UPGRADE-6.3.md index eee4fe2e09dd0..db216be23bea6 100644 --- a/UPGRADE-6.3.md +++ b/UPGRADE-6.3.md @@ -10,7 +10,7 @@ DependencyInjection Form ---- - * Remove argument `$legacyErrorMessages` from the constructor of `FormTypeValidatorExtension` + * Remove argument `$legacyErrorMessages` from the constructor of `FormTypeValidatorExtension` and `ValidatorExtension` FrameworkBundle --------------- diff --git a/src/Symfony/Component/Form/CHANGELOG.md b/src/Symfony/Component/Form/CHANGELOG.md index b2366eb508a82..cf187dddbdd84 100644 --- a/src/Symfony/Component/Form/CHANGELOG.md +++ b/src/Symfony/Component/Form/CHANGELOG.md @@ -4,7 +4,7 @@ CHANGELOG 6.3 --- - * Remove argument `$legacyErrorMessages` from the constructor of `FormTypeValidatorExtension` + * Remove argument `$legacyErrorMessages` from the constructor of `FormTypeValidatorExtension` and `ValidatorExtension` 6.2 --- From 9819cdce94156a31b8875e2ca7b6d7f0f92426a3 Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Sun, 5 Mar 2023 11:47:59 +0100 Subject: [PATCH 4/4] Throw TypeError when args does not match --- .../Type/FormTypeValidatorExtension.php | 8 ++++ .../Validator/ValidatorExtension.php | 8 ++++ .../Validator/ValidatorExtensionTest.php | 40 +++++++++++++++++++ 3 files changed, 56 insertions(+) diff --git a/src/Symfony/Component/Form/Extension/Validator/Type/FormTypeValidatorExtension.php b/src/Symfony/Component/Form/Extension/Validator/Type/FormTypeValidatorExtension.php index 710067d34459f..c4da3a8fb7e0b 100644 --- a/src/Symfony/Component/Form/Extension/Validator/Type/FormTypeValidatorExtension.php +++ b/src/Symfony/Component/Form/Extension/Validator/Type/FormTypeValidatorExtension.php @@ -42,6 +42,14 @@ public function __construct(ValidatorInterface $validator, /* FormRendererInterf $translator = 4 <= \func_num_args() ? func_get_arg(3) : null; } + if (null !== $formRenderer && !$formRenderer instanceof FormRendererInterface) { + throw new \TypeError(sprintf('Argument 2 passed to "%s()" must be an instance of "%s" or null, "%s" given.', __METHOD__, FormRendererInterface::class, get_debug_type($formRenderer))); + } + + if (null !== $translator && !$translator instanceof TranslatorInterface) { + throw new \TypeError(sprintf('Argument 3 passed to "%s()" must be an instance of "%s" or null, "%s" given.', __METHOD__, TranslatorInterface::class, get_debug_type($formRenderer))); + } + $this->validator = $validator; $this->violationMapper = new ViolationMapper($formRenderer, $translator); } diff --git a/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php b/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php index f46a960384568..b1f6478b56792 100644 --- a/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php +++ b/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php @@ -43,6 +43,14 @@ public function __construct(ValidatorInterface $validator, /* FormRendererInterf $translator = 4 <= \func_num_args() ? func_get_arg(3) : null; } + if (null !== $formRenderer && !$formRenderer instanceof FormRendererInterface) { + throw new \TypeError(sprintf('Argument 2 passed to "%s()" must be an instance of "%s" or null, "%s" given.', __METHOD__, FormRendererInterface::class, get_debug_type($formRenderer))); + } + + if (null !== $translator && !$translator instanceof TranslatorInterface) { + throw new \TypeError(sprintf('Argument 3 passed to "%s()" must be an instance of "%s" or null, "%s" given.', __METHOD__, TranslatorInterface::class, get_debug_type($translator))); + } + $metadata = $validator->getMetadataFor(\Symfony\Component\Form\Form::class); // Register the form constraints in the validator programmatically. diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php index 7a21c801a4e19..64bd317fca591 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php @@ -55,6 +55,46 @@ public function testLegacy2Dot5ValidationApi() $this->assertCount(0, $metadata->getPropertyMetadata('children')); } + /** + * @group legacy + */ + public function testLegacyWithBadFormRendererType() + { + $metadata = new ClassMetadata(Form::class); + + $metadataFactory = new FakeMetadataFactory(); + $metadataFactory->addMetadata($metadata); + + $validator = Validation::createValidatorBuilder() + ->setMetadataFactory($metadataFactory) + ->getValidator(); + + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Argument 2 passed to "Symfony\Component\Form\Extension\Validator\ValidatorExtension::__construct()" must be an instance of "Symfony\Component\Form\FormRendererInterface" or null, "stdClass" given.'); + + new ValidatorExtension($validator, new \stdClass()); + } + + /** + * @group legacy + */ + public function testLegacyWithBadTranslatorType() + { + $metadata = new ClassMetadata(Form::class); + + $metadataFactory = new FakeMetadataFactory(); + $metadataFactory->addMetadata($metadata); + + $validator = Validation::createValidatorBuilder() + ->setMetadataFactory($metadataFactory) + ->getValidator(); + + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Argument 3 passed to "Symfony\Component\Form\Extension\Validator\ValidatorExtension::__construct()" must be an instance of "Symfony\Contracts\Translation\TranslatorInterface" or null, "stdClass" given.'); + + new ValidatorExtension($validator, null, new \stdClass()); + } + public function test2Dot5ValidationApi() { $metadata = new ClassMetadata(Form::class);