Add feature allowing change of Path Separator

bburnichon · nicolas-grekas · commit 15e05e1e1789 · 2018-01-21T16:11:55.000+01:00
diff --git a/UPGRADE-4.1.md b/UPGRADE-4.1.md
@@ -1,6 +1,12 @@
 UPGRADE FROM 4.0 to 4.1
 =======================
 
+Config
+------
+
+ * Implementing `ParentNodeDefinitionInterface` without the `getChildNodeDefinitions()` method
+   is deprecated and will be unsupported in 5.0.
+
 EventDispatcher
 ---------------
 
diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md
@@ -1,6 +1,11 @@
 UPGRADE FROM 4.x to 5.0
 =======================
 
+Config
+------
+
+ * Added the `getChildNodeDefinitions()` method to `ParentNodeDefinitionInterface`.
+
 EventDispatcher
 ---------------
 
diff --git a/src/Symfony/Component/Config/CHANGELOG.md b/src/Symfony/Component/Config/CHANGELOG.md
@@ -1,6 +1,12 @@
 CHANGELOG
 =========
 
+4.1.0
+-----
+
+ * added `setPathSeparator` method to `NodeBuilder` class
+ * added third `$pathSeparator` constructor argument to `BaseNode`
+
 4.0.0
 -----
 
diff --git a/src/Symfony/Component/Config/Definition/BaseNode.php b/src/Symfony/Component/Config/Definition/BaseNode.php
@@ -23,6 +23,8 @@
  */
 abstract class BaseNode implements NodeInterface
 {
+    const DEFAULT_PATH_SEPARATOR = '.';
+
     protected $name;
     protected $parent;
     protected $normalizationClosures = array();
@@ -32,18 +34,20 @@ abstract class BaseNode implements NodeInterface
     protected $deprecationMessage = null;
     protected $equivalentValues = array();
     protected $attributes = array();
+    protected $pathSeparator;
 
     /**
      * @throws \InvalidArgumentException if the name contains a period
      */
-    public function __construct(?string $name, NodeInterface $parent = null)
+    public function __construct(?string $name, NodeInterface $parent = null, string $pathSeparator = self::DEFAULT_PATH_SEPARATOR)
     {
-        if (false !== strpos($name, '.')) {
-            throw new \InvalidArgumentException('The name must not contain ".".');
+        if (false !== strpos($name, $pathSeparator)) {
+            throw new \InvalidArgumentException('The name must not contain "'.$pathSeparator.'".');
         }
 
         $this->name = $name;
         $this->parent = $parent;
+        $this->pathSeparator = $pathSeparator;
     }
 
     public function setAttribute($key, $value)
@@ -230,13 +234,11 @@ public function getName()
      */
     public function getPath()
     {
-        $path = $this->name;
-
         if (null !== $this->parent) {
-            $path = $this->parent->getPath().'.'.$path;
+            return $this->parent->getPath().$this->pathSeparator.$this->name;
         }
 
-        return $path;
+        return $this->name;
     }
 
     /**
diff --git a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php
@@ -407,7 +407,7 @@ protected function getNodeBuilder()
     protected function createNode()
     {
         if (null === $this->prototype) {
-            $node = new ArrayNode($this->name, $this->parent);
+            $node = new ArrayNode($this->name, $this->parent, $this->pathSeparator);
 
             $this->validateConcreteNode($node);
 
@@ -418,7 +418,7 @@ protected function createNode()
                 $node->addChild($child->getNode());
             }
         } else {
-            $node = new PrototypedArrayNode($this->name, $this->parent);
+            $node = new PrototypedArrayNode($this->name, $this->parent, $this->pathSeparator);
 
             $this->validatePrototypeNode($node);
 
@@ -545,4 +545,12 @@ protected function validatePrototypeNode(PrototypedArrayNode $node)
             }
         }
     }
+
+    /**
+     * @return NodeDefinition[]
+     */
+    public function getChildNodeDefinitions()
+    {
+        return $this->children;
+    }
 }
diff --git a/src/Symfony/Component/Config/Definition/Builder/BooleanNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/BooleanNodeDefinition.php
@@ -38,7 +38,7 @@ public function __construct(?string $name, NodeParentInterface $parent = null)
      */
     protected function instantiateNode()
     {
-        return new BooleanNode($this->name, $this->parent);
+        return new BooleanNode($this->name, $this->parent, $this->pathSeparator);
     }
 
     /**
diff --git a/src/Symfony/Component/Config/Definition/Builder/EnumNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/EnumNodeDefinition.php
@@ -51,6 +51,6 @@ protected function instantiateNode()
             throw new \RuntimeException('You must call ->values() on enum nodes.');
         }
 
-        return new EnumNode($this->name, $this->parent, $this->values);
+        return new EnumNode($this->name, $this->parent, $this->values, $this->pathSeparator);
     }
 }
diff --git a/src/Symfony/Component/Config/Definition/Builder/FloatNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/FloatNodeDefinition.php
@@ -27,6 +27,6 @@ class FloatNodeDefinition extends NumericNodeDefinition
      */
     protected function instantiateNode()
     {
-        return new FloatNode($this->name, $this->parent, $this->min, $this->max);
+        return new FloatNode($this->name, $this->parent, $this->min, $this->max, $this->pathSeparator);
     }
 }
diff --git a/src/Symfony/Component/Config/Definition/Builder/IntegerNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/IntegerNodeDefinition.php
@@ -27,6 +27,6 @@ class IntegerNodeDefinition extends NumericNodeDefinition
      */
     protected function instantiateNode()
     {
-        return new IntegerNode($this->name, $this->parent, $this->min, $this->max);
+        return new IntegerNode($this->name, $this->parent, $this->min, $this->max, $this->pathSeparator);
     }
 }
diff --git a/src/Symfony/Component/Config/Definition/Builder/NodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/NodeDefinition.php
@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Config\Definition\Builder;
 
+use Symfony\Component\Config\Definition\BaseNode;
 use Symfony\Component\Config\Definition\NodeInterface;
 use Symfony\Component\Config\Definition\Exception\InvalidDefinitionException;
 
@@ -33,6 +34,7 @@ abstract class NodeDefinition implements NodeParentInterface
     protected $nullEquivalent;
     protected $trueEquivalent = true;
     protected $falseEquivalent = false;
+    protected $pathSeparator = BaseNode::DEFAULT_PATH_SEPARATOR;
     protected $parent;
     protected $attributes = array();
 
@@ -346,4 +348,28 @@ protected function normalization()
      * @throws InvalidDefinitionException When the definition is invalid
      */
     abstract protected function createNode();
+
+    /**
+     * Set PathSeparator to use.
+     *
+     * @param string $separator
+     *
+     * @return $this
+     */
+    public function setPathSeparator(string $separator)
+    {
+        if ($this instanceof ParentNodeDefinitionInterface) {
+            if (method_exists($this, 'getChildNodeDefinitions')) {
+                foreach ($this->getChildNodeDefinitions() as $child) {
+                    $child->setPathSeparator($separator);
+                }
+            } else {
+                @trigger_error('Passing a ParentNodeDefinitionInterface without getChildNodeDefinitions() is deprecated since version 4.1 and will be removed in 5.0.', E_USER_DEPRECATED);
+            }
+        }
+
+        $this->pathSeparator = $separator;
+
+        return $this;
+    }
 }
diff --git a/src/Symfony/Component/Config/Definition/Builder/ParentNodeDefinitionInterface.php b/src/Symfony/Component/Config/Definition/Builder/ParentNodeDefinitionInterface.php
@@ -15,9 +15,14 @@
  * An interface that must be implemented by nodes which can have children.
  *
  * @author Victor Berchet <victor@suumit.com>
+ *
+ * @method NodeDefinition[] getChildNodeDefinitions() should be implemented since 4.1
  */
 interface ParentNodeDefinitionInterface
 {
+    /**
+     * @return NodeBuilder
+     */
     public function children();
 
     public function append(NodeDefinition $node);
diff --git a/src/Symfony/Component/Config/Definition/Builder/ScalarNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/ScalarNodeDefinition.php
@@ -27,6 +27,6 @@ class ScalarNodeDefinition extends VariableNodeDefinition
      */
     protected function instantiateNode()
     {
-        return new ScalarNode($this->name, $this->parent);
+        return new ScalarNode($this->name, $this->parent, $this->pathSeparator);
     }
 }
diff --git a/src/Symfony/Component/Config/Definition/Builder/TreeBuilder.php b/src/Symfony/Component/Config/Definition/Builder/TreeBuilder.php
@@ -50,13 +50,31 @@ public function root($name, $type = 'array', NodeBuilder $builder = null)
      */
     public function buildTree()
     {
-        if (null === $this->root) {
-            throw new \RuntimeException('The configuration tree has no root node.');
-        }
+        $this->assertTreeHasRootNode();
         if (null !== $this->tree) {
             return $this->tree;
         }
 
         return $this->tree = $this->root->getNode(true);
     }
+
+    public function setPathSeparator(string $separator)
+    {
+        $this->assertTreeHasRootNode();
+
+        // unset last built as changing path separator changes all nodes
+        $this->tree = null;
+
+        $this->root->setPathSeparator($separator);
+    }
+
+    /**
+     * @throws \RuntimeException if root node is not defined
+     */
+    private function assertTreeHasRootNode()
+    {
+        if (null === $this->root) {
+            throw new \RuntimeException('The configuration tree has no root node.');
+        }
+    }
 }
diff --git a/src/Symfony/Component/Config/Definition/Builder/VariableNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/VariableNodeDefinition.php
@@ -27,7 +27,7 @@ class VariableNodeDefinition extends NodeDefinition
      */
     protected function instantiateNode()
     {
-        return new VariableNode($this->name, $this->parent);
+        return new VariableNode($this->name, $this->parent, $this->pathSeparator);
     }
 
     /**
diff --git a/src/Symfony/Component/Config/Definition/EnumNode.php b/src/Symfony/Component/Config/Definition/EnumNode.php
@@ -22,14 +22,14 @@ class EnumNode extends ScalarNode
 {
     private $values;
 
-    public function __construct(?string $name, NodeInterface $parent = null, array $values = array())
+    public function __construct(?string $name, NodeInterface $parent = null, array $values = array(), string $pathSeparator = BaseNode::DEFAULT_PATH_SEPARATOR)
     {
         $values = array_unique($values);
         if (empty($values)) {
             throw new \InvalidArgumentException('$values must contain at least one element.');
         }
 
-        parent::__construct($name, $parent);
+        parent::__construct($name, $parent, $pathSeparator);
         $this->values = $values;
     }
 
diff --git a/src/Symfony/Component/Config/Definition/NumericNode.php b/src/Symfony/Component/Config/Definition/NumericNode.php
@@ -23,9 +23,9 @@ class NumericNode extends ScalarNode
     protected $min;
     protected $max;
 
-    public function __construct(?string $name, NodeInterface $parent = null, $min = null, $max = null)
+    public function __construct(?string $name, NodeInterface $parent = null, $min = null, $max = null, string $pathSeparator = BaseNode::DEFAULT_PATH_SEPARATOR)
     {
-        parent::__construct($name, $parent);
+        parent::__construct($name, $parent, $pathSeparator);
         $this->min = $min;
         $this->max = $max;
     }
diff --git a/src/Symfony/Component/Config/Tests/Definition/BaseNodeTest.php b/src/Symfony/Component/Config/Tests/Definition/BaseNodeTest.php
@@ -0,0 +1,56 @@
+<?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\Config\Tests\Definition;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Config\Definition\BaseNode;
+use Symfony\Component\Config\Definition\NodeInterface;
+
+class BaseNodeTest extends TestCase
+{
+    /**
+     * @dataProvider providePath
+     */
+    public function testGetPathForChildNode($expected, array $params)
+    {
+        $constructorArgs = array();
+        $constructorArgs[] = $params[0];
+
+        if (isset($params[1])) {
+            // Handle old PHPUnit version for PHP 5.5
+            $parent = method_exists($this, 'createMock')
+                    ? $this->createMock(NodeInterface::class)
+                    : $this->getMock(NodeInterface::class);
+            $parent->method('getPath')->willReturn($params[1]);
+
+            $constructorArgs[] = $parent;
+
+            if (isset($params[2])) {
+                $constructorArgs[] = $params[2];
+            }
+        }
+
+        $node = $this->getMockForAbstractClass(BaseNode::class, $constructorArgs);
+
+        $this->assertSame($expected, $node->getPath());
+    }
+
+    public function providePath()
+    {
+        return array(
+            'name only' => array('root', array('root')),
+            'name and parent' => array('foo.bar.baz.bim', array('bim', 'foo.bar.baz')),
+            'name and separator' => array('foo', array('foo', null, '/')),
+            'name, parent and separator' => array('foo.bar/baz/bim', array('bim', 'foo.bar/baz', '/')),
+        );
+    }
+}
diff --git a/src/Symfony/Component/Config/Tests/Definition/Builder/NodeDefinitionTest.php b/src/Symfony/Component/Config/Tests/Definition/Builder/NodeDefinitionTest.php
@@ -0,0 +1,39 @@
+<?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\Config\Tests\Definition\Builder;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
+use Symfony\Component\Config\Definition\Builder\NodeDefinition;
+use Symfony\Component\Config\Definition\Builder\ScalarNodeDefinition;
+
+class NodeDefinitionTest extends TestCase
+{
+    public function testDefaultPathSeparatorIsDot()
+    {
+        $node = $this->getMockForAbstractClass(NodeDefinition::class, array('foo'));
+
+        $this->assertAttributeSame('.', 'pathSeparator', $node);
+    }
+
+    public function testSetPathSeparatorChangesChildren()
+    {
+        $node = new ArrayNodeDefinition('foo');
+        $scalar = new ScalarNodeDefinition('bar');
+        $node->append($scalar);
+
+        $node->setPathSeparator('/');
+
+        $this->assertAttributeSame('/', 'pathSeparator', $node);
+        $this->assertAttributeSame('/', 'pathSeparator', $scalar);
+    }
+}
diff --git a/src/Symfony/Component/Config/Tests/Definition/Builder/TreeBuilderTest.php b/src/Symfony/Component/Config/Tests/Definition/Builder/TreeBuilderTest.php

Original file line number	Diff line number	Diff line change
`@@ -407,7 +407,7 @@ protected function getNodeBuilder()`
`407`	`407`	`protected function createNode()`
`408`	`408`	`{`
`409`	`409`	`if (null === $this->prototype) {`
`410`		`- $node = new ArrayNode($this->name, $this->parent);`
	`410`	`+ $node = new ArrayNode($this->name, $this->parent, $this->pathSeparator);`
`411`	`411`
`412`	`412`	`$this->validateConcreteNode($node);`
`413`	`413`
`@@ -418,7 +418,7 @@ protected function createNode()`
`418`	`418`	`$node->addChild($child->getNode());`
`419`	`419`	`}`
`420`	`420`	`} else {`
`421`		`- $node = new PrototypedArrayNode($this->name, $this->parent);`
	`421`	`+ $node = new PrototypedArrayNode($this->name, $this->parent, $this->pathSeparator);`
`422`	`422`
`423`	`423`	`$this->validatePrototypeNode($node);`
`424`	`424`
`@@ -545,4 +545,12 @@ protected function validatePrototypeNode(PrototypedArrayNode $node)`
`545`	`545`	`}`
`546`	`546`	`}`
`547`	`547`	`}`
	`548`	`+`
	`549`	`+ /**`
	`550`	`+ * @return NodeDefinition[]`
	`551`	`+ */`
	`552`	`+ public function getChildNodeDefinitions()`
	`553`	`+ {`
	`554`	`+ return $this->children;`
	`555`	`+ }`
`548`	`556`	`}`
Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ public function __construct(?string $name, NodeParentInterface $parent = null)`
`38`	`38`	`*/`
`39`	`39`	`protected function instantiateNode()`
`40`	`40`	`{`
`41`		`- return new BooleanNode($this->name, $this->parent);`
	`41`	`+ return new BooleanNode($this->name, $this->parent, $this->pathSeparator);`
`42`	`42`	`}`
`43`	`43`
`44`	`44`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -51,6 +51,6 @@ protected function instantiateNode()`
`51`	`51`	`throw new \RuntimeException('You must call ->values() on enum nodes.');`
`52`	`52`	`}`
`53`	`53`
`54`		`- return new EnumNode($this->name, $this->parent, $this->values);`
	`54`	`+ return new EnumNode($this->name, $this->parent, $this->values, $this->pathSeparator);`
`55`	`55`	`}`
`56`	`56`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,6 @@ class FloatNodeDefinition extends NumericNodeDefinition`
`27`	`27`	`*/`
`28`	`28`	`protected function instantiateNode()`
`29`	`29`	`{`
`30`		`- return new FloatNode($this->name, $this->parent, $this->min, $this->max);`
	`30`	`+ return new FloatNode($this->name, $this->parent, $this->min, $this->max, $this->pathSeparator);`
`31`	`31`	`}`
`32`	`32`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,6 @@ class IntegerNodeDefinition extends NumericNodeDefinition`
`27`	`27`	`*/`
`28`	`28`	`protected function instantiateNode()`
`29`	`29`	`{`
`30`		`- return new IntegerNode($this->name, $this->parent, $this->min, $this->max);`
	`30`	`+ return new IntegerNode($this->name, $this->parent, $this->min, $this->max, $this->pathSeparator);`
`31`	`31`	`}`
`32`	`32`	`}`