From c9d8b15dcac5ce02bd949d783b9f26003752dbe6 Mon Sep 17 00:00:00 2001 From: Thomas Calvet Date: Thu, 12 Jan 2023 09:19:38 +0100 Subject: [PATCH] [Config] Do not array_unique EnumNode values --- .../Config/Builder/ConfigBuilderGenerator.php | 2 +- .../Definition/Builder/EnumNodeDefinition.php | 2 -- .../Definition/Dumper/XmlReferenceDumper.php | 4 ++-- .../Definition/Dumper/YamlReferenceDumper.php | 2 +- .../Component/Config/Definition/EnumNode.php | 7 ++++++- .../Builder/EnumNodeDefinitionTest.php | 17 ++++++++--------- .../Config/Tests/Definition/EnumNodeTest.php | 16 ++++++++++++++++ 7 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/Symfony/Component/Config/Builder/ConfigBuilderGenerator.php b/src/Symfony/Component/Config/Builder/ConfigBuilderGenerator.php index 44ad6ca894f75..d7b8dd04432ab 100644 --- a/src/Symfony/Component/Config/Builder/ConfigBuilderGenerator.php +++ b/src/Symfony/Component/Config/Builder/ConfigBuilderGenerator.php @@ -426,7 +426,7 @@ private function getComment(BaseNode $node): string } if ($node instanceof EnumNode) { - $comment .= sprintf(' * @param ParamConfigurator|%s $value', implode('|', array_map(fn ($a) => var_export($a, true), $node->getValues())))."\n"; + $comment .= sprintf(' * @param ParamConfigurator|%s $value', implode('|', array_unique(array_map(fn ($a) => var_export($a, true), $node->getValues()))))."\n"; } else { $parameterTypes = $this->getParameterTypes($node); $comment .= ' * @param ParamConfigurator|'.implode('|', $parameterTypes).' $value'."\n"; diff --git a/src/Symfony/Component/Config/Definition/Builder/EnumNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/EnumNodeDefinition.php index 6df62d30ab8b3..99f31812331c1 100644 --- a/src/Symfony/Component/Config/Definition/Builder/EnumNodeDefinition.php +++ b/src/Symfony/Component/Config/Definition/Builder/EnumNodeDefinition.php @@ -27,8 +27,6 @@ class EnumNodeDefinition extends ScalarNodeDefinition */ public function values(array $values): static { - $values = array_unique($values); - if (!$values) { throw new \InvalidArgumentException('->values() must be called with at least one value.'); } diff --git a/src/Symfony/Component/Config/Definition/Dumper/XmlReferenceDumper.php b/src/Symfony/Component/Config/Definition/Dumper/XmlReferenceDumper.php index 563c29d8ed598..eb8fce4385e6b 100644 --- a/src/Symfony/Component/Config/Definition/Dumper/XmlReferenceDumper.php +++ b/src/Symfony/Component/Config/Definition/Dumper/XmlReferenceDumper.php @@ -107,7 +107,7 @@ private function writeNode(NodeInterface $node, int $depth = 0, bool $root = fal FloatNode::class, IntegerNode::class => 'numeric value', BooleanNode::class => 'true|false', - EnumNode::class => implode('|', array_map('json_encode', $prototype->getValues())), + EnumNode::class => implode('|', array_unique(array_map('json_encode', $prototype->getValues()))), default => 'value', }; } @@ -149,7 +149,7 @@ private function writeNode(NodeInterface $node, int $depth = 0, bool $root = fal } if ($child instanceof EnumNode) { - $comments[] = 'One of '.implode('; ', array_map('json_encode', $child->getValues())); + $comments[] = 'One of '.implode('; ', array_unique(array_map('json_encode', $child->getValues()))); } if (\count($comments)) { diff --git a/src/Symfony/Component/Config/Definition/Dumper/YamlReferenceDumper.php b/src/Symfony/Component/Config/Definition/Dumper/YamlReferenceDumper.php index 606c3edaad58f..04cb38c20b451 100644 --- a/src/Symfony/Component/Config/Definition/Dumper/YamlReferenceDumper.php +++ b/src/Symfony/Component/Config/Definition/Dumper/YamlReferenceDumper.php @@ -98,7 +98,7 @@ private function writeNode(NodeInterface $node, NodeInterface $parentNode = null } } } elseif ($node instanceof EnumNode) { - $comments[] = 'One of '.implode('; ', array_map('json_encode', $node->getValues())); + $comments[] = 'One of '.implode('; ', array_unique(array_map('json_encode', $node->getValues()))); $default = $node->hasDefaultValue() ? Inline::dump($node->getDefaultValue()) : '~'; } elseif (VariableNode::class === $node::class && \is_array($example)) { // If there is an array example, we are sure we dont need to print a default value diff --git a/src/Symfony/Component/Config/Definition/EnumNode.php b/src/Symfony/Component/Config/Definition/EnumNode.php index 76d8d12ac4fdb..ebb78daa28508 100644 --- a/src/Symfony/Component/Config/Definition/EnumNode.php +++ b/src/Symfony/Component/Config/Definition/EnumNode.php @@ -24,11 +24,16 @@ class EnumNode extends ScalarNode public function __construct(?string $name, NodeInterface $parent = null, array $values = [], string $pathSeparator = BaseNode::DEFAULT_PATH_SEPARATOR) { - $values = array_unique($values); if (!$values) { throw new \InvalidArgumentException('$values must contain at least one element.'); } + foreach ($values as $value) { + if (null !== $value && !\is_scalar($value)) { + throw new \InvalidArgumentException(sprintf('"%s" only supports scalar or null values, "%s" given.', __CLASS__, get_debug_type($value))); + } + } + parent::__construct($name, $parent, $pathSeparator); $this->values = $values; } diff --git a/src/Symfony/Component/Config/Tests/Definition/Builder/EnumNodeDefinitionTest.php b/src/Symfony/Component/Config/Tests/Definition/Builder/EnumNodeDefinitionTest.php index 06a227ca3cb38..c75841afe9439 100644 --- a/src/Symfony/Component/Config/Tests/Definition/Builder/EnumNodeDefinitionTest.php +++ b/src/Symfony/Component/Config/Tests/Definition/Builder/EnumNodeDefinitionTest.php @@ -25,15 +25,6 @@ public function testWithOneValue() $this->assertEquals(['foo'], $node->getValues()); } - public function testWithOneDistinctValue() - { - $def = new EnumNodeDefinition('foo'); - $def->values(['foo', 'foo']); - - $node = $def->getNode(); - $this->assertEquals(['foo'], $node->getValues()); - } - public function testNoValuesPassed() { $this->expectException(\RuntimeException::class); @@ -73,4 +64,12 @@ public function testSetDeprecated() $this->assertSame('vendor/package', $deprecation['package']); $this->assertSame('1.1', $deprecation['version']); } + + public function testSameStringCoercedValuesAreDifferent() + { + $def = new EnumNodeDefinition('ccc'); + $def->values(['', false, null]); + + $this->assertSame(['', false, null], $def->getNode()->getValues()); + } } diff --git a/src/Symfony/Component/Config/Tests/Definition/EnumNodeTest.php b/src/Symfony/Component/Config/Tests/Definition/EnumNodeTest.php index b728dec5b1c78..86bc045eeca0f 100644 --- a/src/Symfony/Component/Config/Tests/Definition/EnumNodeTest.php +++ b/src/Symfony/Component/Config/Tests/Definition/EnumNodeTest.php @@ -71,4 +71,20 @@ public function testWithPlaceHolderWithInvalidValue() $this->expectExceptionMessage('The value "foo" is not allowed for path "cookie_samesite". Permissible values: "lax", "strict", "none"'); $node->finalize('custom'); } + + public function testSameStringCoercedValuesAreDifferent() + { + $node = new EnumNode('ccc', null, ['', false, null]); + $this->assertSame('', $node->finalize('')); + $this->assertFalse($node->finalize(false)); + $this->assertNull($node->finalize(null)); + } + + public function testNonScalarOrNullValueThrows() + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('"Symfony\Component\Config\Definition\EnumNode" only supports scalar or null values, "stdClass" given.'); + + new EnumNode('ccc', null, [new \stdClass()]); + } }