bug #54758 [Validator] handle edge cases when constructing constraint…

…s with named arguments (xabbuh) This PR was merged into the 6.4 branch. Discussion ---------- [Validator] handle edge cases when constructing constraints with named arguments | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | | License | MIT spotted while working on #54744 Commits ------- 93eb8a0 handle edge cases when constructing constraints with named arguments
symfony · May 1, 2024 · 0c764e7 · 0c764e7
2 parents 2722fb5 + 93eb8a0
commit 0c764e7
Show file tree

Hide file tree

Showing 9 changed files with 119 additions and 1 deletion.
diff --git a/src/Symfony/Component/Validator/Mapping/Loader/AbstractLoader.php b/src/Symfony/Component/Validator/Mapping/Loader/AbstractLoader.php
@@ -87,6 +87,18 @@ protected function newConstraint(string $name, mixed $options = null): Constrain
         }
 
         if ($this->namedArgumentsCache[$className] ??= (bool) (new \ReflectionMethod($className, '__construct'))->getAttributes(HasNamedArguments::class)) {
+            if (null === $options) {
+                return new $className();
+            }
+
+            if (!\is_array($options)) {
+                return new $className($options);
+            }
+
+            if (1 === \count($options) && isset($options['value'])) {
+                return new $className($options['value']);
+            }
+
             return new $className(...$options);
         }
 

diff --git a/src/Symfony/Component/Validator/Mapping/Loader/XmlFileLoader.php b/src/Symfony/Component/Validator/Mapping/Loader/XmlFileLoader.php
@@ -80,7 +80,9 @@ protected function parseConstraints(\SimpleXMLElement $nodes): array
         foreach ($nodes as $node) {
             if (\count($node) > 0) {
                 if (\count($node->value) > 0) {
-                    $options = $this->parseValues($node->value);
+                    $options = [
+                        'value' => $this->parseValues($node->value),
+                    ];
                 } elseif (\count($node->constraint) > 0) {
                     $options = $this->parseConstraints($node->constraint);
                 } elseif (\count($node->option) > 0) {
@@ -94,6 +96,10 @@ protected function parseConstraints(\SimpleXMLElement $nodes): array
                 $options = null;
             }
 
+            if (isset($options['groups']) && !\is_array($options['groups'])) {
+                $options['groups'] = (array) $options['groups'];
+            }
+
             $constraints[] = $this->newConstraint((string) $node['name'], $options);
         }
 

diff --git a/src/Symfony/Component/Validator/Mapping/Loader/YamlFileLoader.php b/src/Symfony/Component/Validator/Mapping/Loader/YamlFileLoader.php
@@ -91,6 +91,12 @@ protected function parseNodes(array $nodes): array
                     $options = $this->parseNodes($options);
                 }
 
+                if (null !== $options && (!\is_array($options) || array_is_list($options))) {
+                    $options = [
+                        'value' => $options,
+                    ];
+                }
+
                 $values[] = $this->newConstraint(key($childNodes), $options);
             } else {
                 if (\is_array($childNodes)) {

diff --git a/...ymfony/Component/Validator/Tests/Mapping/Loader/Fixtures/ConstraintWithNamedArguments.php b/...ymfony/Component/Validator/Tests/Mapping/Loader/Fixtures/ConstraintWithNamedArguments.php
@@ -0,0 +1,33 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Validator\Tests\Mapping\Loader\Fixtures;
+
+use Symfony\Component\Validator\Attribute\HasNamedArguments;
+use Symfony\Component\Validator\Constraint;
+
+class ConstraintWithNamedArguments extends Constraint
+{
+    public $choices;
+
+    #[HasNamedArguments]
+    public function __construct(array|string|null $choices = [], ?array $groups = null)
+    {
+        parent::__construct([], $groups);
+
+        $this->choices = $choices;
+    }
+
+    public function getTargets(): string
+    {
+        return self::CLASS_CONSTRAINT;
+    }
+}
diff --git a/...nent/Validator/Tests/Mapping/Loader/Fixtures/ConstraintWithoutValueWithNamedArguments.php b/...nent/Validator/Tests/Mapping/Loader/Fixtures/ConstraintWithoutValueWithNamedArguments.php
@@ -0,0 +1,29 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Validator\Tests\Mapping\Loader\Fixtures;
+
+use Symfony\Component\Validator\Attribute\HasNamedArguments;
+use Symfony\Component\Validator\Constraint;
+
+class ConstraintWithoutValueWithNamedArguments extends Constraint
+{
+    #[HasNamedArguments]
+    public function __construct(?array $groups = null)
+    {
+        parent::__construct([], $groups);
+    }
+
+    public function getTargets(): string
+    {
+        return self::CLASS_CONSTRAINT;
+    }
+}
diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/Validator/Tests/Mapping/Loader/XmlFileLoaderTest.php
@@ -32,6 +32,8 @@
 use Symfony\Component\Validator\Tests\Fixtures\Entity_81;
 use Symfony\Component\Validator\Tests\Fixtures\NestedAttribute\Entity;
 use Symfony\Component\Validator\Tests\Fixtures\NestedAttribute\GroupSequenceProviderEntity;
+use Symfony\Component\Validator\Tests\Mapping\Loader\Fixtures\ConstraintWithNamedArguments;
+use Symfony\Component\Validator\Tests\Mapping\Loader\Fixtures\ConstraintWithoutValueWithNamedArguments;
 
 class XmlFileLoaderTest extends TestCase
 {
@@ -66,6 +68,9 @@ public function testLoadClassMetadata()
         $expected->addConstraint(new Callback('validateMeStatic'));
         $expected->addConstraint(new Callback(['Symfony\Component\Validator\Tests\Fixtures\CallbackClass', 'callback']));
         $expected->addConstraint(new Traverse(false));
+        $expected->addConstraint(new ConstraintWithNamedArguments('foo'));
+        $expected->addConstraint(new ConstraintWithNamedArguments(['foo', 'bar']));
+        $expected->addConstraint(new ConstraintWithoutValueWithNamedArguments(['foo']));
         $expected->addPropertyConstraint('firstName', new NotNull());
         $expected->addPropertyConstraint('firstName', new Range(['min' => 3]));
         $expected->addPropertyConstraint('firstName', new Choice(['A', 'B']));

diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/Validator/Tests/Mapping/Loader/YamlFileLoaderTest.php
@@ -29,6 +29,8 @@
 use Symfony\Component\Validator\Tests\Fixtures\Entity_81;
 use Symfony\Component\Validator\Tests\Fixtures\NestedAttribute\Entity;
 use Symfony\Component\Validator\Tests\Fixtures\NestedAttribute\GroupSequenceProviderEntity;
+use Symfony\Component\Validator\Tests\Mapping\Loader\Fixtures\ConstraintWithNamedArguments;
+use Symfony\Component\Validator\Tests\Mapping\Loader\Fixtures\ConstraintWithoutValueWithNamedArguments;
 
 class YamlFileLoaderTest extends TestCase
 {
@@ -109,6 +111,9 @@ public function testLoadClassMetadata()
         $expected->addConstraint(new Callback('validateMe'));
         $expected->addConstraint(new Callback('validateMeStatic'));
         $expected->addConstraint(new Callback(['Symfony\Component\Validator\Tests\Fixtures\CallbackClass', 'callback']));
+        $expected->addConstraint(new ConstraintWithoutValueWithNamedArguments());
+        $expected->addConstraint(new ConstraintWithNamedArguments('foo'));
+        $expected->addConstraint(new ConstraintWithNamedArguments(['foo', 'bar']));
         $expected->addPropertyConstraint('firstName', new NotNull());
         $expected->addPropertyConstraint('firstName', new Range(['min' => 3]));
         $expected->addPropertyConstraint('firstName', new Choice(['A', 'B']));

diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Loader/constraint-mapping.xml b/src/Symfony/Component/Validator/Tests/Mapping/Loader/constraint-mapping.xml
@@ -36,6 +36,22 @@
         false
     </constraint>
 
+    <!-- Constraint with named arguments support with scalar value -->
+    <constraint name="Symfony\Component\Validator\Tests\Mapping\Loader\Fixtures\ConstraintWithNamedArguments">
+      foo
+    </constraint>
+
+    <!-- Constraint with named arguments support with array value -->
+    <constraint name="Symfony\Component\Validator\Tests\Mapping\Loader\Fixtures\ConstraintWithNamedArguments">
+      <value>foo</value>
+      <value>bar</value>
+    </constraint>
+
+    <!-- Constraint with named arguments support with exactly one group -->
+    <constraint name="Symfony\Component\Validator\Tests\Mapping\Loader\Fixtures\ConstraintWithoutValueWithNamedArguments">
+      <option name="groups">foo</option>
+    </constraint>
+
     <!-- PROPERTY CONSTRAINTS -->
 
     <property name="firstName">

diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Loader/constraint-mapping.yml b/src/Symfony/Component/Validator/Tests/Mapping/Loader/constraint-mapping.yml
@@ -15,6 +15,12 @@ Symfony\Component\Validator\Tests\Fixtures\NestedAttribute\Entity:
     - Callback: validateMe
     - Callback: validateMeStatic
     - Callback: [Symfony\Component\Validator\Tests\Fixtures\CallbackClass, callback]
+    # Constraint with named arguments support without value
+    - Symfony\Component\Validator\Tests\Mapping\Loader\Fixtures\ConstraintWithoutValueWithNamedArguments: ~
+    # Constraint with named arguments support with scalar value
+    - Symfony\Component\Validator\Tests\Mapping\Loader\Fixtures\ConstraintWithNamedArguments: foo
+    # Constraint with named arguments support with array value
+    - Symfony\Component\Validator\Tests\Mapping\Loader\Fixtures\ConstraintWithNamedArguments: [foo, bar]
 
   properties:
     firstName: