diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php index e734aebb27626..7c22d3c088438 100644 --- 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 index 4d36ae7678f15..83486f0533714 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ServiceReferenceGraph.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ServiceReferenceGraph.php @@ -91,11 +91,13 @@ 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; @@ -103,7 +105,7 @@ public function connect($sourceId, $sourceValue, $destId, $destValue = null, $re $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 index 018905394f0cf..5b8c84b6d61f6 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ServiceReferenceGraphEdge.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ServiceReferenceGraphEdge.php @@ -25,6 +25,7 @@ class ServiceReferenceGraphEdge private $value; private $lazy; private $weak; + private $byConstructor; /** * @param ServiceReferenceGraphNode $sourceNode @@ -32,14 +33,16 @@ class ServiceReferenceGraphEdge * @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 index 416f954cfd3c8..57b9e698e6aa8 100644 --- 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 index 8e9e65ad98a35..37b95567ce880 100644 --- 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/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php index 0c9228e64c704..bd08c4d15f67b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php @@ -174,11 +174,16 @@ protected function getBaz6Service() */ protected function getConnectionService() { - $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); } /**