From 380cb10587727f0dc1b51fbecb79018c1d0b078e Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 1 Sep 2020 16:31:57 +0200 Subject: [PATCH] [DI] fix inlining of non-shared services --- .../Compiler/InlineServiceDefinitionsPass.php | 8 ++ .../DependencyInjection/Dumper/PhpDumper.php | 4 + .../Tests/Dumper/PhpDumperTest.php | 19 ++++ .../php/services_non_shared_duplicates.php | 88 +++++++++++++++++++ 4 files changed, 119 insertions(+) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_duplicates.php diff --git a/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php index ac3b4fe352..68baf3c089 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php @@ -30,6 +30,7 @@ class InlineServiceDefinitionsPass extends AbstractRecursivePass implements Repe private $connectedIds = []; private $notInlinedIds = []; private $inlinedIds = []; + private $notInlinableIds = []; private $graph; public function __construct(AnalyzeServiceReferencesPass $analyzingPass = null) @@ -99,6 +100,10 @@ class InlineServiceDefinitionsPass extends AbstractRecursivePass implements Repe } foreach ($remainingInlinedIds as $id) { + if (isset($this->notInlinableIds[$id])) { + continue; + } + $definition = $container->getDefinition($id); if (!$definition->isShared() && !$definition->isPublic()) { @@ -108,6 +113,7 @@ class InlineServiceDefinitionsPass extends AbstractRecursivePass implements Repe } finally { $this->container = null; $this->connectedIds = $this->notInlinedIds = $this->inlinedIds = []; + $this->notInlinableIds = []; $this->graph = null; } } @@ -138,6 +144,8 @@ class InlineServiceDefinitionsPass extends AbstractRecursivePass implements Repe $definition = $this->container->getDefinition($id); if (!$this->isInlineableDefinition($id, $definition)) { + $this->notInlinableIds[$id] = true; + return $value; } diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index d8b31dc18d..6bd5f8350a 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -885,6 +885,10 @@ EOF; return ''; } + if ($this->container->hasDefinition($targetId) && ($def = $this->container->getDefinition($targetId)) && !$def->isShared()) { + return ''; + } + $hasSelfRef = isset($this->circularReferences[$id][$targetId]) && !isset($this->definitionVariables[$definition]); if ($hasSelfRef && !$forConstructor && !$forConstructor = !$this->circularReferences[$id][$targetId]) { diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php index 58a186f009..de88e8b22f 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php @@ -19,6 +19,7 @@ use Symfony\Component\DependencyInjection\Argument\IteratorArgument; use Symfony\Component\DependencyInjection\Argument\RewindableGenerator; use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument; use Symfony\Component\DependencyInjection\Argument\ServiceLocator as ArgumentServiceLocator; +use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\Compiler\PassConfig; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -714,6 +715,24 @@ class PhpDumperTest extends TestCase $this->assertStringEqualsFile(self::$fixturesPath.'/php/services_non_shared_lazy.php', $dumper->dump()); } + public function testNonSharedDuplicates() + { + $container = new ContainerBuilder(); + $container->register('foo', 'stdClass')->setShared(false); + $container->register('baz', 'stdClass')->setPublic(true) + ->addArgument(new ServiceLocatorArgument(['foo' => new Reference('foo')])); + $container->register('bar', 'stdClass')->setPublic(true) + ->addArgument(new Reference('foo')) + ->addArgument(new Reference('foo')) + ; + $container->compile(); + + $dumper = new PhpDumper($container); + $dumper->setProxyDumper(new \DummyProxyDumper()); + + $this->assertStringEqualsFile(self::$fixturesPath.'/php/services_non_shared_duplicates.php', $dumper->dump()); + } + public function testInitializePropertiesBeforeMethodCalls() { require_once self::$fixturesPath.'/includes/classes.php'; diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_duplicates.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_duplicates.php new file mode 100644 index 0000000000..2ae7b4cea9 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_duplicates.php @@ -0,0 +1,88 @@ +getService = \Closure::fromCallable([$this, 'getService']); + $this->services = $this->privates = []; + $this->methodMap = [ + 'bar' => 'getBarService', + 'baz' => 'getBazService', + ]; + + $this->aliases = []; + } + + public function compile(): void + { + throw new LogicException('You cannot compile a dumped container that was already compiled.'); + } + + public function isCompiled(): bool + { + return true; + } + + public function getRemovedIds(): array + { + return [ + '.service_locator.BHJD0.a' => true, + 'Psr\\Container\\ContainerInterface' => true, + 'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true, + 'foo' => true, + ]; + } + + /** + * Gets the public 'bar' shared service. + * + * @return \stdClass + */ + protected function getBarService() + { + return $this->services['bar'] = new \stdClass((new \stdClass()), (new \stdClass())); + } + + /** + * Gets the public 'baz' shared service. + * + * @return \stdClass + */ + protected function getBazService() + { + return $this->services['baz'] = new \stdClass(new \Symfony\Component\DependencyInjection\Argument\ServiceLocator($this->getService, [ + 'foo' => [false, 'foo', 'getFooService', false], + ], [ + 'foo' => '?', + ])); + } + + /** + * Gets the private 'foo' service. + * + * @return \stdClass + */ + protected function getFooService() + { + return new \stdClass(); + } +}