symfony · nicolas-grekas · Nov 20, 2018 · Nov 18, 2018
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php
@@ -37,6 +37,7 @@ class AnalyzeServiceReferencesPass extends AbstractRecursivePass implements Repe
     private $hasProxyDumper;
     private $lazy;
     private $expressionLanguage;
+    private $byConstructor;
 
     /**
      * @param bool $onlyConstructorArguments Sets this Service Reference pass to ignore method calls
@@ -64,6 +65,7 @@ public function process(ContainerBuilder $container)
         $this->graph = $container->getCompiler()->getServiceReferenceGraph();
         $this->graph->clear();
         $this->lazy = false;
+        $this->byConstructor = false;
 
         foreach ($container->getAliases() as $id => $alias) {
             $targetId = $this->getDefinitionId((string) $alias);
@@ -100,7 +102,8 @@ protected function processValue($value, $isRoot = false)
                 $targetDefinition,
                 $value,
                 $this->lazy || ($this->hasProxyDumper && $targetDefinition && $targetDefinition->isLazy()),
-                ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $value->getInvalidBehavior()
+                ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $value->getInvalidBehavior(),
+                $this->byConstructor
             );
 
             return $value;
@@ -118,8 +121,11 @@ protected function processValue($value, $isRoot = false)
         }
         $this->lazy = false;
 
+        $byConstructor = $this->byConstructor;
+        $this->byConstructor = true;
         $this->processValue($value->getFactory());
         $this->processValue($value->getArguments());
+        $this->byConstructor = $byConstructor;
 
         if (!$this->onlyConstructorArguments) {
             $this->processValue($value->getProperties());

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ServiceReferenceGraph.php b/src/Symfony/Component/DependencyInjection/Compiler/ServiceReferenceGraph.php
@@ -91,19 +91,21 @@ public function clear()
      * @param string $reference
      * @param bool   $lazy
      * @param bool   $weak
+     * @param bool   $byConstructor
      */
-    public function connect($sourceId, $sourceValue, $destId, $destValue = null, $reference = null/*, bool $lazy = false, bool $weak = false*/)
+    public function connect($sourceId, $sourceValue, $destId, $destValue = null, $reference = null/*, bool $lazy = false, bool $weak = false, bool $byConstructor = false*/)
     {
         $lazy = \func_num_args() >= 6 ? func_get_arg(5) : false;
         $weak = \func_num_args() >= 7 ? func_get_arg(6) : false;
+        $byConstructor = \func_num_args() >= 8 ? func_get_arg(7) : false;
 
         if (null === $sourceId || null === $destId) {
             return;
         }
 
         $sourceNode = $this->createNode($sourceId, $sourceValue);
         $destNode = $this->createNode($destId, $destValue);
-        $edge = new ServiceReferenceGraphEdge($sourceNode, $destNode, $reference, $lazy, $weak);
+        $edge = new ServiceReferenceGraphEdge($sourceNode, $destNode, $reference, $lazy, $weak, $byConstructor);
 
         $sourceNode->addOutEdge($edge);
         $destNode->addInEdge($edge);

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ServiceReferenceGraphEdge.php b/src/Symfony/Component/DependencyInjection/Compiler/ServiceReferenceGraphEdge.php
@@ -25,21 +25,24 @@ class ServiceReferenceGraphEdge
     private $value;
     private $lazy;
     private $weak;
+    private $byConstructor;
 
     /**
      * @param ServiceReferenceGraphNode $sourceNode
      * @param ServiceReferenceGraphNode $destNode
      * @param mixed                     $value
      * @param bool                      $lazy
      * @param bool                      $weak
+     * @param bool                      $byConstructor
      */
-    public function __construct(ServiceReferenceGraphNode $sourceNode, ServiceReferenceGraphNode $destNode, $value = null, $lazy = false, $weak = false)
+    public function __construct(ServiceReferenceGraphNode $sourceNode, ServiceReferenceGraphNode $destNode, $value = null, $lazy = false, $weak = false, $byConstructor = false)
     {
         $this->sourceNode = $sourceNode;
         $this->destNode = $destNode;
         $this->value = $value;
         $this->lazy = $lazy;
         $this->weak = $weak;
+        $this->byConstructor = $byConstructor;
     }
 
     /**
@@ -91,4 +94,14 @@ public function isWeak()
     {
         return $this->weak;
     }
+
+    /**
+     * Returns true if the edge links with a constructor argument.
+     *
+     * @return bool
+     */
+    public function isReferencedByConstructor()
+    {
+        return $this->byConstructor;
+    }
 }
diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@@ -154,12 +154,16 @@ public function dump(array $options = array())
             }
         }
 
-        (new AnalyzeServiceReferencesPass(false))->process($this->container);
+        (new AnalyzeServiceReferencesPass(false, !$this->getProxyDumper() instanceof NullDumper))->process($this->container);
         $this->circularReferences = array();
-        $checkedNodes = array();
-        foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) {
-            $currentPath = array($id => $id);
-            $this->analyzeCircularReferences($node->getOutEdges(), $checkedNodes, $currentPath);
+        foreach (array(true, false) as $byConstructor) {
+            foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) {
+                if (!$node->getValue() instanceof Definition) {
+                    continue;
+                }
+                $currentPath = array($id => true);
+                $this->analyzeCircularReferences($node->getOutEdges(), $currentPath, $id, $byConstructor);
+            }
         }
         $this->container->getCompiler()->getServiceReferenceGraph()->clear();
 
@@ -297,27 +301,31 @@ private function getProxyDumper()
         return $this->proxyDumper;
     }
 
-    private function analyzeCircularReferences(array $edges, &$checkedNodes, &$currentPath)
+    private function analyzeCircularReferences(array $edges, &$currentPath, $sourceId, $byConstructor)
     {
         foreach ($edges as $edge) {
+            if ($byConstructor && !$edge->isReferencedByConstructor()) {
+                continue;
+            }
             $node = $edge->getDestNode();
             $id = $node->getId();
 
-            if ($node->getValue() && ($edge->isLazy() || $edge->isWeak())) {
+            if (!$node->getValue() instanceof Definition || $sourceId === $id || $edge->isLazy() || $edge->isWeak()) {
                 // no-op
             } elseif (isset($currentPath[$id])) {
                 $currentId = $id;
                 foreach (array_reverse($currentPath) as $parentId) {
-                    $this->circularReferences[$parentId][$currentId] = $currentId;
+                    if (!isset($this->circularReferences[$parentId][$currentId])) {
+                        $this->circularReferences[$parentId][$currentId] = $byConstructor;
+                    }
                     if ($parentId === $id) {
                         break;
                     }
                     $currentId = $parentId;
                 }
-            } elseif (!isset($checkedNodes[$id])) {
-                $checkedNodes[$id] = true;
+            } else {
                 $currentPath[$id] = $id;
-                $this->analyzeCircularReferences($node->getOutEdges(), $checkedNodes, $currentPath);
+                $this->analyzeCircularReferences($node->getOutEdges(), $currentPath, $id, $byConstructor);
                 unset($currentPath[$id]);
             }
         }
@@ -700,8 +708,14 @@ private function addInlineVariables($id, Definition $definition, array $argument
 
     private function addInlineReference($id, Definition $definition, $targetId, $forConstructor)
     {
+        list($callCount, $behavior) = $this->serviceCalls[$targetId];
+
+        while ($this->container->hasAlias($targetId)) {
+            $targetId = (string) $this->container->getAlias($targetId);
+        }
+
         if ($id === $targetId) {
-            return $this->addInlineService($id, $definition, $definition, $forConstructor);
+            return $this->addInlineService($id, $definition, $definition);
         }
 
         if ('service_container' === $targetId || isset($this->referenceVariables[$targetId])) {
@@ -710,9 +724,7 @@ private function addInlineReference($id, Definition $definition, $targetId, $for
 
         $hasSelfRef = isset($this->circularReferences[$id][$targetId]);
         $forConstructor = $forConstructor && !isset($this->definitionVariables[$definition]);
-        list($callCount, $behavior) = $this->serviceCalls[$targetId];
-
-        $code = $hasSelfRef && !$forConstructor ? $this->addInlineService($id, $definition, $definition, $forConstructor) : '';
+        $code = $hasSelfRef && $this->circularReferences[$id][$targetId] && !$forConstructor ? $this->addInlineService($id, $definition, $definition) : '';
 
         if (isset($this->referenceVariables[$targetId]) || (2 > $callCount && (!$hasSelfRef || !$forConstructor))) {
             return $code;

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_adawson.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_adawson.php
@@ -133,7 +133,13 @@ protected function getHandler1Service()
      */
     protected function getHandler2Service()
     {
-        return $this->services['App\Handler2'] = new \App\Handler2(${($_ = isset($this->services['App\Db']) ? $this->services['App\Db'] : $this->getDbService()) && false ?: '_'}, ${($_ = isset($this->services['App\Schema']) ? $this->services['App\Schema'] : $this->getSchemaService()) && false ?: '_'}, ${($_ = isset($this->services['App\Processor']) ? $this->services['App\Processor'] : $this->getProcessorService()) && false ?: '_'});
+        $a = ${($_ = isset($this->services['App\Processor']) ? $this->services['App\Processor'] : $this->getProcessorService()) && false ?: '_'};
+
+        if (isset($this->services['App\Handler2'])) {
+            return $this->services['App\Handler2'];
+        }
+
+        return $this->services['App\Handler2'] = new \App\Handler2(${($_ = isset($this->services['App\Db']) ? $this->services['App\Db'] : $this->getDbService()) && false ?: '_'}, ${($_ = isset($this->services['App\Schema']) ? $this->services['App\Schema'] : $this->getSchemaService()) && false ?: '_'}, $a);
     }
 
     /**

diff --git a/...fony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php b/...fony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php
@@ -174,11 +174,16 @@ protected function getBaz6Service()
      */
     protected function getConnectionService()<
DF2F
/span>
     {
-        $a = new \stdClass();
+        $a = ${($_ = isset($this->services['dispatcher']) ? $this->services['dispatcher'] : $this->getDispatcherService()) && false ?: '_'};
+
+        if (isset($this->services['connection'])) {
+            return $this->services['connection'];
+        }
+        $b = new \stdClass();
 
-        $this->services['connection'] = $instance = new \stdClass(${($_ = isset($this->services['dispatcher']) ? $this->services['dispatcher'] : $this->getDispatcherService()) && false ?: '_'}, $a);
+        $this->services['connection'] = $instance = new \stdClass($a, $b);
 
-        $a->logger = ${($_ = isset($this->services['logger']) ? $this->services['logger'] : $this->getLoggerService()) && false ?: '_'};
+        $b->logger = ${($_ = isset($this->services['logger']) ? $this->services['logger'] : $this->getLoggerService()) && false ?: '_'};
 
         return $instance;
     }
@@ -190,14 +195,19 @@ protected function getConnectionService()
      */
     protected function getConnection2Service()
     {
-        $a = new \stdClass();
+        $a = ${($_ = isset($this->services['dispatcher2']) ? $this->services['dispatcher2'] : $this->getDispatcher2Service()) && false ?: '_'};
 
-        $this->services['connection2'] = $instance = new \stdClass(${($_ = isset($this->services['dispatcher2']) ? $this->services['dispatcher2'] : $this->getDispatcher2Service()) && false ?: '_'}, $a);
+        if (isset($this->services['connection2'])) {
+            return $this->services['connection2'];
+        }
+        $b = new \stdClass();
 
-        $b = new \stdClass($instance);
-        $b->handler2 = new \stdClass(${($_ = isset($this->services['manager2']) ? $this->services['manager2'] : $this->getManager2Service()) && false ?: '_'});
+        $this->services['connection2'] = $instance = new \stdClass($a, $b);
 
-        $a->logger2 = $b;
+        $c = new \stdClass($instance);
+        $c->handler2 = new \stdClass(${($_ = isset($this->services['manager2']) ? $this->services['manager2'] : $this->getManager2Service()) && false ?: '_'});
+
+        $b->logger2 = $c;
 
         return $instance;
     }
@@ -431,7 +441,13 @@ protected function getRootService()
      */
     protected function getSubscriberService()
     {
-        return $this->services['subscriber'] = new \stdClass(${($_ = isset($this->services['manager']) ? $this->services['manager'] : $this->getManagerService()) && false ?: '_'});
+        $a = ${($_ = isset($this->services['manager']) ? $this->services['manager'] : $this->getManagerService()) && false ?: '_'};
+
+        if (isset($this->services['subscriber'])) {
+            return $this->services['subscriber'];
+        }
+
+        return $this->services['subscriber'] = new \stdClass($a);
     }
 
     /**