[Config] Deprecate setting a default value to a node that is required, and vice versa

GromNaN · GromNaN · commit 9549b89feeab · 2025-10-16T21:08:00.000+02:00
diff --git a/UPGRADE-7.4.md b/UPGRADE-7.4.md
@@ -22,6 +22,7 @@ Config
 ------
 
  * Deprecate accessing the internal scope of the loader in PHP config files, use only its public API instead
+ * Deprecate setting a default value to a node that is required, and vice versa
 
 Console
 -------
diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/UserProvider/LdapFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/UserProvider/LdapFactory.php
@@ -53,7 +53,7 @@ public function addConfiguration(NodeDefinition $node): void
     {
         $node
             ->children()
-                ->scalarNode('service')->isRequired()->cannotBeEmpty()->defaultValue('ldap')->end()
+                ->scalarNode('service')->isRequired()->cannotBeEmpty()->example('ldap')->end()
                 ->scalarNode('base_dn')->isRequired()->cannotBeEmpty()->end()
                 ->scalarNode('search_dn')->defaultNull()->end()
                 ->scalarNode('search_password')->defaultNull()->end()
diff --git a/src/Symfony/Component/Config/CHANGELOG.md b/src/Symfony/Component/Config/CHANGELOG.md
@@ -10,6 +10,7 @@ CHANGELOG
  * Add `ArrayNodeDefinition::acceptAndWrap()` to list alternative types that should be accepted and wrapped in an array
  * Add array-shapes to generated config builders
  * Deprecate accessing the internal scope of the loader in PHP config files, use only its public API instead
+ * Deprecate setting a default value to a node that is required, and vice versa
 
 7.3
 ---
diff --git a/src/Symfony/Component/Config/Definition/Builder/NodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/NodeDefinition.php
@@ -178,6 +178,12 @@ public function getNode(bool $forceRootNode = false): NodeInterface
      */
     public function defaultValue(mixed $value): static
     {
+        if ($this->required) {
+            // Uncomment in 8.0
+            // throw new InvalidDefinitionException(sprintf('You cannot set a default value to the node "%s" as it is required.', $this->name));
+            trigger_deprecation('symfony/config', '7.4', 'Setting a default value to the node "%s" is deprecated when it is required.', $this->name);
+        }
+
         $this->default = true;
         $this->defaultValue = $value;
 
@@ -191,6 +197,12 @@ public function defaultValue(mixed $value): static
      */
     public function isRequired(): static
     {
+        if ($this->default) {
+            // Uncomment in 8.0
+            // throw new InvalidDefinitionException(sprintf('You cannot set the node "%s" as required as it has a default value.', $this->name));
+            trigger_deprecation('symfony/config', '7.4', 'Setting the node "%s" as required is deprecated when it has a default value.', $this->name);
+        }
+
         $this->required = true;
 
         return $this;
diff --git a/src/Symfony/Component/Config/Tests/Definition/Builder/NodeDefinitionTest.php b/src/Symfony/Component/Config/Tests/Definition/Builder/NodeDefinitionTest.php
@@ -11,9 +11,17 @@
 
 namespace Symfony\Component\Config\Tests\Definition\Builder;
 
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\Attributes\Group;
+use PHPUnit\Framework\Attributes\IgnoreDeprecations;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
+use Symfony\Component\Config\Definition\Builder\BooleanNodeDefinition;
 use Symfony\Component\Config\Definition\Builder\NodeDefinition;
+use Symfony\Component\Config\Definition\Builder\ScalarNodeDefinition;
+use Symfony\Component\Config\Definition\Builder\StringNodeDefinition;
+use Symfony\Component\Config\Definition\Builder\VariableNodeDefinition;
+use Symfony\Component\Config\Definition\Exception\InvalidDefinitionException;
 
 class NodeDefinitionTest extends TestCase
 {
@@ -60,10 +68,50 @@ public function testDocUrlWithoutPackage()
 
     public function testUnknownPackageThrowsException()
     {
+        $node = new ArrayNodeDefinition('node');
+
         $this->expectException(\OutOfBoundsException::class);
         $this->expectExceptionMessage('Package "phpunit/invalid" is not installed');
 
-        $node = new ArrayNodeDefinition('node');
         $node->docUrl('https://example.com/doc/{package}/{version:major}.{version:minor}', 'phpunit/invalid');
     }
+
+    #[IgnoreDeprecations]
+    #[Group('legacy')]
+    #[DataProvider('provideDefinitionClassesAndDefaultValues')]
+    public function testIncoherentRequiredAndDefaultValue(string $class, mixed $defaultValue)
+    {
+        $node = new $class('foo');
+        self::assertInstanceOf(NodeDefinition::class, $node);
+
+        // $this->expectException(InvalidDefinitionException::class);
+        // $this->expectExceptionMessage('You cannot set the node "foo" as required as it has a default value.');
+        $this->expectUserDeprecationMessage('Since symfony/config 7.4: Setting the node "foo" as required is deprecated when it has a default value.');
+
+        $node->defaultValue($defaultValue)->isRequired();
+    }
+
+    #[IgnoreDeprecations]
+    #[Group('legacy')]
+    #[DataProvider('provideDefinitionClassesAndDefaultValues')]
+    public function testIncoherentDefaultValueAndRequired(string $class, mixed $defaultValue)
+    {
+        $node = new $class('foo');
+        self::assertInstanceOf(NodeDefinition::class, $node);
+
+        // $this->expectException(InvalidDefinitionException::class);
+        // $this->expectExceptionMessage('You cannot set a default value to the node "foo" as it is required.');
+        $this->expectUserDeprecationMessage('Since symfony/config 7.4: Setting a default value to the node "foo" is deprecated when it is required.');
+
+        $node->isRequired()->defaultValue($defaultValue);
+    }
+
+    public static function provideDefinitionClassesAndDefaultValues()
+    {
+        yield [ArrayNodeDefinition::class, []];
+        yield [ScalarNodeDefinition::class, null];
+        yield [BooleanNodeDefinition::class, false];
+        yield [StringNodeDefinition::class, 'default'];
+        yield [VariableNodeDefinition::class, 'default'];
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ public function addConfiguration(NodeDefinition $node): void`
`53`	`53`	`{`
`54`	`54`	`$node`
`55`	`55`	`->children()`
`56`		`- ->scalarNode('service')->isRequired()->cannotBeEmpty()->defaultValue('ldap')->end()`
	`56`	`+ ->scalarNode('service')->isRequired()->cannotBeEmpty()->example('ldap')->end()`
`57`	`57`	`->scalarNode('base_dn')->isRequired()->cannotBeEmpty()->end()`
`58`	`58`	`->scalarNode('search_dn')->defaultNull()->end()`
`59`	`59`	`->scalarNode('search_password')->defaultNull()->end()`