diff --git a/.php_cs.dist b/.php_cs.dist index c001103399..e6def5bc44 100644 --- 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 index 3ba2fd6347..d0f844df4f 100644 --- 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 index cd6ad87218..65da19bd8f 100644 --- 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 @@ class AutowirePass extends AbstractRecursivePass $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 @@ class AutowirePass extends AbstractRecursivePass return $value; } - $methodCalls = $value->getMethodCalls(); + $this->methodCalls = $value->getMethodCalls(); try { $constructor = $this->getConstructor($value, false); @@ -98,21 +110,21 @@ class AutowirePass extends AbstractRecursivePass } 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; @@ -120,13 +132,20 @@ class AutowirePass extends AbstractRecursivePass /** * @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 @@ class AutowirePass extends AbstractRecursivePass $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 @@ class AutowirePass extends AbstractRecursivePass 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); + 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. + + $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 index f36293c6cf..c9cde471b6 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/DecoratorServicePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/DecoratorServicePass.php @@ -43,6 +43,7 @@ class DecoratorServicePass implements CompilerPassInterface 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 index 9865adbd7a..6f2e94bf5b 100644 --- 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 index fb07f47932..61fc4f33ba 100644 --- 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 @@ class AutowirePassTest extends TestCase $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 index 04c17ee188..23075d1d12 100644 --- 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 @@ class ScalarSetter { } } + +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 index 4f41330edb..11b935cc61 100644 --- 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 @@ class ProjectServiceContainer extends Container '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, ); }