Add feature allowing change of Path Separator

bburnichon · bburnichon · commit 2e5448635659 · 2017-10-26T21:56:25.000+02:00
diff --git a/UPGRADE-4.1.md b/UPGRADE-4.1.md
@@ -0,0 +1,8 @@
+UPGRADE FROM 4.0 to 4.1
+=======================
+
+Config
+------
+
+ * Implementing `ParentNodeDefinitionInterface` without the `getChildNodeDefinitions()` method
+   is deprecated and will be unsupported in 5.0.
diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md
@@ -0,0 +1,7 @@
+UPGRADE FROM 4.x to 5.0
+=======================
+
+Config
+------
+
+ * Added the `getChildNodeDefinitions()` method to `ParentNodeDefinitionInterface`.
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,21 +34,24 @@ abstract class BaseNode implements NodeInterface
     protected $deprecationMessage = null;
     protected $equivalentValues = array();
     protected $attributes = array();
+    protected $pathSeparator;
 
     /**
-     * @param string        $name   The name of the node
-     * @param NodeInterface $parent The parent of this node
+     * @param string        $name          The name of the node
+     * @param NodeInterface $parent        The parent of this node
+     * @param string        $pathSeparator The Path Separator that is used
      *
-     * @throws \InvalidArgumentException if the name contains a period
+     * @throws \InvalidArgumentException if the name contains path separator
      */
-    public function __construct($name, NodeInterface $parent = null)
+    public function __construct($name, NodeInterface $parent = null, $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)
@@ -233,13 +238,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
@@ -405,7 +405,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);
 
@@ -416,7 +416,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($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();
 
@@ -350,4 +352,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
@@ -22,6 +22,10 @@ class TreeBuilder implements NodeParentInterface
 {
     protected $tree;
     protected $root;
+
+    /**
+     * @deprecated since version 4.1, to be removed in 5.0
+     */
     protected $builder;
 
     /**
@@ -51,13 +55,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($name, NodeInterface $parent = null, array $values = array())
+    public function __construct($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($name, NodeInterface $parent = null, $min = null, $max = null)
+    public function __construct($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
`@@ -405,7 +405,7 @@ protected function getNodeBuilder()`
`405`	`405`	`protected function createNode()`
`406`	`406`	`{`
`407`	`407`	`if (null === $this->prototype) {`
`408`		`- $node = new ArrayNode($this->name, $this->parent);`
	`408`	`+ $node = new ArrayNode($this->name, $this->parent, $this->pathSeparator);`
`409`	`409`
`410`	`410`	`$this->validateConcreteNode($node);`
`411`	`411`
`@@ -416,7 +416,7 @@ protected function createNode()`
`416`	`416`	`$node->addChild($child->getNode());`
`417`	`417`	`}`
`418`	`418`	`} else {`
`419`		`- $node = new PrototypedArrayNode($this->name, $this->parent);`
	`419`	`+ $node = new PrototypedArrayNode($this->name, $this->parent, $this->pathSeparator);`
`420`	`420`
`421`	`421`	`$this->validatePrototypeNode($node);`
`422`	`422`
`@@ -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($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`	`}`