feature #20361 [Config] Enable cannotBeEmpty along with requiresAtLeastOneElement (ro0NL)

fabpot · fabpot · commit 73d3d5a91f99 · 2017-08-05T19:34:46.000+02:00
This PR was squashed before being merged into the 3.4 branch (closes #20361). Discussion ---------- [Config] Enable cannotBeEmpty along with requiresAtLeastOneElement | Q | A | | --- | --- | | Branch? | "master" | | Bug fix? | no | | New feature? | yes | | BC breaks? | no | | Deprecations? | no | | Tests pass? | yes | Fixed tickets | #20356 | | License | MIT | | Doc PR | reference to the documentation PR, if any | As @vudaltsov mentioned, we ignore any calls to `ArrayNodeDefinition::cannotBeEmpty`, which can lead to unexpected behavior. Imo. all subclasses should follow the base API. Commits ------- d40e7e4 [Config] Enable cannotBeEmpty along with requiresAtLeastOneElement
diff --git a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php
@@ -428,7 +428,7 @@ protected function createNode()
                 $node->setKeyAttribute($this->key, $this->removeKeyItem);
             }
 
-            if (true === $this->atLeastOne) {
+            if (true === $this->atLeastOne || false === $this->allowEmptyValue) {
                 $node->setMinNumberOfElements(1);
             }
 
@@ -490,6 +490,12 @@ protected function validateConcreteNode(ArrayNode $node)
             );
         }
 
+        if (false === $this->allowEmptyValue) {
+            throw new InvalidDefinitionException(
+                sprintf('->cannotBeEmpty() is not applicable to concrete nodes at path "%s"', $path)
+            );
+        }
+
         if (true === $this->atLeastOne) {
             throw new InvalidDefinitionException(
                 sprintf('->requiresAtLeastOneElement() is not applicable to concrete nodes at path "%s"', $path)
diff --git a/src/Symfony/Component/Config/Tests/Definition/Builder/ArrayNodeDefinitionTest.php b/src/Symfony/Component/Config/Tests/Definition/Builder/ArrayNodeDefinitionTest.php
@@ -54,6 +54,7 @@ public function providePrototypeNodeSpecificCalls()
             array('defaultValue', array(array())),
             array('addDefaultChildrenIfNoneSet', array()),
             array('requiresAtLeastOneElement', array()),
+            array('cannotBeEmpty', array()),
             array('useAttributeAsKey', array('foo')),
         );
     }
@@ -285,6 +286,32 @@ public function getEnableableNodeFixtures()
         );
     }
 
+    public function testRequiresAtLeastOneElement()
+    {
+        $node = new ArrayNodeDefinition('root');
+        $node
+            ->requiresAtLeastOneElement()
+            ->integerPrototype();
+
+        $node->getNode()->finalize(array(1));
+
+        $this->addToAssertionCount(1);
+    }
+
+    /**
+     * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
+     * @expectedExceptionMessage The path "root" should have at least 1 element(s) defined.
+     */
+    public function testCannotBeEmpty()
+    {
+        $node = new ArrayNodeDefinition('root');
+        $node
+            ->cannotBeEmpty()
+            ->integerPrototype();
+
+        $node->getNode()->finalize(array());
+    }
+
     protected function getField($object, $field)
     {
         $reflection = new \ReflectionProperty($object, $field);

Original file line number	Diff line number	Diff line change
`@@ -428,7 +428,7 @@ protected function createNode()`
`428`	`428`	`$node->setKeyAttribute($this->key, $this->removeKeyItem);`
`429`	`429`	`}`
`430`	`430`
`431`		`- if (true === $this->atLeastOne) {`
	`431`	`+ if (true === $this->atLeastOne \|\| false === $this->allowEmptyValue) {`
`432`	`432`	`$node->setMinNumberOfElements(1);`
`433`	`433`	`}`
`434`	`434`
`@@ -490,6 +490,12 @@ protected function validateConcreteNode(ArrayNode $node)`
`490`	`490`	`);`
`491`	`491`	`}`
`492`	`492`
	`493`	`+ if (false === $this->allowEmptyValue) {`
	`494`	`+ throw new InvalidDefinitionException(`
	`495`	`+ sprintf('->cannotBeEmpty() is not applicable to concrete nodes at path "%s"', $path)`
	`496`	`+ );`
	`497`	`+ }`
	`498`	`+`
`493`	`499`	`if (true === $this->atLeastOne) {`
`494`	`500`	`throw new InvalidDefinitionException(`
`495`	`501`	`sprintf('->requiresAtLeastOneElement() is not applicable to concrete nodes at path "%s"', $path)`