Skip to content

Commit

Permalink
feature #27476 [Config] deprecate tree builders without root nodes (x…
Browse files Browse the repository at this point in the history
…abbuh)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Config] deprecate tree builders without root nodes

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

While reviewing #27472 I wondered if we really need support config trees without a root node. If we did not support it, users wouldn't create pseudo configuration classes when they were actually not needed.

Commits
-------

c2ce153 deprecate tree builders without root nodes
  • Loading branch information
fabpot committed Jun 25, 2018
2 parents d871473 + c2ce153 commit 21a3439
Show file tree
Hide file tree
Showing 24 changed files with 110 additions and 77 deletions.
6 changes: 6 additions & 0 deletions UPGRADE-4.2.md
Expand Up @@ -25,6 +25,12 @@ Form
{{ form_widget(field) }} {{ form_widget(field) }}
{% endfor %} {% endfor %}
``` ```

Config
------

* Deprecated constructing a `TreeBuilder` without passing root node information.

Security Security
-------- --------


Expand Down
1 change: 1 addition & 0 deletions UPGRADE-5.0.md
Expand Up @@ -9,6 +9,7 @@ Cache
Config Config
------ ------


* Dropped support for constructing a `TreeBuilder` without passing root node information.
* Added the `getChildNodeDefinitions()` method to `ParentNodeDefinitionInterface`. * Added the `getChildNodeDefinitions()` method to `ParentNodeDefinitionInterface`.
* The `Processor` class has been made final * The `Processor` class has been made final


Expand Down
Expand Up @@ -26,10 +26,9 @@ class Configuration implements ConfigurationInterface
*/ */
public function getConfigTreeBuilder() public function getConfigTreeBuilder()
{ {
$treeBuilder = new TreeBuilder(); $treeBuilder = new TreeBuilder('debug');
$rootNode = $treeBuilder->root('debug');


$rootNode $treeBuilder->getRootNode()
->children() ->children()
->integerNode('max_items') ->integerNode('max_items')
->info('Max number of displayed items past the first level, -1 means no limit') ->info('Max number of displayed items past the first level, -1 means no limit')
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Bundle/DebugBundle/composer.json
Expand Up @@ -23,11 +23,12 @@
"symfony/var-dumper": "^4.1.1" "symfony/var-dumper": "^4.1.1"
}, },
"require-dev": { "require-dev": {
"symfony/config": "~3.4|~4.0", "symfony/config": "~4.2",
"symfony/dependency-injection": "~3.4|~4.0", "symfony/dependency-injection": "~3.4|~4.0",
"symfony/web-profiler-bundle": "~3.4|~4.0" "symfony/web-profiler-bundle": "~3.4|~4.0"
}, },
"conflict": { "conflict": {
"symfony/config": "<4.2",
"symfony/dependency-injection": "<3.4" "symfony/dependency-injection": "<3.4"
}, },
"suggest": { "suggest": {
Expand Down
Expand Up @@ -54,8 +54,8 @@ public function __construct(bool $debug)
*/ */
public function getConfigTreeBuilder() public function getConfigTreeBuilder()
{ {
$treeBuilder = new TreeBuilder(); $treeBuilder = new TreeBuilder('framework');
$rootNode = $treeBuilder->root('framework'); $rootNode = $treeBuilder->getRootNode();


$rootNode $rootNode
->beforeNormalization() ->beforeNormalization()
Expand Down
Expand Up @@ -25,11 +25,10 @@ public function __construct($customConfig = null)


public function getConfigTreeBuilder() public function getConfigTreeBuilder()
{ {
$treeBuilder = new TreeBuilder(); $treeBuilder = new TreeBuilder('test');
$rootNode = $treeBuilder->root('test');


if ($this->customConfig) { if ($this->customConfig) {
$this->customConfig->addConfiguration($rootNode); $this->customConfig->addConfiguration($treeBuilder->getRootNode());
} }


return $treeBuilder; return $treeBuilder;
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/FrameworkBundle/composer.json
Expand Up @@ -20,7 +20,7 @@
"ext-xml": "*", "ext-xml": "*",
"symfony/cache": "~3.4|~4.0", "symfony/cache": "~3.4|~4.0",
"symfony/dependency-injection": "^4.1.1", "symfony/dependency-injection": "^4.1.1",
"symfony/config": "~3.4|~4.0", "symfony/config": "~4.2",
"symfony/event-dispatcher": "^4.1", "symfony/event-dispatcher": "^4.1",
"symfony/http-foundation": "^4.1", "symfony/http-foundation": "^4.1",
"symfony/http-kernel": "^4.1", "symfony/http-kernel": "^4.1",
Expand Down
Expand Up @@ -41,8 +41,8 @@ public function __construct(array $factories, array $userProviderFactories)
*/ */
public function getConfigTreeBuilder() public function getConfigTreeBuilder()
{ {
$tb = new TreeBuilder(); $tb = new TreeBuilder('security');
$rootNode = $tb->root('security'); $rootNode = $tb->getRootNode();


$rootNode $rootNode
->beforeNormalization() ->beforeNormalization()
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/SecurityBundle/composer.json
Expand Up @@ -18,6 +18,7 @@
"require": { "require": {
"php": "^7.1.3", "php": "^7.1.3",
"ext-xml": "*", "ext-xml": "*",
"symfony/config": "^4.2",
"symfony/security": "~4.2", "symfony/security": "~4.2",
"symfony/dependency-injection": "^3.4.3|^4.0.3", "symfony/dependency-injection": "^3.4.3|^4.0.3",
"symfony/http-kernel": "^4.1" "symfony/http-kernel": "^4.1"
Expand Down
Expand Up @@ -29,8 +29,8 @@ class Configuration implements ConfigurationInterface
*/ */
public function getConfigTreeBuilder() public function getConfigTreeBuilder()
{ {
$treeBuilder = new TreeBuilder(); $treeBuilder = new TreeBuilder('twig');
$rootNode = $treeBuilder->root('twig'); $rootNode = $treeBuilder->getRootNode();


$rootNode $rootNode
->children() ->children()
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/TwigBundle/composer.json
Expand Up @@ -17,7 +17,7 @@
], ],
"require": { "require": {
"php": "^7.1.3", "php": "^7.1.3",
"symfony/config": "~3.4|~4.0", "symfony/config": "~4.2",
"symfony/twig-bridge": "^3.4.3|^4.0.3", "symfony/twig-bridge": "^3.4.3|^4.0.3",
"symfony/http-foundation": "~4.1", "symfony/http-foundation": "~4.1",
"symfony/http-kernel": "~4.1", "symfony/http-kernel": "~4.1",
Expand Down
Expand Up @@ -31,10 +31,9 @@ class Configuration implements ConfigurationInterface
*/ */
public function getConfigTreeBuilder() public function getConfigTreeBuilder()
{ {
$treeBuilder = new TreeBuilder(); $treeBuilder = new TreeBuilder('web_profiler');
$rootNode = $treeBuilder->root('web_profiler');


$rootNode $treeBuilder->getRootNode()
->children() ->children()
->booleanNode('toolbar')->defaultFalse()->end() ->booleanNode('toolbar')->defaultFalse()->end()
->booleanNode('intercept_redirects')->defaultFalse()->end() ->booleanNode('intercept_redirects')->defaultFalse()->end()
Expand Down
3 changes: 1 addition & 2 deletions src/Symfony/Bundle/WebProfilerBundle/composer.json
Expand Up @@ -17,20 +17,19 @@
], ],
"require": { "require": {
"php": "^7.1.3", "php": "^7.1.3",
"symfony/config": "^4.2",
"symfony/http-kernel": "~4.1", "symfony/http-kernel": "~4.1",
"symfony/routing": "~3.4|~4.0", "symfony/routing": "~3.4|~4.0",
"symfony/twig-bundle": "^3.4.3|^4.0.3", "symfony/twig-bundle": "^3.4.3|^4.0.3",
"symfony/var-dumper": "~3.4|~4.0", "symfony/var-dumper": "~3.4|~4.0",
"twig/twig": "~1.34|~2.4" "twig/twig": "~1.34|~2.4"
}, },
"require-dev": { "require-dev": {
"symfony/config": "~3.4|~4.0",
"symfony/console": "~3.4|~4.0", "symfony/console": "~3.4|~4.0",
"symfony/dependency-injection": "~3.4|~4.0", "symfony/dependency-injection": "~3.4|~4.0",
"symfony/stopwatch": "~3.4|~4.0" "symfony/stopwatch": "~3.4|~4.0"
}, },
"conflict": { "conflict": {
"symfony/config": "<3.4",
"symfony/dependency-injection": "<3.4", "symfony/dependency-injection": "<3.4",
"symfony/event-dispatcher": "<3.4", "symfony/event-dispatcher": "<3.4",
"symfony/var-dumper": "<3.4" "symfony/var-dumper": "<3.4"
Expand Down
5 changes: 5 additions & 0 deletions src/Symfony/Component/Config/CHANGELOG.md
@@ -1,6 +1,11 @@
CHANGELOG CHANGELOG
========= =========


4.2.0
-----

* deprecated constructing a `TreeBuilder` without passing root node information

4.1.0 4.1.0
----- -----


Expand Down
18 changes: 18 additions & 0 deletions src/Symfony/Component/Config/Definition/Builder/TreeBuilder.php
Expand Up @@ -24,6 +24,15 @@ class TreeBuilder implements NodeParentInterface
protected $tree; protected $tree;
protected $root; protected $root;


public function __construct(string $name = null, string $type = 'array', NodeBuilder $builder = null)
{
if (null === $name) {
@trigger_error('A tree builder without a root node is deprecated since Symfony 4.2 and will not be supported anymore in 5.0.', E_USER_DEPRECATED);
} else {
$this->root($name, $type, $builder);
}
}

/** /**
* Creates the root node. * Creates the root node.
* *
Expand All @@ -42,6 +51,15 @@ public function root($name, $type = 'array', NodeBuilder $builder = null)
return $this->root = $builder->node($name, $type)->setParent($this); return $this->root = $builder->node($name, $type)->setParent($this);
} }


public function getRootNode(): NodeDefinition
{
if (null === $this->root) {
throw new \RuntimeException(sprintf('Calling %s() before creating the root node is not supported, migrate to the new constructor signature instead.', __METHOD__));
}

return $this->root;
}

/** /**
* Builds the tree. * Builds the tree.
* *
Expand Down
Expand Up @@ -212,10 +212,10 @@ public function testEndThenPartNotSpecified()
*/ */
protected function getTestBuilder() protected function getTestBuilder()
{ {
$builder = new TreeBuilder(); $builder = new TreeBuilder('test');


return $builder return $builder
->root('test') ->getRootNode()
->children() ->children()
->variableNode('key') ->variableNode('key')
->validate() ->validate()
Expand Down
Expand Up @@ -19,10 +19,9 @@ class TreeBuilderTest extends TestCase
{ {
public function testUsingACustomNodeBuilder() public function testUsingACustomNodeBuilder()
{ {
$builder = new TreeBuilder(); $builder = new TreeBuilder('custom', 'array', new CustomNodeBuilder());
$root = $builder->root('custom', 'array', new CustomNodeBuilder());


$nodeBuilder = $root->children(); $nodeBuilder = $builder->getRootNode()->children();


$this->assertInstanceOf('Symfony\Component\Config\Tests\Fixtures\Builder\NodeBuilder', $nodeBuilder); $this->assertInstanceOf('Symfony\Component\Config\Tests\Fixtures\Builder\NodeBuilder', $nodeBuilder);


Expand All @@ -33,49 +32,46 @@ public function testUsingACustomNodeBuilder()


public function testOverrideABuiltInNodeType() public function testOverrideABuiltInNodeType()
{ {
$builder = new TreeBuilder(); $builder = new TreeBuilder('override', 'array', new CustomNodeBuilder());
$root = $builder->root('override', 'array', new CustomNodeBuilder());


$definition = $root->children()->variableNode('variable'); $definition = $builder->getRootNode()->children()->variableNode('variable');


$this->assertInstanceOf('Symfony\Component\Config\Tests\Fixtures\Builder\VariableNodeDefinition', $definition); $this->assertInstanceOf('Symfony\Component\Config\Tests\Fixtures\Builder\VariableNodeDefinition', $definition);
} }


public function testAddANodeType() public function testAddANodeType()
{ {
$builder = new TreeBuilder(); $builder = new TreeBuilder('override', 'array', new CustomNodeBuilder());
$root = $builder->root('override', 'array', new CustomNodeBuilder());


$definition = $root->children()->barNode('variable'); $definition = $builder->getRootNode()->children()->barNode('variable');


$this->assertInstanceOf('Symfony\Component\Config\Tests\Fixtures\Builder\BarNodeDefinition', $definition); $this->assertInstanceOf('Symfony\Component\Config\Tests\Fixtures\Builder\BarNodeDefinition', $definition);
} }


public function testCreateABuiltInNodeTypeWithACustomNodeBuilder() public function testCreateABuiltInNodeTypeWithACustomNodeBuilder()
{ {
$builder = new TreeBuilder(); $builder = new TreeBuilder('builtin', 'array', new CustomNodeBuilder());
$root = $builder->root('builtin', 'array', new CustomNodeBuilder());


$definition = $root->children()->booleanNode('boolean'); $definition = $builder->getRootNode()->children()->booleanNode('boolean');


$this->assertInstanceOf('Symfony\Component\Config\Definition\Builder\BooleanNodeDefinition', $definition); $this->assertInstanceOf('Symfony\Component\Config\Definition\Builder\BooleanNodeDefinition', $definition);
} }


public function testPrototypedArrayNodeUseTheCustomNodeBuilder() public function testPrototypedArrayNodeUseTheCustomNodeBuilder()
{ {
$builder = new TreeBuilder(); $builder = new TreeBuilder('override', 'array', new CustomNodeBuilder());
$root = $builder->root('override', 'array', new CustomNodeBuilder());


$root = $builder->getRootNode();
$root->prototype('bar')->end(); $root->prototype('bar')->end();


$this->assertInstanceOf('Symfony\Component\Config\Tests\Fixtures\BarNode', $root->getNode(true)->getPrototype()); $this->assertInstanceOf('Symfony\Component\Config\Tests\Fixtures\BarNode', $root->getNode(true)->getPrototype());
} }


public function testAnExtendedNodeBuilderGetsPropagatedToTheChildren() public function testAnExtendedNodeBuilderGetsPropagatedToTheChildren()
{ {
$builder = new TreeBuilder(); $builder = new TreeBuilder('propagation');


$builder->root('propagation') $builder->getRootNode()
->children() ->children()
->setNodeClass('extended', 'Symfony\Component\Config\Definition\Builder\BooleanNodeDefinition') ->setNodeClass('extended', 'Symfony\Component\Config\Definition\Builder\BooleanNodeDefinition')
->node('foo', 'extended')->end() ->node('foo', 'extended')->end()
Expand All @@ -99,9 +95,9 @@ public function testAnExtendedNodeBuilderGetsPropagatedToTheChildren()


public function testDefinitionInfoGetsTransferredToNode() public function testDefinitionInfoGetsTransferredToNode()
{ {
$builder = new TreeBuilder(); $builder = new TreeBuilder('test');


$builder->root('test')->info('root info') $builder->getRootNode()->info('root info')
->children() ->children()
->node('child', 'variable')->info('child info')->defaultValue('default') ->node('child', 'variable')->info('child info')->defaultValue('default')
->end() ->end()
Expand All @@ -116,9 +112,9 @@ public function testDefinitionInfoGetsTransferredToNode()


public function testDefinitionExampleGetsTransferredToNode() public function testDefinitionExampleGetsTransferredToNode()
{ {
$builder = new TreeBuilder(); $builder = new TreeBuilder('test');


$builder->root('test') $builder->getRootNode()
->example(array('key' => 'value')) ->example(array('key' => 'value'))
->children() ->children()
->node('child', 'variable')->info('child info')->defaultValue('default')->example('example') ->node('child', 'variable')->info('child info')->defaultValue('default')->example('example')
Expand All @@ -134,9 +130,9 @@ public function testDefinitionExampleGetsTransferredToNode()


public function testDefaultPathSeparatorIsDot() public function testDefaultPathSeparatorIsDot()
{ {
$builder = new TreeBuilder(); $builder = new TreeBuilder('propagation');


$builder->root('propagation') $builder->getRootNode()
->children() ->children()
->node('foo', 'variable')->end() ->node('foo', 'variable')->end()
->arrayNode('child') ->arrayNode('child')
Expand Down Expand Up @@ -164,9 +160,9 @@ public function testDefaultPathSeparatorIsDot()


public function testPathSeparatorIsPropagatedToChildren() public function testPathSeparatorIsPropagatedToChildren()
{ {
$builder = new TreeBuilder(); $builder = new TreeBuilder('propagation');


$builder->root('propagation') $builder->getRootNode()
->children() ->children()
->node('foo', 'variable')->end() ->node('foo', 'variable')->end()
->arrayNode('child') ->arrayNode('child')
Expand All @@ -192,4 +188,13 @@ public function testPathSeparatorIsPropagatedToChildren()
$this->assertInstanceOf('Symfony\Component\Config\Definition\BaseNode', $childChildren['foo']); $this->assertInstanceOf('Symfony\Component\Config\Definition\BaseNode', $childChildren['foo']);
$this->assertSame('propagation/child/foo', $childChildren['foo']->getPath()); $this->assertSame('propagation/child/foo', $childChildren['foo']->getPath());
} }

/**
* @group legacy
* @expectedDeprecation A tree builder without a root node is deprecated since Symfony 4.2 and will not be supported anymore in 5.0.
*/
public function testInitializingTreeBuildersWithoutRootNode()
{
new TreeBuilder();
}
} }
Expand Up @@ -20,9 +20,9 @@ class FinalizationTest extends TestCase
{ {
public function testUnsetKeyWithDeepHierarchy() public function testUnsetKeyWithDeepHierarchy()
{ {
$tb = new TreeBuilder(); $tb = new TreeBuilder('config', 'array');
$tree = $tb $tree = $tb
->root('config', 'array') ->getRootNode()
->children() ->children()
->node('level1', 'array') ->node('level1', 'array')
->canBeUnset() ->canBeUnset()
Expand Down

0 comments on commit 21a3439

Please sign in to comment.