[DI] Fix dumping Doctrine-like service graphs (bis)

symfony · Jul 28, 2019 · 53c9316 · 53c9316
1 parent ee49144
commit 53c9316
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 12 deletions.
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php
@@ -122,7 +122,7 @@ protected function processValue($value, $isRoot = false)
         $this->lazy = false;
 
         $byConstructor = $this->byConstructor;
-        $this->byConstructor = true;
+        $this->byConstructor = $isRoot || $byConstructor;
         $this->processValue($value->getFactory());
         $this->processValue($value->getArguments());
         $this->byConstructor = $byConstructor;

diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@@ -157,15 +157,27 @@ public function dump(array $options = [])
         (new AnalyzeServiceReferencesPass(false, !$this->getProxyDumper() instanceof NullDumper))->process($this->container);
         $checkedNodes = [];
         $this->circularReferences = [];
-        foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) {
+        $graph = $this->container->getCompiler()->getServiceReferenceGraph();
+        foreach ($graph->getNodes() as $id => $node) {
             if (!$node->getValue() instanceof Definition) {
                 continue;
             }
             if (!isset($checkedNodes[$id])) {
                 $this->analyzeCircularReferences($id, $node->getOutEdges(), $checkedNodes);
             }
         }
-        $this->container->getCompiler()->getServiceReferenceGraph()->clear();
+
+        foreach ($this->circularReferences as $parentId => $targetIds) {
+            foreach ($graph->getNode($parentId)->getOutEdges() as $edge) {
+                if (!isset($targetIds[$id = $edge->getDestNode()->getId()])) {
+                    continue;
+                }
+                if ($this->circularReferences[$parentId][$id] = $edge->isReferencedByConstructor()) {
+                    unset($targetIds[$id]);
+                }
+            }
+        }
+        $graph->clear();
         $checkedNodes = [];
 
         $this->docStar = $options['debug'] ? '*' : '';
@@ -661,7 +673,6 @@ private function addService($id, Definition $definition, &$file = null)
         $autowired = $definition->isAutowired() ? ' autowired' : '';
 
         if ($definition->isLazy()) {
-            unset($this->circularReferences[$id]);
             $lazyInitialization = '$lazyLoad = true';
         } else {
             $lazyInitialization = '';
@@ -736,12 +747,12 @@ 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);
         }
 
+        list($callCount, $behavior) = $this->serviceCalls[$targetId];
+
         if ($id === $targetId) {
             return $this->addInlineService($id, $definition, $definition);
         }
@@ -751,7 +762,7 @@ private function addInlineReference($id, Definition $definition, $targetId, $for
         }
 
         $hasSelfRef = isset($this->circularReferences[$id][$targetId]);
-        $forConstructor = $forConstructor && !isset($this->definitionVariables[$definition]);
+        $forConstructor = $forConstructor && !isset($this->definitionVariables[$definition]) && !empty($this->circularReferences[$id][$targetId]);
         $code = $hasSelfRef && !$forConstructor ? $this->addInlineService($id, $definition, $definition) : '';
 
         if (isset($this->referenceVariables[$targetId]) || (2 > $callCount && (!$hasSelfRef || !$forConstructor))) {
@@ -1562,6 +1573,10 @@ private function getDefinitionsFromArguments(array $arguments, \SplObjectStorage
             } elseif ($argument instanceof Reference) {
                 $id = $this->container->normalizeId($argument);
 
+                while ($this->container->hasAlias($id)) {
+                    $id = (string) $this->container->getAlias($id);
+                }
+
                 if (!isset($calls[$id])) {
                     $calls[$id] = [0, $argument->getInvalidBehavior()];
                 } else {
@@ -1716,7 +1731,12 @@ private function dumpValue($value, $interpolate = true)
         } elseif ($value instanceof Variable) {
             return '$'.$value;
         } elseif ($value instanceof Reference) {
-            $id = $this->container->normalizeId($value);
+            $id = $this->container->normalizeId($id);
+
+            while ($this->container->hasAlias($id)) {
+                $id = (string) $this->container->getAlias($id);
+            }
+
             if (null !== $this->referenceVariables && isset($this->referenceVariables[$id])) {
                 return $this->dumpValue($this->referenceVariables[$id], $interpolate);
             }

diff --git a/...ony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_private.php b/...ony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_private.php
@@ -370,9 +370,12 @@ protected function getManager2Service()
     protected function getManager3Service($lazyLoad = true)
     {
         $a = new \stdClass();
+
+        $this->services['manager3'] = $instance = new \stdClass($a);
+
         $a->listener = [0 => ${($_ = isset($this->services['listener3']) ? $this->services['listener3'] : $this->getListener3Service()) && false ?: '_'}];
 
-        return $this->services['manager3'] = new \stdClass($a);
+        return $instance;
     }
 
     /**
@@ -489,9 +492,12 @@ protected function getLevel6Service()
     protected function getManager4Service($lazyLoad = true)
     {
         $a = new \stdClass();
+
+        $this->services['manager4'] = $instance = new \stdClass($a);
+
         $a->listener = [0 => ${($_ = isset($this->services['listener4']) ? $this->services['listener4'] : $this->getListener4Service()) && false ?: '_'}];
 
-        return $this->services['manager4'] = new \stdClass($a);
+        return $instance;
     }
 
     /**

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
@@ -497,7 +497,13 @@ protected function getManager2Service()
      */
     protected function getManager3Service($lazyLoad = true)
     {
-        return $this->services['manager3'] = new \stdClass(${($_ = isset($this->services['connection3']) ? $this->services['connection3'] : $this->getConnection3Service()) && false ?: '_'});
+        $a = ${($_ = isset($this->services['connection3']) ? $this->services['connection3'] : $this->getConnection3Service()) && false ?: '_'};
+
+        if (isset($this->services['manager3'])) {
+            return $this->services['manager3'];
+        }
+
+        return $this->services['manager3'] = new \stdClass($a);
     }
 
     /**
@@ -613,7 +619,13 @@ protected function getLevel6Service()
      */
     protected function getManager4Service($lazyLoad = true)
     {
-        return $this->services['manager4'] = new \stdClass(${($_ = isset($this->services['connection4']) ? $this->services['connection4'] : $this->getConnection4Service()) && false ?: '_'});
+        $a = ${($_ = isset($this->services['connection4']) ? $this->services['connection4'] : $this->getConnection4Service()) && false ?: '_'};
+
+        if (isset($this->services['manager4'])) {
+            return $this->services['manager4'];
+        }
+
+        return $this->services['manager4'] = new \stdClass($a);
     }
 
     /**