symfony · dunglas · Dec 30, 2017 · Jan 11, 2018
diff --git a/.php_cs.dist b/.php_cs.dist
@@ -40,5 +40,7 @@ return PhpCsFixer\Config::create()
            ->notPath('Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/default.phpt')
            ->notPath('Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/weak.phpt')
            ->notPath('Symfony/Component/Debug/Tests/DebugClassLoaderTest.php')
+            // invalid annotations on purpose
+            ->notPath('Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php')
    )
 ;
diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md
@@ -6,6 +6,7 @@ CHANGELOG

 * added support for variadics in named arguments
 * added PSR-11 `ContainerBagInterface` and its `ContainerBag` implementation to access parameters as-a-service
+ * added support for service's decorators autowiring

 4.0.0
 -----

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
@@ -32,6 +32,12 @@ class AutowirePass extends AbstractRecursivePass
    private $autowired = array();
    private $lastFailure;
    private $throwOnAutowiringException;
+    private $decoratedClass;
+    private $decoratedId;
+    private $methodCalls;
+    private $getPreviousValue;
+    private $decoratedMethodIndex;
+    private $decoratedMethodArgumentIndex;

    public function __construct(bool $throwOnAutowireException = true)
    {
@@ -49,6 +55,12 @@ public function process(ContainerBuilder $container)
            $this->types = null;
            $this->ambiguousServiceTypes = array();
            $this->autowired = array();
+            $this->decoratedClass = null;
+            $this->decoratedId = null;
+            $this->methodCalls = null;
+            $this->getPreviousValue = null;
+            $this->decoratedMethodIndex = null;
+            $this->decoratedMethodArgumentIndex = null;
        }
    }

@@ -89,7 +101,7 @@ private function doProcessValue($value, $isRoot = false)
            return $value;
        }

-        $methodCalls = $value->getMethodCalls();
+        $this->methodCalls = $value->getMethodCalls();

        try {
            $constructor = $this->getConstructor($value, false);
@@ -98,35 +110,42 @@ private function doProcessValue($value, $isRoot = false)
        }

        if ($constructor) {
-            array_unshift($methodCalls, array($constructor, $value->getArguments()));
+            array_unshift($this->methodCalls, array($constructor, $value->getArguments()));
        }

-        $methodCalls = $this->autowireCalls($reflectionClass, $methodCalls);
+        $this->methodCalls = $this->autowireCalls($reflectionClass, $isRoot);

        if ($constructor) {
-            list(, $arguments) = array_shift($methodCalls);
+            list(, $arguments) = array_shift($this->methodCalls);

            if ($arguments !== $value->getArguments()) {
                $value->setArguments($arguments);
            }
        }

-        if ($methodCalls !== $value->getMethodCalls()) {
-            $value->setMethodCalls($methodCalls);
+        if ($this->methodCalls !== $value->getMethodCalls()) {
+            $value->setMethodCalls($this->methodCalls);
        }

        return $value;
    }

    /**
     * @param \ReflectionClass $reflectionClass
-     * @param array            $methodCalls
     *
     * @return array
     */
-    private function autowireCalls(\ReflectionClass $reflectionClass, array $methodCalls)
+    private function autowireCalls(\ReflectionClass $reflectionClass, bool $isRoot): array
    {
-        foreach ($methodCalls as $i => $call) {
+        if ($isRoot && ($definition = $this->container->getDefinition($this->currentId)) && $this->container->has($this->decoratedId = $definition->innerServiceId)) {
+            $this->decoratedClass = $this->container->findDefinition($this->decoratedId)->getClass();
+        } else {
+            $this->decoratedId = null;
+            $this->decoratedClass = null;
+        }
+
+        foreach ($this->methodCalls as $i => $call) {
+            $this->decoratedMethodIndex = $i;
            list($method, $arguments) = $call;

            if ($method instanceof \ReflectionFunctionAbstract) {
@@ -138,11 +157,11 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC
            $arguments = $this->autowireMethod($reflectionMethod, $arguments);

            if ($arguments !== $call[1]) {
-                $methodCalls[$i][1] = $arguments;
+                $this->methodCalls[$i][1] = $arguments;
            }
        }

-        return $methodCalls;
+        return $this->methodCalls;
    }

    /**
@@ -190,18 +209,40 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a
                continue;
            }

-            if (!$value = $this->getAutowiredReference($ref = new TypedReference($type, $type, !$parameter->isOptional() ? $class : ''), 'for '.sprintf('argument "$%s" of method "%s()"', $parameter->name, $class.'::'.$method))) {
-                $failureMessage = $this->createTypeNotFoundMessage($ref, sprintf('argument "$%s" of method "%s()"', $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
+            $getValue = function () use ($type, $parameter, $class, $method) {
+                if (!$value = $this->getAutowiredReference($ref = new TypedReference($type, $type, !$parameter->isOptional() ? $class : ''), 'for '.sprintf('argument "$%s" of method "%s()"', $parameter->name, $class.'::'.$method))) {
+                    $failureMessage = $this->createTypeNotFoundMessage($ref, sprintf('argument "$%s" of method "%s()"', $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
+
+                    if ($parameter->isDefaultValueAvailable()) {
+                        $value = $parameter->getDefaultValue();
+                    } elseif (!$parameter->allowsNull()) {
+                        throw new AutowiringFailedException($this->currentId, $failureMessage);
+                    }
+                    $this->container->log($this, $failureMessage);
+                }
+
+                return $value;
+            };
+
+            if ($this->decoratedClass && $isDecorated = is_a($this->decoratedClass, $type, true)) {
+                if ($this->getPreviousValue) {
+                    // The inner service is injected only if there is only 1 argument matching the type of the decorated class
+                    // across all arguments of all autowired methods.
+                    // If a second matching argument is found, the default behavior is restored.

-                if ($parameter->isDefaultValueAvailable()) {
-                    $value = $parameter->getDefaultValue();
-                } elseif (!$parameter->allowsNull()) {
-                    throw new AutowiringFailedException($this->currentId, $failureMessage);
+                    $getPreviousValue = $this->getPreviousValue;
+                    $this->methodCalls[$this->decoratedMethodIndex][1][$this->decoratedMethodArgumentIndex] = $getPreviousValue();
+                    $this->decoratedClass = null; // Prevent further checks
+                } else {
+                    $arguments[$index] = new TypedReference($this->decoratedId, $this->decoratedClass);
+                    $this->getPreviousValue = $getValue;
+                    $this->decoratedMethodArgumentIndex = $index;
+
+                    continue;
                }
-                $this->container->log($this, $failureMessage);
            }

-            $arguments[$index] = $value;
+            $arguments[$index] = $getValue();
        }

        if ($parameters && !isset($arguments[++$index])) {

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/DecoratorServicePass.php b/src/Symfony/Component/DependencyInjection/Compiler/DecoratorServicePass.php
@@ -43,6 +43,7 @@ public function process(ContainerBuilder $container)
            if (!$renamedId) {
                $renamedId = $id.'.inner';
            }
+            $definition->innerServiceId = $renamedId;

            // we create a new alias/service for the service we are replacing
            // to be able to reference it in the new one

diff --git a/src/Symfony/Component/DependencyInjection/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php
@@ -49,6 +49,13 @@ class Definition

    private static $defaultDeprecationTemplate = 'The "%service_id%" service is deprecated. You should stop using it, as it will soon be removed.';

+    /**
+     * @internal
+     *
+     * Used to store the name of the inner id when using service decoration together with autowiring
+     */
+    public $innerServiceId;
+
    /**
     * @param string|null $class     The service class
     * @param array       $arguments An array of arguments to pass to the service constructor

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
@@ -12,9 +12,12 @@
 namespace Symfony\Component\DependencyInjection\Tests\Compiler;

 use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\Compiler\AutowireRequiredMethodsPass;
 use Symfony\Component\DependencyInjection\Compiler\AutowirePass;
+use Symfony\Component\DependencyInjection\Compiler\DecoratorServicePass;
 use Symfony\Component\DependencyInjection\Compiler\ResolveClassPass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Exception\RuntimeException;
@@ -787,4 +790,59 @@ public function testInlineServicesAreNotCandidates()

        $this->assertSame(array(), $container->getDefinition('autowired')->getArguments());
    }
+
+    public function testAutowireDecorator()
+    {
+        $container = new ContainerBuilder();
+        $container->register(LoggerInterface::class, NullLogger::class);
+        $container->register(Decorated::class, Decorated::class);
+        $container
+            ->register(Decorator::class, Decorator::class)
+            ->setDecoratedService(Decorated::class)
+            ->setAutowired(true)
+        ;
+
+        (new DecoratorServicePass())->process($container);
+        (new AutowirePass())->process($container);
+
+        $definition = $container->getDefinition(Decorator::class);
+        $this->assertSame(Decorator::class.'.inner', (string) $definition->getArgument(1));
+    }
+
+    public function testAutowireDecoratorRenamedId()
+    {
+        $container = new ContainerBuilder();
+        $container->register(LoggerInterface::class, NullLogger::class);
+        $container->register(Decorated::class, Decorated::class);
+        $container
+            ->register(Decorator::class, Decorator::class)
+            ->setDecoratedService(Decorated::class, 'renamed')
+            ->setAutowired(true)
+        ;
+
+        (new DecoratorServicePass())->process($container);
+        (new AutowirePass())->process($container);
+
+        $definition = $container->getDefinition(Decorator::class);
+        $this->assertSame('renamed', (string) $definition->getArgument(1));
+    }
+
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\AutowiringFailedException
+     * @expectedExceptionMessage 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". Did you create a class that implements this interface?
+     */
+    public function testDoNotAutowireDecoratorWhenSeveralArgumentOfTheType()
+    {
+        $container = new ContainerBuilder();
+        $container->register(LoggerInterface::class, NullLogger::class);
+        $container->register(Decorated::class, Decorated::class);
+        $container
+            ->register(NonAutowirableDecorator::class, NonAutowirableDecorator::class)
+            ->setDecoratedService(Decorated::class)
+            ->setAutowired(true)
+        ;
+
+        (new DecoratorServicePass())->process($container);
+        (new AutowirePass())->process($container);
+    }
 }
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php
@@ -2,6 +2,8 @@

 namespace Symfony\Component\DependencyInjection\Tests\Compiler;

+use Psr\Log\LoggerInterface;
+
 class Foo
 {
 }
@@ -352,3 +354,28 @@ public function setDefaultLocale($defaultLocale)
    {
    }
 }
+
+interface DecoratorInterface
+{
+}
+
+class Decorated implements DecoratorInterface
+{
+    public function __construct($quz = null, \NonExistent $nonExistent = null, DecoratorInterface $decorated = null, array $foo = array())
+    {
+    }
+}
+
+class Decorator implements DecoratorInterface
+{
+    public function __construct(LoggerInterface $logger, DecoratorInterface $decorated)
+    {
+    }
+}
+
+class NonAutowirableDecorator implements DecoratorInterface
+{
+    public function __construct(LoggerInterface $logger, DecoratorInterface $decorated1, DecoratorInterface $decorated2)
+    {
+    }
+}
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php
@@ -57,8 +57,8 @@ public function getRemovedIds()
            'Psr\\Container\\ContainerInterface' => true,
            'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
            'Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CustomDefinition' => true,
-            'service_locator.MtGsMEd' => true,
-            'service_locator.MtGsMEd.foo_service' => true,
+            'service_locator.KT3jhJ7' => true,
+            'service_locator.KT3jhJ7.foo_service' => true,
        );
    }