[DI] fix inlining of non-shared services

symfony · nicolas-grekas · Sep 1, 2020 · Sep 1, 2020 · Sep 1, 2020 · 380cb10587727f0dc1b51fbecb79018c1d0b078e
commit 380cb10587727f0dc1b51fbecb79018c1d0b078e
diff --git 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 @@ public function process(ContainerBuilder $container)
             }
 
             foreach ($remainingInlinedIds as $id) {
+                if (isset($this->notInlinableIds[$id])) {
+                    continue;
+                }
+
                 $definition = $container->getDefinition($id);
 
                 if (!$definition->isShared() && !$definition->isPublic()) {
@@ -108,6 +113,7 @@ public function process(ContainerBuilder $container)
         } finally {
             $this->container = null;
             $this->connectedIds = $this->notInlinedIds = $this->inlinedIds = [];
+            $this->notInlinableIds = [];
             $this->graph = null;
         }
     }
@@ -138,6 +144,8 @@ protected function processValue($value, $isRoot = false)
         $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
@@ -885,6 +885,10 @@ private function addInlineReference(string $id, Definition $definition, string $
             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
@@ -19,6 +19,7 @@
 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 @@ public function testNonSharedLazyDefinitionReferences()
         $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/...mfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_duplicates.php b/...mfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_duplicates.php
@@ -0,0 +1,88 @@
+<?php
+
+use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Symfony\Component\DependencyInjection\Container;
+use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
+use Symfony\Component\DependencyInjection\Exception\LogicException;
+use Symfony\Component\DependencyInjection\Exception\RuntimeException;
+use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag;
+use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface;
+
+/**
+ * This class has been auto-generated
+ * by the Symfony Dependency Injection Component.
+ *
+ * @final
+ */
+class ProjectServiceContainer extends Container
+{
+    private $parameters = [];
+    private $getService;
+
+    public function __construct()
+    {
+        $this->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();
+    }
+}