From 6e622e4797ac879a908204d0f347659cfb50ce26 Mon Sep 17 00:00:00 2001 From: Sylvain Fabre Date: Tue, 17 Mar 2026 21:18:55 +0100 Subject: [PATCH] Refactors constraints to use named arguments Adopts named arguments for Symfony Validator constraint instantiation within field providers and their corresponding tests. This improves code readability and explicitness by clearly labeling each argument's purpose, making the constraint configuration easier to understand. --- phpstan-baseline.neon | 54 +++++++++---------- src/Validator/Constraints/Email.php | 21 +++++++- src/Validator/Constraints/EntityValidator.php | 2 +- src/Validator/Constraints/FloatScale.php | 26 ++------- src/Validator/Constraints/Money.php | 11 ++++ src/Validator/Constraints/Postal.php | 26 ++------- .../Field/EmailProvider.php | 6 ++- .../Field/IpProvider.php | 2 +- .../Field/LocaleProvider.php | 2 +- .../Field/StringProvider.php | 8 ++- .../Field/TextProvider.php | 9 ++-- .../Constraints/EmailValidatorTest.php | 24 ++++----- .../Constraints/EntityValidatorTest.php | 2 +- .../Constraints/FloatScaleValidatorTest.php | 2 +- .../Constraints/MoneyValidatorTest.php | 2 +- .../Constraints/PostalValidatorTest.php | 11 +--- .../Field/EmailProviderTest.php | 4 +- .../Field/IpProviderTest.php | 2 +- .../Field/LocaleProviderTest.php | 2 +- .../Field/StringProviderTest.php | 8 +-- .../Field/TextProviderTest.php | 4 +- 21 files changed, 105 insertions(+), 123 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 5af8fc5..e6c5c68 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -552,12 +552,6 @@ parameters: count: 1 path: src/Validator/Constraints/Email.php - - - message: '#^Method AssoConnect\\ValidatorBundle\\Validator\\Constraints\\Email\:\:__construct\(\) has parameter \$options with no value type specified in iterable type array\.$#' - identifier: missingType.iterableValue - count: 1 - path: src/Validator/Constraints/Email.php - - message: '#^Public property `checkDNS` not marked as readonly\.$#' identifier: shipmonk.publicPropertyNotReadonly @@ -582,18 +576,6 @@ parameters: count: 1 path: src/Validator/Constraints/EmployerIdentificationNumber.php - - - message: '#^Class AssoConnect\\ValidatorBundle\\Validator\\Constraints\\FloatScale has an uninitialized property \$scale\. Give it default value or assign it in the constructor\.$#' - identifier: property.uninitialized - count: 1 - path: src/Validator/Constraints/FloatScale.php - - - - message: '#^Method getRequiredOptions always return list, but is marked as array\$#' - identifier: shipmonk.returnListNotUsed - count: 1 - path: src/Validator/Constraints/FloatScale.php - - message: '#^Public property `message` not marked as readonly\.$#' identifier: shipmonk.publicPropertyNotReadonly @@ -636,6 +618,30 @@ parameters: count: 1 path: src/Validator/Constraints/LastDigitsUsSocialSecurityNumber.php + - + message: '#^Parameter \#1 \$min of method AssoConnect\\ValidatorBundle\\Validator\\Constraints\\Money\:\:__construct\(\) cannot have float as its type \- floats are not allowed\.$#' + identifier: float.type + count: 1 + path: src/Validator/Constraints/Money.php + + - + message: '#^Parameter \#2 \$max of method AssoConnect\\ValidatorBundle\\Validator\\Constraints\\Money\:\:__construct\(\) cannot have float as its type \- floats are not allowed\.$#' + identifier: float.type + count: 1 + path: src/Validator/Constraints/Money.php + + - + message: '#^Cannot assign float to \$this\->min \- floats are not allowed\.$#' + identifier: float.assign + count: 1 + path: src/Validator/Constraints/Money.php + + - + message: '#^Cannot assign float to \$this\->max \- floats are not allowed\.$#' + identifier: float.assign + count: 1 + path: src/Validator/Constraints/Money.php + - message: '#^Property AssoConnect\\ValidatorBundle\\Validator\\Constraints\\Money\:\:\$max cannot have float as its type \- floats are not allowed\.$#' identifier: float.property @@ -714,18 +720,6 @@ parameters: count: 1 path: src/Validator/Constraints/PhoneMobile.php - - - message: '#^Class AssoConnect\\ValidatorBundle\\Validator\\Constraints\\Postal has an uninitialized property \$countryPropertyPath\. Give it default value or assign it in the constructor\.$#' - identifier: property.uninitialized - count: 1 - path: src/Validator/Constraints/Postal.php - - - - message: '#^Method getRequiredOptions always return list, but is marked as array\$#' - identifier: shipmonk.returnListNotUsed - count: 1 - path: src/Validator/Constraints/Postal.php - - message: '#^Public property `countryPropertyPath` not marked as readonly\.$#' identifier: shipmonk.publicPropertyNotReadonly diff --git a/src/Validator/Constraints/Email.php b/src/Validator/Constraints/Email.php index f5ff9b1..9e46d20 100644 --- a/src/Validator/Constraints/Email.php +++ b/src/Validator/Constraints/Email.php @@ -21,14 +21,31 @@ class Email extends _Email public bool $checkDNS = false; public function __construct( - ?array $options = null, ?string $message = null, ?string $mode = null, ?callable $normalizer = null, ?array $groups = null, mixed $payload = null, + ?string $tldMessage = null, + ?string $dnsMessage = null, + ?bool $checkDNS = null, ) { - parent::__construct($options, $message, $mode, $normalizer, $groups, $payload); + parent::__construct( + message: $message, + mode: $mode, + normalizer: $normalizer, + groups: $groups, + payload: $payload, + ); $this->mode ??= 'strict'; + if (null !== $tldMessage) { + $this->tldMessage = $tldMessage; + } + if (null !== $dnsMessage) { + $this->dnsMessage = $dnsMessage; + } + if (null !== $checkDNS) { + $this->checkDNS = $checkDNS; + } } } diff --git a/src/Validator/Constraints/EntityValidator.php b/src/Validator/Constraints/EntityValidator.php index ff66da7..f7c7473 100644 --- a/src/Validator/Constraints/EntityValidator.php +++ b/src/Validator/Constraints/EntityValidator.php @@ -131,7 +131,7 @@ public function getConstraints(string $class, string $field): array } } elseif (($fieldMapping['type'] & ClassMetadataInfo::TO_MANY) !== 0) { // ToMany - $constraints[] = new All([ + $constraints[] = new All(constraints: [ new Type($fieldMapping['targetEntity']), ]); } else { diff --git a/src/Validator/Constraints/FloatScale.php b/src/Validator/Constraints/FloatScale.php index 8de1279..7979be6 100644 --- a/src/Validator/Constraints/FloatScale.php +++ b/src/Validator/Constraints/FloatScale.php @@ -18,32 +18,12 @@ class FloatScale extends Constraint public int $scale; - /** - * @param int|array $scale - * @param array $options - */ public function __construct( - $scale, + int $scale, ?array $groups = null, mixed $payload = null, - array $options = [] ) { - if (\is_array($scale)) { - $options = array_merge($scale, $options); - } elseif (null !== $scale) { - $options['value'] = $scale; - } - - parent::__construct($options, $groups, $payload); - } - - public function getDefaultOption(): string - { - return 'scale'; - } - - public function getRequiredOptions(): array - { - return ['scale']; + $this->scale = $scale; + parent::__construct(groups: $groups, payload: $payload); } } diff --git a/src/Validator/Constraints/Money.php b/src/Validator/Constraints/Money.php index 671e1de..9d71407 100644 --- a/src/Validator/Constraints/Money.php +++ b/src/Validator/Constraints/Money.php @@ -16,4 +16,15 @@ class Money extends Constraint public float $min = 0.0; public float $max = self::MAX; + + public function __construct( + float $min = 0.0, + float $max = self::MAX, + ?array $groups = null, + mixed $payload = null, + ) { + parent::__construct(groups: $groups, payload: $payload); + $this->min = $min; + $this->max = $max; + } } diff --git a/src/Validator/Constraints/Postal.php b/src/Validator/Constraints/Postal.php index df35022..4952c65 100644 --- a/src/Validator/Constraints/Postal.php +++ b/src/Validator/Constraints/Postal.php @@ -16,33 +16,13 @@ class Postal extends Constraint public string $countryPropertyPath; - /** - * @param string|array $countryPropertyPath - * @param array $options - */ public function __construct( - $countryPropertyPath, + string $countryPropertyPath, ?array $groups = null, mixed $payload = null, - array $options = [] ) { - if (\is_array($countryPropertyPath)) { - $options = array_merge($countryPropertyPath, $options); - } elseif (null !== $countryPropertyPath) { - $options['value'] = $countryPropertyPath; - } - - parent::__construct($options, $groups, $payload); - } - - public function getDefaultOption(): string - { - return 'countryPropertyPath'; - } - - public function getRequiredOptions(): array - { - return ['countryPropertyPath']; + $this->countryPropertyPath = $countryPropertyPath; + parent::__construct(groups: $groups, payload: $payload); } public string $message = 'The value {{ value }} is not a valid postal code.'; diff --git a/src/Validator/ConstraintsSetProvider/Field/EmailProvider.php b/src/Validator/ConstraintsSetProvider/Field/EmailProvider.php index 4ff870d..0767cc3 100644 --- a/src/Validator/ConstraintsSetProvider/Field/EmailProvider.php +++ b/src/Validator/ConstraintsSetProvider/Field/EmailProvider.php @@ -7,6 +7,7 @@ use AssoConnect\DoctrineTypesBundle\Doctrine\DBAL\Types\EmailType; use AssoConnect\ValidatorBundle\Validator\Constraints\Email; use Symfony\Component\Validator\Constraints\Length; +use Webmozart\Assert\Assert; class EmailProvider implements FieldConstraintsSetProviderInterface { @@ -17,9 +18,12 @@ public function supports(string $type): bool public function getConstraints(array $fieldMapping): array { + $length = $fieldMapping['length'] ?? 255; + Assert::positiveInteger($length); + return [ new Email(), - new Length(['max' => ($fieldMapping['length'] ?? 255)]), + new Length(max: $length), ]; } } diff --git a/src/Validator/ConstraintsSetProvider/Field/IpProvider.php b/src/Validator/ConstraintsSetProvider/Field/IpProvider.php index bb315ee..0afe509 100644 --- a/src/Validator/ConstraintsSetProvider/Field/IpProvider.php +++ b/src/Validator/ConstraintsSetProvider/Field/IpProvider.php @@ -17,7 +17,7 @@ public function supports(string $type): bool public function getConstraints(array $fieldMapping): array { return [ - new Ip(['version' => 'all']), + new Ip(version: 'all'), ]; } } diff --git a/src/Validator/ConstraintsSetProvider/Field/LocaleProvider.php b/src/Validator/ConstraintsSetProvider/Field/LocaleProvider.php index d4ee078..cc4b32d 100644 --- a/src/Validator/ConstraintsSetProvider/Field/LocaleProvider.php +++ b/src/Validator/ConstraintsSetProvider/Field/LocaleProvider.php @@ -17,7 +17,7 @@ public function supports(string $type): bool public function getConstraints(array $fieldMapping): array { return [ - new Locale(['canonicalize' => true]), + new Locale(canonicalize: true), ]; } } diff --git a/src/Validator/ConstraintsSetProvider/Field/StringProvider.php b/src/Validator/ConstraintsSetProvider/Field/StringProvider.php index 4d75585..7fd2223 100644 --- a/src/Validator/ConstraintsSetProvider/Field/StringProvider.php +++ b/src/Validator/ConstraintsSetProvider/Field/StringProvider.php @@ -7,6 +7,7 @@ use Doctrine\DBAL\Types\Types; use Symfony\Component\Validator\Constraints\Length; use Symfony\Component\Validator\Constraints\NotBlank; +use Webmozart\Assert\Assert; class StringProvider implements FieldConstraintsSetProviderInterface { @@ -17,10 +18,13 @@ public function supports(string $type): bool public function getConstraints(array $fieldMapping): array { - $constraints = [new Length(['max' => $fieldMapping['length'] ?? 255])]; + $length = $fieldMapping['length'] ?? 255; + Assert::positiveInteger($length); + + $constraints = [new Length(max: $length)]; if (isset($fieldMapping['nullable']) && true === $fieldMapping['nullable']) { - $constraints[] = new NotBlank(['allowNull' => true]); + $constraints[] = new NotBlank(allowNull: true); } return $constraints; diff --git a/src/Validator/ConstraintsSetProvider/Field/TextProvider.php b/src/Validator/ConstraintsSetProvider/Field/TextProvider.php index 9edd9cb..08cfa4e 100644 --- a/src/Validator/ConstraintsSetProvider/Field/TextProvider.php +++ b/src/Validator/ConstraintsSetProvider/Field/TextProvider.php @@ -6,6 +6,7 @@ use Doctrine\DBAL\Types\Types; use Symfony\Component\Validator\Constraints\Length; +use Webmozart\Assert\Assert; class TextProvider implements FieldConstraintsSetProviderInterface { @@ -16,11 +17,11 @@ public function supports(string $type): bool public function getConstraints(array $fieldMapping): array { + $length = $fieldMapping['length'] ?? 65535; + Assert::positiveInteger($length); + return [ - new Length([ - 'max' => $fieldMapping['length'] ?? 65535, - 'charset' => '8bit', - ]), + new Length(max: $length, charset: '8bit'), ]; } } diff --git a/tests/Validator/Constraints/EmailValidatorTest.php b/tests/Validator/Constraints/EmailValidatorTest.php index 78c574c..ab88c88 100644 --- a/tests/Validator/Constraints/EmailValidatorTest.php +++ b/tests/Validator/Constraints/EmailValidatorTest.php @@ -36,12 +36,12 @@ public function createValidator(): ConstraintValidator protected function getConstraint(): Constraint { - return new Email([ - 'message' => 'myMessage', - 'tldMessage' => 'myTldMessage', - 'dnsMessage' => 'myDnsMessage', - 'checkDNS' => true, - ]); + return new Email( + message: 'myMessage', + tldMessage: 'myTldMessage', + dnsMessage: 'myDnsMessage', + checkDNS: true, + ); } public function testValidateUnknownConstraint(): void @@ -137,12 +137,12 @@ public static function providerValidValues(): iterable public function testDisabledCheckDns(): void { - $this->validator->validate('john.doe@xn--gmail-9fa.com', new Email([ - 'message' => 'myMessage', - 'tldMessage' => 'myTldMessage', - 'dnsMessage' => 'myDnsMessage', - 'checkDNS' => false, - ])); + $this->validator->validate('john.doe@xn--gmail-9fa.com', new Email( + message: 'myMessage', + tldMessage: 'myTldMessage', + dnsMessage: 'myDnsMessage', + checkDNS: false, + )); self::assertNoViolation(); } diff --git a/tests/Validator/Constraints/EntityValidatorTest.php b/tests/Validator/Constraints/EntityValidatorTest.php index 387229e..ad49faf 100644 --- a/tests/Validator/Constraints/EntityValidatorTest.php +++ b/tests/Validator/Constraints/EntityValidatorTest.php @@ -119,7 +119,7 @@ public function testGetConstraintsForRelationToMany(): void $constraints = $this->validator->getConstraints('class', 'owningToMany'); self::assertArrayContainsSameObjects( $constraints, - [new All([new Type(MyEntityParent::class)])] + [new All(constraints: [new Type(MyEntityParent::class)])] ); self::assertInstanceOf(All::class, $constraints[0]); self::assertIsArray($constraints[0]->constraints); diff --git a/tests/Validator/Constraints/FloatScaleValidatorTest.php b/tests/Validator/Constraints/FloatScaleValidatorTest.php index 457bf65..3b405b1 100644 --- a/tests/Validator/Constraints/FloatScaleValidatorTest.php +++ b/tests/Validator/Constraints/FloatScaleValidatorTest.php @@ -17,7 +17,7 @@ class FloatScaleValidatorTest extends ConstraintValidatorTestCase { protected function getConstraint(): Constraint { - return new FloatScale(['scale' => 2]); + return new FloatScale(scale: 2); } public function createValidator(): ConstraintValidator diff --git a/tests/Validator/Constraints/MoneyValidatorTest.php b/tests/Validator/Constraints/MoneyValidatorTest.php index 0ebf047..ed9c21a 100644 --- a/tests/Validator/Constraints/MoneyValidatorTest.php +++ b/tests/Validator/Constraints/MoneyValidatorTest.php @@ -21,7 +21,7 @@ class MoneyValidatorTest extends ConstraintValidatorTestCase { protected function getConstraint(): Constraint { - return new Money(['min' => 0.0, 'max' => 90.0]); + return new Money(min: 0.0, max: 90.0); } public function createValidator(): ConstraintValidator diff --git a/tests/Validator/Constraints/PostalValidatorTest.php b/tests/Validator/Constraints/PostalValidatorTest.php index 98b05e1..eeaddf5 100644 --- a/tests/Validator/Constraints/PostalValidatorTest.php +++ b/tests/Validator/Constraints/PostalValidatorTest.php @@ -10,7 +10,6 @@ use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\ConstraintValidator; use Symfony\Component\Validator\Exception\ConstraintDefinitionException; -use Symfony\Component\Validator\Exception\MissingOptionsException; /** * @extends ConstraintValidatorTestCase @@ -19,9 +18,7 @@ class PostalValidatorTest extends ConstraintValidatorTestCase { protected function getConstraint(): Constraint { - return new Postal([ - 'countryPropertyPath' => 'country', - ]); + return new Postal(countryPropertyPath: 'country'); } public function createValidator(): ConstraintValidator @@ -29,12 +26,6 @@ public function createValidator(): ConstraintValidator return new PostalValidator(); } - public function testMissingPropertyPath(): void - { - $this->expectException(MissingOptionsException::class); - new Postal([]); - } - public function testMissingObject(): void { $this->setObject(null); diff --git a/tests/Validator/ConstraintsSetProvider/Field/EmailProviderTest.php b/tests/Validator/ConstraintsSetProvider/Field/EmailProviderTest.php index cac99d0..b7203a9 100644 --- a/tests/Validator/ConstraintsSetProvider/Field/EmailProviderTest.php +++ b/tests/Validator/ConstraintsSetProvider/Field/EmailProviderTest.php @@ -21,12 +21,12 @@ public static function getConstraintsForTypeProvider(): iterable { yield [ ['type' => 'email', 'length' => 10], - [new Email(), new Length(['max' => 10])], + [new Email(), new Length(max: 10)], ]; yield [ ['type' => 'email'], - [new Email(), new Length(['max' => 255])], + [new Email(), new Length(max: 255)], ]; } } diff --git a/tests/Validator/ConstraintsSetProvider/Field/IpProviderTest.php b/tests/Validator/ConstraintsSetProvider/Field/IpProviderTest.php index 102898f..e87ba0e 100644 --- a/tests/Validator/ConstraintsSetProvider/Field/IpProviderTest.php +++ b/tests/Validator/ConstraintsSetProvider/Field/IpProviderTest.php @@ -20,7 +20,7 @@ public static function getConstraintsForTypeProvider(): iterable { yield [ ['type' => 'ip'], - [new Ip(['version' => 'all'])], + [new Ip(version: 'all')], ]; } } diff --git a/tests/Validator/ConstraintsSetProvider/Field/LocaleProviderTest.php b/tests/Validator/ConstraintsSetProvider/Field/LocaleProviderTest.php index 0d57a9e..ad80485 100644 --- a/tests/Validator/ConstraintsSetProvider/Field/LocaleProviderTest.php +++ b/tests/Validator/ConstraintsSetProvider/Field/LocaleProviderTest.php @@ -20,7 +20,7 @@ public static function getConstraintsForTypeProvider(): iterable { yield [ ['type' => 'locale'], - [new Locale(['canonicalize' => true])], + [new Locale(canonicalize: true)], ]; } } diff --git a/tests/Validator/ConstraintsSetProvider/Field/StringProviderTest.php b/tests/Validator/ConstraintsSetProvider/Field/StringProviderTest.php index 1cdf5b3..516f866 100644 --- a/tests/Validator/ConstraintsSetProvider/Field/StringProviderTest.php +++ b/tests/Validator/ConstraintsSetProvider/Field/StringProviderTest.php @@ -21,7 +21,7 @@ public static function getConstraintsForTypeProvider(): iterable { yield [ ['type' => 'string'], - [new Length(['max' => 255])], + [new Length(max: 255)], ]; yield [ @@ -29,7 +29,7 @@ public static function getConstraintsForTypeProvider(): iterable 'type' => 'string', 'length' => 10, ], - [new Length(['max' => 10])], + [new Length(max: 10)], ]; yield [ @@ -38,8 +38,8 @@ public static function getConstraintsForTypeProvider(): iterable 'nullable' => true, ], [ - new Length(['max' => 255]), - new NotBlank(['allowNull' => true]), + new Length(max: 255), + new NotBlank(allowNull: true), ], ]; } diff --git a/tests/Validator/ConstraintsSetProvider/Field/TextProviderTest.php b/tests/Validator/ConstraintsSetProvider/Field/TextProviderTest.php index 1ba90f2..4683486 100644 --- a/tests/Validator/ConstraintsSetProvider/Field/TextProviderTest.php +++ b/tests/Validator/ConstraintsSetProvider/Field/TextProviderTest.php @@ -20,12 +20,12 @@ public static function getConstraintsForTypeProvider(): iterable { yield [ ['type' => 'text'], - [new Length(['max' => 65535, 'charset' => '8bit'])], + [new Length(max: 65535, charset: '8bit')], ]; yield [ ['type' => 'text', 'length' => 1000], - [new Length(['max' => 1000, 'charset' => '8bit'])], + [new Length(max: 1000, charset: '8bit')], ]; } }