bug #51307 [DependencyInjection] fix dump xml with array/object/enum default value (Jean-Beru)

nicolas-grekas · nicolas-grekas · commit 997b97b0d9fd · 2023-08-14T12:47:42.000+02:00
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [DependencyInjection] fix dump xml with array/object/enum default value | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #48178 | License | MIT | Doc PR | This PR fixes #48178 when attempting to dump the container in these conditions: * The constructor has in its arguments a non-empty array, an object or an enum with a default value * A second argument is placed after * A third argument is placed after and is using configuration Example (might be more explicit): ```php class Foo { public function __construct( array $array = ['a', 'b', 'c'], bool $firstOptional = false, bool $secondOptional = false ) {} } ``` ```yaml services: Foo: arguments: secondOptional: true ``` :warning: This PR might cause Git conflicts in 6.3 and 6.4 versions. Do I have to create a dedicated PR for these versions ? Commits ------- ddc699a [DependencyInjection] fix dump xml with array/object/enum default value
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
@@ -212,13 +212,16 @@ private function autowireCalls(\ReflectionClass $reflectionClass, bool $isRoot,
                     unset($arguments[$j]);
                     $arguments[$namedArguments[$j]] = $value;
                 }
-                if ($namedArguments || !$value instanceof $this->defaultArgument) {
+                if (!$value instanceof $this->defaultArgument) {
                     continue;
                 }
 
                 if (\PHP_VERSION_ID >= 80100 && (\is_array($value->value) ? $value->value : \is_object($value->value))) {
-                    unset($arguments[$j]);
                     $namedArguments = $value->names;
+                }
+
+                if ($namedArguments) {
+                    unset($arguments[$j]);
                 } else {
                     $arguments[$j] = $value->value;
                 }
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/XmlDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/XmlDumperTest.php
@@ -17,11 +17,15 @@
 use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
 use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
 use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
+use Symfony\Component\DependencyInjection\Compiler\AutowirePass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\Dumper\XmlDumper;
 use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
 use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithDefaultArrayAttribute;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithDefaultEnumAttribute;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithDefaultObjectAttribute;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithEnumAttribute;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\FooUnitEnum;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\FooWithAbstractArgument;
@@ -287,6 +291,34 @@ public function testDumpHandlesEnumeration()
         $this->assertEquals(file_get_contents(self::$fixturesPath.'/xml/services_with_enumeration.xml'), $dumper->dump());
     }
 
+    /**
+     * @requires PHP 8.1
+     *
+     * @dataProvider provideDefaultClasses
+     */
+    public function testDumpHandlesDefaultAttribute($class, $expectedFile)
+    {
+        $container = new ContainerBuilder();
+        $container
+            ->register('foo', $class)
+            ->setPublic(true)
+            ->setAutowired(true)
+            ->setArguments([2 => true]);
+
+        (new AutowirePass())->process($container);
+
+        $dumper = new XmlDumper($container);
+
+        $this->assertSame(file_get_contents(self::$fixturesPath.'/xml/'.$expectedFile), $dumper->dump());
+    }
+
+    public static function provideDefaultClasses()
+    {
+        yield [FooClassWithDefaultArrayAttribute::class, 'services_with_default_array.xml'];
+        yield [FooClassWithDefaultObjectAttribute::class, 'services_with_default_object.xml'];
+        yield [FooClassWithDefaultEnumAttribute::class, 'services_with_default_enumeration.xml'];
+    }
+
     public function testDumpServiceWithAbstractArgument()
     {
         $container = new ContainerBuilder();
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php
@@ -17,12 +17,16 @@
 use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
 use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
 use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
+use Symfony\Component\DependencyInjection\Compiler\AutowirePass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Dumper\YamlDumper;
 use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
 use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithDefaultArrayAttribute;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithDefaultEnumAttribute;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithDefaultObjectAttribute;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithEnumAttribute;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\FooUnitEnum;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\FooWithAbstractArgument;
@@ -153,6 +157,34 @@ public function testDumpHandlesEnumeration()
         $this->assertEquals(file_get_contents(self::$fixturesPath.'/yaml/services_with_enumeration.yml'), $dumper->dump());
     }
 
+    /**
+     * @requires PHP 8.1
+     *
+     * @dataProvider provideDefaultClasses
+     */
+    public function testDumpHandlesDefaultAttribute($class, $expectedFile)
+    {
+        $container = new ContainerBuilder();
+        $container
+            ->register('foo', $class)
+            ->setPublic(true)
+            ->setAutowired(true)
+            ->setArguments([2 => true]);
+
+        (new AutowirePass())->process($container);
+
+        $dumper = new YamlDumper($container);
+
+        $this->assertSame(file_get_contents(self::$fixturesPath.'/yaml/'.$expectedFile), $dumper->dump());
+    }
+
+    public static function provideDefaultClasses()
+    {
+        yield [FooClassWithDefaultArrayAttribute::class, 'services_with_default_array.yml'];
+        yield [FooClassWithDefaultObjectAttribute::class, 'services_with_default_object.yml'];
+        yield [FooClassWithDefaultEnumAttribute::class, 'services_with_default_enumeration.yml'];
+    }
+
     public function testDumpServiceWithAbstractArgument()
     {
         $container = new ContainerBuilder();
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooClassWithDefaultArrayAttribute.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooClassWithDefaultArrayAttribute.php
@@ -0,0 +1,12 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
+
+class FooClassWithDefaultArrayAttribute
+{
+    public function __construct(
+        array $array = ['a', 'b', 'c'],
+        bool $firstOptional = false,
+        bool $secondOptional = false
+    ) {}
+}
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooClassWithDefaultEnumAttribute.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooClassWithDefaultEnumAttribute.php
@@ -0,0 +1,12 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
+
+class FooClassWithDefaultEnumAttribute
+{
+    public function __construct(
+        FooUnitEnum $enum = FooUnitEnum::FOO,
+        bool $firstOptional = false,
+        bool $secondOptional = false,
+    ) {}
+}
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooClassWithDefaultObjectAttribute.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooClassWithDefaultObjectAttribute.php
@@ -0,0 +1,12 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
+
+class FooClassWithDefaultObjectAttribute
+{
+    public function __construct(
+        object $object = new \stdClass(),
+        bool $firstOptional = false,
+        bool $secondOptional = false,
+    ) {}
+}
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_with_default_array.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_with_default_array.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="utf-8"?>
+<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
+  <services>
+    <service id="service_container" class="Symfony\Component\DependencyInjection\ContainerInterface" public="true" synthetic="true"/>
+    <service id="foo" class="Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithDefaultArrayAttribute" public="true" autowire="true">
+      <argument key="secondOptional">true</argument>
+    </service>
+    <service id="Psr\Container\ContainerInterface" alias="service_container">
+      <deprecated package="symfony/dependency-injection" version="5.1">The "%alias_id%" autowiring alias is deprecated. Define it explicitly in your app if you want to keep using it.</deprecated>
+    </service>
+    <service id="Symfony\Component\DependencyInjection\ContainerInterface" alias="service_container">
+      <deprecated package="symfony/dependency-injection" version="5.1">The "%alias_id%" autowiring alias is deprecated. Define it explicitly in your app if you want to keep using it.</deprecated>
+    </service>
+  </services>
+</container>
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_with_default_enumeration.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_with_default_enumeration.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="utf-8"?>
+<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
+  <services>
+    <service id="service_container" class="Symfony\Component\DependencyInjection\ContainerInterface" public="true" synthetic="true"/>
+    <service id="foo" class="Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithDefaultEnumAttribute" public="true" autowire="true">
+      <argument key="secondOptional">true</argument>
+    </service>
+    <service id="Psr\Container\ContainerInterface" alias="service_container">
+      <deprecated package="symfony/dependency-injection" version="5.1">The "%alias_id%" autowiring alias is deprecated. Define it explicitly in your app if you want to keep using it.</deprecated>
+    </service>
+    <service id="Symfony\Component\DependencyInjection\ContainerInterface" alias="service_container">
+      <deprecated package="symfony/dependency-injection" version="5.1">The "%alias_id%" autowiring alias is deprecated. Define it explicitly in your app if you want to keep using it.</deprecated>
+    </service>
+  </services>
+</container>
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_with_default_object.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_with_default_object.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="utf-8"?>
+<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
+  <services>
+    <service id="service_container" class="Symfony\Component\DependencyInjection\ContainerInterface" public="true" synthetic="true"/>
+    <service id="foo" class="Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithDefaultObjectAttribute" public="true" autowire="true">
+      <argument key="secondOptional">true</argument>
+    </service>
+    <service id="Psr\Container\ContainerInterface" alias="service_container">
+      <deprecated package="symfony/dependency-injection" version="5.1">The "%alias_id%" autowiring alias is deprecated. Define it explicitly in your app if you want to keep using it.</deprecated>
+    </service>
+    <service id="Symfony\Component\DependencyInjection\ContainerInterface" alias="service_container">
+      <deprecated package="symfony/dependency-injection" version="5.1">The "%alias_id%" autowiring alias is deprecated. Define it explicitly in your app if you want to keep using it.</deprecated>
+    </service>
+  </services>
+</container>
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_default_array.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_default_array.yml
@@ -0,0 +1,23 @@
+
+services:
+    service_container:
+        class: Symfony\Component\DependencyInjection\ContainerInterface
+        public: true
+        synthetic: true
+    foo:
+        class: Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithDefaultArrayAttribute
+        public: true
+        autowire: true
+        arguments: { secondOptional: true }
+    Psr\Container\ContainerInterface:
+        alias: service_container
+        deprecated:
+            package: symfony/dependency-injection
+            version: 5.1
+            message: The "%alias_id%" autowiring alias is deprecated. Define it explicitly in your app if you want to keep using it.
+    Symfony\Component\DependencyInjection\ContainerInterface:
+        alias: service_container
+        deprecated:
+            package: symfony/dependency-injection
+            version: 5.1
+            message: The "%alias_id%" autowiring alias is deprecated. Define it explicitly in your app if you want to keep using it.
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_default_enumeration.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_default_enumeration.yml
@@ -0,0 +1,23 @@
+
+services:
+    service_container:
+        class: Symfony\Component\DependencyInjection\ContainerInterface
+        public: true
+        synthetic: true
+    foo:
+        class: Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithDefaultEnumAttribute
+        public: true
+        autowire: true
+        arguments: { secondOptional: true }
+    Psr\Container\ContainerInterface:
+        alias: service_container
+        deprecated:
+            package: symfony/dependency-injection
+            version: 5.1
+            message: The "%alias_id%" autowiring alias is deprecated. Define it explicitly in your app if you want to keep using it.
+    Symfony\Component\DependencyInjection\ContainerInterface:
+        alias: service_container
+        deprecated:
+            package: symfony/dependency-injection
+            version: 5.1
+            message: The "%alias_id%" autowiring alias is deprecated. Define it explicitly in your app if you want to keep using it.
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_default_object.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_default_object.yml
@@ -0,0 +1,23 @@
+
+services:
+    service_container:
+        class: Symfony\Component\DependencyInjection\ContainerInterface
+        public: true
+        synthetic: true
+    foo:
+        class: Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithDefaultObjectAttribute
+        public: true
+        autowire: true
+        arguments: { secondOptional: true }
+    Psr\Container\ContainerInterface:
+        alias: service_container
+        deprecated:
+            package: symfony/dependency-injection
+            version: 5.1
+            message: The "%alias_id%" autowiring alias is deprecated. Define it explicitly in your app if you want to keep using it.
+    Symfony\Component\DependencyInjection\ContainerInterface:
+        alias: service_container
+        deprecated:
+            package: symfony/dependency-injection
+            version: 5.1
+            message: The "%alias_id%" autowiring alias is deprecated. Define it explicitly in your app if you want to keep using it.

Original file line number	Diff line number	Diff line change
`@@ -212,13 +212,16 @@ private function autowireCalls(\ReflectionClass $reflectionClass, bool $isRoot,`
`212`	`212`	`unset($arguments[$j]);`
`213`	`213`	`$arguments[$namedArguments[$j]] = $value;`
`214`	`214`	`}`
`215`		`- if ($namedArguments \|\| !$value instanceof $this->defaultArgument) {`
	`215`	`+ if (!$value instanceof $this->defaultArgument) {`
`216`	`216`	`continue;`
`217`	`217`	`}`
`218`	`218`
`219`	`219`	`if (\PHP_VERSION_ID >= 80100 && (\is_array($value->value) ? $value->value : \is_object($value->value))) {`
`220`		`- unset($arguments[$j]);`
`221`	`220`	`$namedArguments = $value->names;`
	`221`	`+ }`
	`222`	`+`
	`223`	`+ if ($namedArguments) {`
	`224`	`+ unset($arguments[$j]);`
`222`	`225`	`} else {`
`223`	`226`	`$arguments[$j] = $value->value;`
`224`	`227`	`}`