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

Author: GromNaN

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
 ---

Definition/Builder/NodeDefinition.php

@@ -178,6 +178,11 @@ public function getNode(bool $forceRootNode = false): NodeInterface
      */
     public function defaultValue(mixed $value): static
     {
+        if ($this->required) {
+            // throw new InvalidDefinitionException(sprintf('The node "%s" cannot be required and have a default value.', $this->name));
+            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);
+        }
+
         $this->default = true;
         $this->defaultValue = $value;
 
@@ -191,6 +196,11 @@ public function defaultValue(mixed $value): static
      */
     public function isRequired(): static
     {
+        if ($this->default) {
+            // throw new InvalidDefinitionException(sprintf('The node "%s" cannot be required and have a default value.', $this->name));
+            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);
+        }
+
         $this->required = true;
 
         return $this;

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('The node "foo" cannot be required and have a default value.');
+        $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.');
+
+        $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('The node "foo" cannot be required and have a default value.');
+        $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.');
+
+        $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'];
+    }
 }