bug #43579 [DependencyInjection] Fix autowiring tagged arguments from attributes (Okhoshi)

nicolas-grekas · nicolas-grekas · commit 97c32a13d043 · 2021-10-26T10:01:46.000+02:00
This PR was merged into the 5.3 branch. Discussion ---------- [DependencyInjection] Fix autowiring tagged arguments from attributes | Q | A | ------------- | --- | Branch? | 5.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #43272 | License | MIT | Doc PR | no Reimplement #40406 with `AttributeAutoconfigurationPass` to avoid the BC following the change in `CompilerPass` ordering in `PassConfig`. Also revert the various fix made in an attempt to recover the BC introduced by #40406. ~Note: `5.4` branch was needed because `AttributeAutoconfigurationPass` is not handling parameters in 5.3~ To-do: - [x] Add a test to cover the breakage presented in #43272 Commits ------- 4c7566f [DependencyInjection] Fix autowiring tagged arguments from attributes
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
@@ -159,10 +159,8 @@ private function autowireCalls(\ReflectionClass $reflectionClass, bool $isRoot,
         $this->decoratedClass = null;
         $this->getPreviousValue = null;
 
-        if ($isRoot && ($definition = $this->container->getDefinition($this->currentId)) && ($decoratedDefinition = $definition->getDecoratedService()) && null !== ($innerId = $decoratedDefinition[0]) && $this->container->has($innerId)) {
-            // If the class references to itself and is decorated, provide the inner service id and class to not get a circular reference
-            $this->decoratedClass = $this->container->findDefinition($innerId)->getClass();
-            $this->decoratedId = $decoratedDefinition[1] ?? $this->currentId.'.inner';
+        if ($isRoot && ($definition = $this->container->getDefinition($this->currentId)) && null !== ($this->decoratedId = $definition->innerServiceId) && $this->container->has($this->decoratedId)) {
+            $this->decoratedClass = $this->container->findDefinition($this->decoratedId)->getClass();
         }
 
         $patchedIndexes = [];
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php
@@ -62,10 +62,11 @@ public function __construct()
             new AutowireRequiredMethodsPass(),
             new AutowireRequiredPropertiesPass(),
             new ResolveBindingsPass(),
+            new ServiceLocatorTagPass(),
+            new DecoratorServicePass(),
             new CheckDefinitionValidityPass(),
             new AutowirePass(false),
             new ServiceLocatorTagPass(),
-            new DecoratorServicePass(),
             new ResolveTaggedIteratorArgumentPass(),
             new ResolveServiceSubscribersPass(),
             new ResolveReferencesToAliasesPass(),
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
@@ -985,8 +985,8 @@ public function testAutowireDecorator()
             ->setAutowired(true)
         ;
 
-        (new AutowirePass())->process($container);
         (new DecoratorServicePass())->process($container);
+        (new AutowirePass())->process($container);
 
         $definition = $container->getDefinition(Decorator::class);
         $this->assertSame(Decorator::class.'.inner', (string) $definition->getArgument(1));
@@ -1008,8 +1008,8 @@ public function testAutowireDecoratorChain()
             ->setAutowired(true)
         ;
 
-        (new AutowirePass())->process($container);
         (new DecoratorServicePass())->process($container);
+        (new AutowirePass())->process($container);
 
         $definition = $container->getDefinition(DecoratedDecorator::class);
         $this->assertSame(DecoratedDecorator::class.'.inner', (string) $definition->getArgument(0));
@@ -1026,8 +1026,8 @@ public function testAutowireDecoratorRenamedId()
             ->setAutowired(true)
         ;
 
-        (new AutowirePass())->process($container);
         (new DecoratorServicePass())->process($container);
+        (new AutowirePass())->process($container);
 
         $definition = $container->getDefinition(Decorator::class);
         $this->assertSame('renamed', (string) $definition->getArgument(1));
@@ -1044,11 +1044,12 @@ public function testDoNotAutowireDecoratorWhenSeveralArgumentOfTheType()
             ->setAutowired(true)
         ;
 
+        (new DecoratorServicePass())->process($container);
         try {
             (new AutowirePass())->process($container);
             $this->fail('AutowirePass should have thrown an exception');
         } catch (AutowiringFailedException $e) {
-            $this->assertSame('Cannot autowire service "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator": argument "$decorated1" of method "__construct()" references interface "Symfony\Component\DependencyInjection\Tests\Compiler\DecoratorInterface" but no such service exists. You should maybe alias this interface to one of these existing services: "Symfony\Component\DependencyInjection\Tests\Compiler\Decorated", "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator".', (string) $e->getMessage());
+            $this->assertSame('Cannot autowire service "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator": argument "$decorated1" of method "__construct()" references interface "Symfony\Component\DependencyInjection\Tests\Compiler\DecoratorInterface" but no such service exists. You should maybe alias this interface to one of these existing services: "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator", "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator.inner".', (string) $e->getMessage());
         }
     }
 
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php
@@ -175,6 +175,33 @@ public function testCanDecorateServiceLocator()
         $this->assertSame($container->get('foo'), $container->get(DecoratedServiceLocator::class)->get('foo'));
     }
 
+    public function testAliasDecoratedService()
+    {
+        $container = new ContainerBuilder();
+
+        $container->register('service', ServiceLocator::class)
+            ->setPublic(true)
+            ->setArguments([[]])
+        ;
+        $container->register('decorator', DecoratedServiceLocator::class)
+            ->setDecoratedService('service')
+            ->setAutowired(true)
+            ->setPublic(true)
+        ;
+        $container->setAlias(ServiceLocator::class, 'decorator.inner')
+            ->setPublic(true)
+        ;
+        $container->register('user_service', DecoratedServiceLocator::class)
+            ->setAutowired(true)
+        ;
+
+        $container->compile();
+
+        $this->assertInstanceOf(DecoratedServiceLocator::class, $container->get('service'));
+        $this->assertInstanceOf(ServiceLocator::class, $container->get(ServiceLocator::class));
+        $this->assertSame($container->get('service'), $container->get('decorator'));
+    }
+
     /**
      * @dataProvider getYamlCompileTests
      */

Original file line number	Diff line number	Diff line change
`@@ -985,8 +985,8 @@ public function testAutowireDecorator()`
`985`	`985`	`->setAutowired(true)`
`986`	`986`	`;`
`987`	`987`
`988`		`- (new AutowirePass())->process($container);`
`989`	`988`	`(new DecoratorServicePass())->process($container);`
	`989`	`+ (new AutowirePass())->process($container);`
`990`	`990`
`991`	`991`	`$definition = $container->getDefinition(Decorator::class);`
`992`	`992`	`$this->assertSame(Decorator::class.'.inner', (string) $definition->getArgument(1));`
`@@ -1008,8 +1008,8 @@ public function testAutowireDecoratorChain()`
`1008`	`1008`	`->setAutowired(true)`
`1009`	`1009`	`;`
`1010`	`1010`
`1011`		`- (new AutowirePass())->process($container);`
`1012`	`1011`	`(new DecoratorServicePass())->process($container);`
	`1012`	`+ (new AutowirePass())->process($container);`
`1013`	`1013`
`1014`	`1014`	`$definition = $container->getDefinition(DecoratedDecorator::class);`
`1015`	`1015`	`$this->assertSame(DecoratedDecorator::class.'.inner', (string) $definition->getArgument(0));`
`@@ -1026,8 +1026,8 @@ public function testAutowireDecoratorRenamedId()`
`1026`	`1026`	`->setAutowired(true)`
`1027`	`1027`	`;`
`1028`	`1028`
`1029`		`- (new AutowirePass())->process($container);`
`1030`	`1029`	`(new DecoratorServicePass())->process($container);`
	`1030`	`+ (new AutowirePass())->process($container);`
`1031`	`1031`
`1032`	`1032`	`$definition = $container->getDefinition(Decorator::class);`
`1033`	`1033`	`$this->assertSame('renamed', (string) $definition->getArgument(1));`
`@@ -1044,11 +1044,12 @@ public function testDoNotAutowireDecoratorWhenSeveralArgumentOfTheType()`
`1044`	`1044`	`->setAutowired(true)`
`1045`	`1045`	`;`
`1046`	`1046`
	`1047`	`+ (new DecoratorServicePass())->process($container);`
`1047`	`1048`	`try {`
`1048`	`1049`	`(new AutowirePass())->process($container);`
`1049`	`1050`	`$this->fail('AutowirePass should have thrown an exception');`
`1050`	`1051`	`} catch (AutowiringFailedException $e) {`
`1051`		- $this->assertSame('Cannot autowire service "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator": argument "$decorated1" of method "__construct()" references interface "Symfony\Component\DependencyInjection\Tests\Compiler\DecoratorInterface" but no such service exists. You should maybe alias this interface to one of these existing services: "Symfony\Component\DependencyInjection\Tests\Compiler\Decorated", "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator".', (string) $e->getMessage());
	`1052`	+ $this->assertSame('Cannot autowire service "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator": argument "$decorated1" of method "__construct()" references interface "Symfony\Component\DependencyInjection\Tests\Compiler\DecoratorInterface" but no such service exists. You should maybe alias this interface to one of these existing services: "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator", "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator.inner".', (string) $e->getMessage());
`1052`	`1053`	`}`
`1053`	`1054`	`}`
`1054`	`1055`