bug #34081 [Validator] Set Length::$allowEmptyString to false when a …

…NotBlank contraint is defined (nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[Validator] Set Length::$allowEmptyString to false when a NotBlank contraint is defined

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Since #31528, we are told to do this kind of changes to our entities:
```diff
     /**
      * @Assert\NotBlank()
-     * @Assert\Length(min="10")
+     * @Assert\Length(min="10", allowEmptyString=false)
      */
     public $description;
```

But the `NotBlank` already says it - this is just boilerplate. More critically, this also means we cannot write annotations that are compatible with 3.4, 4.4 and 5.0 at the same time, making FC/BC hard.

By setting `Length::$allowEmptyString` to `false` when a `NotBlank` contraint is defined, we fix both issues.

/cc @ogizanagi

Commits
-------

840f7e7 [Validator] Set Length::$allowEmptyString to false when a NotBlank contraint is defined

src/Symfony/Component/Validator/Constraints/Length.php

@@ -57,13 +57,6 @@ public function __construct($options = null)
 
         parent::__construct($options);
 
-        if (null === $this->allowEmptyString) {
-            $this->allowEmptyString = true;
-            if (null !== $this->min) {
-                @trigger_error(sprintf('Using the "%s" constraint with the "min" option without setting the "allowEmptyString" one is deprecated and defaults to true. In 5.0, it will become optional and default to false.', self::class), E_USER_DEPRECATED);
-            }
-        }
-
         if (null === $this->min && null === $this->max) {
             throw new MissingOptionsException(sprintf('Either option "min" or "max" must be given for constraint %s', __CLASS__), ['min', 'max']);
         }

src/Symfony/Component/Validator/Constraints/LengthValidator.php

@@ -30,7 +30,11 @@ public function validate($value, Constraint $constraint)
             throw new UnexpectedTypeException($constraint, __NAMESPACE__.'\Length');
         }
 
-        if (null === $value || ('' === $value && $constraint->allowEmptyString)) {
+        if (null !== $constraint->min && null === $constraint->allowEmptyString) {
+            @trigger_error(sprintf('Using the "%s" constraint with the "min" option without setting the "allowEmptyString" one is deprecated and defaults to true. In 5.0, it will become optional and default to false.', Length::class), E_USER_DEPRECATED);
+        }
+
+        if (null === $value || ('' === $value && ($constraint->allowEmptyString ?? true))) {
             return;
         }
 

src/Symfony/Component/Validator/Mapping/GenericMetadata.php

@@ -12,6 +12,8 @@
 namespace Symfony\Component\Validator\Mapping;
 
 use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\Constraints\Length;
+use Symfony\Component\Validator\Constraints\NotBlank;
 use Symfony\Component\Validator\Constraints\Traverse;
 use Symfony\Component\Validator\Constraints\Valid;
 use Symfony\Component\Validator\Exception\ConstraintDefinitionException;
@@ -167,6 +169,8 @@ public function addConstraints(array $constraints)
      */
     public function getConstraints()
     {
+        $this->configureLengthConstraints($this->constraints);
+
         return $this->constraints;
     }
 
@@ -187,9 +191,10 @@ public function hasConstraints()
      */
     public function findConstraints($group)
     {
-        return isset($this->constraintsByGroup[$group])
-            ? $this->constraintsByGroup[$group]
-            : [];
+        $constraints = $this->constraintsByGroup[$group] ?? [];
+        $this->configureLengthConstraints($constraints);
+
+        return $constraints;
     }
 
     /**
@@ -207,4 +212,26 @@ public function getTraversalStrategy()
     {
         return $this->traversalStrategy;
     }
+
+    private function configureLengthConstraints(array $constraints): void
+    {
+        $allowEmptyString = true;
+
+        foreach ($constraints as $constraint) {
+            if ($constraint instanceof NotBlank) {
+                $allowEmptyString = false;
+                break;
+            }
+        }
+
+        if ($allowEmptyString) {
+            return;
+        }
+
+        foreach ($constraints as $constraint) {
+            if ($constraint instanceof Length && null === $constraint->allowEmptyString) {
+                $constraint->allowEmptyString = false;
+            }
+        }
+    }
 }