Skip to content

Commit 9ef8396

Browse files
GromNaNGromNaN
committed
feature #62090 [Config] Deprecate setting a default value to a node that is required (GromNaN)
This PR was merged into the 7.4 branch. Discussion ---------- [Config] Deprecate setting a default value to a node that is required | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | no | Deprecations? | yes | Issues | Fix #62076 | License | MIT Commits ------- 5a47df9 [Config] Deprecate setting a default value to a node that is required, and vice versa
2 parents bfbe7ac + 5a47df9 commit 9ef8396

File tree

5 files changed

+62
-2
lines changed

5 files changed

+62
-2
lines changed

‎UPGRADE-7.4.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ Config
2222
------
2323

2424
* Deprecate accessing the internal scope of the loader in PHP config files, use only its public API instead
25+
* Deprecate setting a default value to a node that is required, and vice versa
2526

2627
Console
2728
-------

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/UserProvider/LdapFactory.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public function addConfiguration(NodeDefinition $node): void
5353
{
5454
$node
5555
->children()
56-
->scalarNode('service')->isRequired()->cannotBeEmpty()->defaultValue('ldap')->end()
56+
->scalarNode('service')->isRequired()->cannotBeEmpty()->example('ldap')->end()
5757
->scalarNode('base_dn')->isRequired()->cannotBeEmpty()->end()
5858
->scalarNode('search_dn')->defaultNull()->end()
5959
->scalarNode('search_password')->defaultNull()->end()

‎src/Symfony/Component/Config/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ CHANGELOG
1010
* Add `ArrayNodeDefinition::acceptAndWrap()` to list alternative types that should be accepted and wrapped in an array
1111
* Add array-shapes to generated config builders
1212
* Deprecate accessing the internal scope of the loader in PHP config files, use only its public API instead
13+
* Deprecate setting a default value to a node that is required, and vice versa
1314

1415
7.3
1516
---

‎src/Symfony/Component/Config/Definition/Builder/NodeDefinition.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,11 @@ public function getNode(bool $forceRootNode = false): NodeInterface
178178
*/
179179
public function defaultValue(mixed $value): static
180180
{
181+
if ($this->required) {
182+
// throw new InvalidDefinitionException(sprintf('The node "%s" cannot be required and have a default value.', $this->name));
183+
trigger_deprecation('symfony/config', '7.4', 'Setting a default value to a required node is deprecated. Remove the default value from the node "%s" or make it optional.', $this->name);
184+
}
185+
181186
$this->default = true;
182187
$this->defaultValue = $value;
183188

@@ -191,6 +196,11 @@ public function defaultValue(mixed $value): static
191196
*/
192197
public function isRequired(): static
193198
{
199+
if ($this->default) {
200+
// throw new InvalidDefinitionException(sprintf('The node "%s" cannot be required and have a default value.', $this->name));
201+
trigger_deprecation('symfony/config', '7.4', 'Flagging a node with a default value as required is deprecated. Remove the default from node "%s" or make it optional.', $this->name);
202+
}
203+
194204
$this->required = true;
195205

196206
return $this;

‎src/Symfony/Component/Config/Tests/Definition/Builder/NodeDefinitionTest.php‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,17 @@
1111

1212
namespace Symfony\Component\Config\Tests\Definition\Builder;
1313

14+
use PHPUnit\Framework\Attributes\DataProvider;
15+
use PHPUnit\Framework\Attributes\Group;
16+
use PHPUnit\Framework\Attributes\IgnoreDeprecations;
1417
use PHPUnit\Framework\TestCase;
1518
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
19+
use Symfony\Component\Config\Definition\Builder\BooleanNodeDefinition;
1620
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
21+
use Symfony\Component\Config\Definition\Builder\ScalarNodeDefinition;
22+
use Symfony\Component\Config\Definition\Builder\StringNodeDefinition;
23+
use Symfony\Component\Config\Definition\Builder\VariableNodeDefinition;
24+
use Symfony\Component\Config\Definition\Exception\InvalidDefinitionException;
1725

1826
class NodeDefinitionTest extends TestCase
1927
{
@@ -60,10 +68,50 @@ public function testDocUrlWithoutPackage()
6068

6169
public function testUnknownPackageThrowsException()
6270
{
71+
$node = new ArrayNodeDefinition('node');
72+
6373
$this->expectException(\OutOfBoundsException::class);
6474
$this->expectExceptionMessage('Package "phpunit/invalid" is not installed');
6575

66-
$node = new ArrayNodeDefinition('node');
6776
$node->docUrl('https://example.com/doc/{package}/{version:major}.{version:minor}', 'phpunit/invalid');
6877
}
78+
79+
#[IgnoreDeprecations]
80+
#[Group('legacy')]
81+
#[DataProvider('provideDefinitionClassesAndDefaultValues')]
82+
public function testIncoherentRequiredAndDefaultValue(string $class, mixed $defaultValue)
83+
{
84+
$node = new $class('foo');
85+
self::assertInstanceOf(NodeDefinition::class, $node);
86+
87+
// $this->expectException(InvalidDefinitionException::class);
88+
// $this->expectExceptionMessage('The node "foo" cannot be required and have a default value.');
89+
$this->expectUserDeprecationMessage('Since symfony/config 7.4: Flagging a node with a default value as required is deprecated. Remove the default from node "foo" or make it optional.');
90+
91+
$node->defaultValue($defaultValue)->isRequired();
92+
}
93+
94+
#[IgnoreDeprecations]
95+
#[Group('legacy')]
96+
#[DataProvider('provideDefinitionClassesAndDefaultValues')]
97+
public function testIncoherentDefaultValueAndRequired(string $class, mixed $defaultValue)
98+
{
99+
$node = new $class('foo');
100+
self::assertInstanceOf(NodeDefinition::class, $node);
101+
102+
// $this->expectException(InvalidDefinitionException::class);
103+
// $this->expectExceptionMessage('The node "foo" cannot be required and have a default value.');
104+
$this->expectUserDeprecationMessage('Since symfony/config 7.4: Setting a default value to a required node is deprecated. Remove the default value from the node "foo" or make it optional.');
105+
106+
$node->isRequired()->defaultValue($defaultValue);
107+
}
108+
109+
public static function provideDefinitionClassesAndDefaultValues()
110+
{
111+
yield [ArrayNodeDefinition::class, []];
112+
yield [ScalarNodeDefinition::class, null];
113+
yield [BooleanNodeDefinition::class, false];
114+
yield [StringNodeDefinition::class, 'default'];
115+
yield [VariableNodeDefinition::class, 'default'];
116+
}
69117
}

0 commit comments

Comments
 (0)