feature #31528 [Validator] Add a Length::$allowEmptyString option to reject empty strings (ogizanagi)

fabpot · fabpot · commit ce1f7535a83e · 2019-07-03T15:15:57.000+02:00
This PR was merged into the 4.4 branch. Discussion ---------- [Validator] Add a Length::$allowEmptyString option to reject empty strings | Q | A | ------------- | --- | Branch? | 4.4  | Bug fix? | no | New feature? | yes  | BC breaks? | no  | Deprecations? | yes  | Tests pass? | yes  | Fixed tickets | N/A  | License | MIT | Doc PR | Todo (change the warning on top of https://symfony.com/doc/current/reference/constraints/Length.html) which defaults to `true` in 4.4 but will trigger a deprecation if not set explicitly in order to make the default `false` in 5.0. While it could be solved now thanks to #29641 by using both `@Length(min=1)` & `@NotBlank(allowNull=true)` constraints, as expressed in symfony/symfony#27876 (comment) and following comments, the `@Length(min=1)` behavior doesn't match our expectations when reading it: it feels logical to invalidate empty strings, but it actually doesn't. Hence the proposal of making the behavior of rejecting empty strings the default in 5.0. In my opinion, the flag could even be removed later. Commits ------- e113e7f812 [Validator] Add a Length::$allowEmptyString option to reject empty strings
diff --git a/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php b/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php
@@ -57,13 +57,15 @@ public function testValidConstraint()
 
     public function testGroupSequenceWithConstraintsOption()
     {
+        $allowEmptyString = property_exists(Length::class, 'allowEmptyString') ? ['allowEmptyString' => true] : [];
+
         $form = Forms::createFormFactoryBuilder()
             ->addExtension(new ValidatorExtension(Validation::createValidator()))
             ->getFormFactory()
             ->create(FormTypeTest::TESTED_TYPE, null, (['validation_groups' => new GroupSequence(['First', 'Second'])]))
             ->add('field', TextTypeTest::TESTED_TYPE, [
                 'constraints' => [
-                    new Length(['min' => 10, 'groups' => ['First']]),
+                    new Length(['min' => 10, 'groups' => ['First']] + $allowEmptyString),
                     new Email(['groups' => ['Second']]),
                 ],
             ])
diff --git a/Tests/Extension/Validator/ValidatorTypeGuesserTest.php b/Tests/Extension/Validator/ValidatorTypeGuesserTest.php
@@ -61,11 +61,13 @@ protected function setUp()
 
     public function guessRequiredProvider()
     {
+        $allowEmptyString = property_exists(Length::class, 'allowEmptyString') ? ['allowEmptyString' => true] : [];
+
         return [
             [new NotNull(), new ValueGuess(true, Guess::HIGH_CONFIDENCE)],
             [new NotBlank(), new ValueGuess(true, Guess::HIGH_CONFIDENCE)],
             [new IsTrue(), new ValueGuess(true, Guess::HIGH_CONFIDENCE)],
-            [new Length(10), new ValueGuess(false, Guess::LOW_CONFIDENCE)],
+            [new Length(['min' => 10, 'max' => 10] + $allowEmptyString), new ValueGuess(false, Guess::LOW_CONFIDENCE)],
             [new Range(['min' => 1, 'max' => 20]), new ValueGuess(false, Guess::LOW_CONFIDENCE)],
         ];
     }
@@ -101,7 +103,9 @@ public function testGuessMaxLengthForConstraintWithMaxValue()
 
     public function testGuessMaxLengthForConstraintWithMinValue()
     {
-        $constraint = new Length(['min' => '2']);
+        $allowEmptyString = property_exists(Length::class, 'allowEmptyString') ? ['allowEmptyString' => true] : [];
+
+        $constraint = new Length(['min' => '2'] + $allowEmptyString);
 
         $result = $this->guesser->guessMaxLengthForConstraint($constraint);
         $this->assertNull($result);
