[DI] Analyze setter-circular deps more precisely

This commit is contained in:
Nicolas Grekas 2017-11-20 15:27:11 +01:00
parent 1b6597d19c
commit 9cc4a213c9
9 changed files with 73 additions and 143 deletions

View File

@ -59,7 +59,7 @@ class CheckCircularReferencesPass implements CompilerPassInterface
if (empty($this->checkedNodes[$id])) {
// Don't check circular references for lazy edges
if (!$node->getValue() || !$edge->isLazy()) {
if (!$node->getValue() || (!$edge->isLazy() && !$edge->isWeak())) {
$searchKey = array_search($id, $this->currentPath);
$this->currentPath[] = $id;

View File

@ -16,6 +16,7 @@ use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
use Symfony\Component\DependencyInjection\Variable;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Compiler\AnalyzeServiceReferencesPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\DependencyInjection\ContainerInterface;
@ -66,6 +67,7 @@ class PhpDumper extends Dumper
private $hotPathTag;
private $inlineRequires;
private $inlinedRequires = array();
private $circularReferences = array();
/**
* @var ProxyDumper
@ -133,6 +135,14 @@ class PhpDumper extends Dumper
$this->initializeMethodNamesMap('Container' === $baseClass ? Container::class : $baseClass);
(new AnalyzeServiceReferencesPass())->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);
}
$this->docStar = $options['debug'] ? '*' : '';
if (!empty($options['file']) && is_dir($dir = dirname($options['file']))) {
@ -228,6 +238,7 @@ EOF;
$this->targetDirRegex = null;
$this->inlinedRequires = array();
$this->circularReferences = array();
$unusedEnvs = array();
foreach ($this->container->getEnvCounters() as $env => $use) {
@ -272,18 +283,18 @@ EOF;
array_unshift($inlinedDefinitions, $definition);
$collectLineage = $this->inlineRequires && !$this->isHotPath($definition);
$isNonLazyShared = !$this->getProxyDumper()->isProxyCandidate($definition) && $definition->isShared();
$isNonLazyShared = isset($this->circularReferences[$cId]) && !$this->getProxyDumper()->isProxyCandidate($definition) && $definition->isShared();
$lineage = $calls = $behavior = array();
foreach ($inlinedDefinitions as $iDefinition) {
if ($collectLineage && !$iDefinition->isDeprecated() && $class = is_array($factory = $iDefinition->getFactory()) && is_string($factory[0]) ? $factory[0] : $iDefinition->getClass()) {
$this->collectLineage($class, $lineage);
}
$this->getServiceCallsFromArguments($iDefinition->getArguments(), $calls, $behavior, $isNonLazyShared);
$this->getServiceCallsFromArguments($iDefinition->getArguments(), $calls, $behavior, $isNonLazyShared, $cId);
$isPreInstantiation = $isNonLazyShared && $iDefinition !== $definition && !$this->hasReference($cId, $iDefinition->getMethodCalls(), true) && !$this->hasReference($cId, $iDefinition->getProperties(), true);
$this->getServiceCallsFromArguments($iDefinition->getMethodCalls(), $calls, $behavior, $isPreInstantiation);
$this->getServiceCallsFromArguments($iDefinition->getProperties(), $calls, $behavior, $isPreInstantiation);
$this->getServiceCallsFromArguments(array($iDefinition->getConfigurator()), $calls, $behavior, $isPreInstantiation);
$this->getServiceCallsFromArguments(array($iDefinition->getFactory()), $calls, $behavior, $isNonLazyShared);
$this->getServiceCallsFromArguments($iDefinition->getMethodCalls(), $calls, $behavior, $isPreInstantiation, $cId);
$this->getServiceCallsFromArguments($iDefinition->getProperties(), $calls, $behavior, $isPreInstantiation, $cId);
$this->getServiceCallsFromArguments(array($iDefinition->getConfigurator()), $calls, $behavior, $isPreInstantiation, $cId);
$this->getServiceCallsFromArguments(array($iDefinition->getFactory()), $calls, $behavior, $isNonLazyShared, $cId);
}
$code = '';
@ -336,6 +347,33 @@ EOTXT;
return $code;
}
private function analyzeCircularReferences(array $edges, &$checkedNodes, &$currentPath)
{
foreach ($edges as $edge) {
$node = $edge->getDestNode();
$id = $node->getId();
if (isset($checkedNodes[$id])) {
continue;
}
if ($node->getValue() && ($edge->isLazy() || $edge->isWeak())) {
// no-op
} elseif (isset($currentPath[$id])) {
foreach (array_reverse($currentPath) as $parentId) {
$this->circularReferences[$parentId][$id] = $id;
$id = $parentId;
}
} else {
$currentPath[$id] = $id;
$this->analyzeCircularReferences($node->getOutEdges(), $checkedNodes, $currentPath);
}
$checkedNodes[$id] = true;
array_pop($currentPath);
}
}
private function collectLineage($class, array &$lineage)
{
if (isset($lineage[$class])) {
@ -562,7 +600,7 @@ EOTXT;
}
foreach ($definition->getArguments() as $arg) {
if (!$arg || ($arg instanceof Reference && 'service_container' === (string) $arg)) {
if (!$arg) {
continue;
}
if (is_array($arg) && 3 >= count($arg)) {
@ -570,7 +608,7 @@ EOTXT;
if ($this->dumpValue($k) !== $this->dumpValue($k, false)) {
return false;
}
if (!$v || ($v instanceof Reference && 'service_container' === (string) $v)) {
if (!$v) {
continue;
}
if ($v instanceof Reference && $this->container->has($id = (string) $v) && $this->container->findDefinition($id)->isSynthetic()) {
@ -1501,16 +1539,16 @@ EOF;
/**
* Builds service calls from arguments.
*/
private function getServiceCallsFromArguments(array $arguments, array &$calls, array &$behavior, $isPreInstantiation)
private function getServiceCallsFromArguments(array $arguments, array &$calls, array &$behavior, $isPreInstantiation, $callerId)
{
foreach ($arguments as $argument) {
if (is_array($argument)) {
$this->getServiceCallsFromArguments($argument, $calls, $behavior, $isPreInstantiation);
$this->getServiceCallsFromArguments($argument, $calls, $behavior, $isPreInstantiation, $callerId);
} elseif ($argument instanceof Reference) {
$id = (string) $argument;
if (!isset($calls[$id])) {
$calls[$id] = (int) ($isPreInstantiation && $this->container->has($id) && !$this->container->findDefinition($id)->isSynthetic());
$calls[$id] = (int) ($isPreInstantiation && isset($this->circularReferences[$callerId][$id]));
}
if (!isset($behavior[$id])) {
$behavior[$id] = $argument->getInvalidBehavior();
@ -1582,6 +1620,10 @@ EOF;
*/
private function hasReference($id, array $arguments, $deep = false, array &$visited = array())
{
if (!isset($this->circularReferences[$id])) {
return false;
}
foreach ($arguments as $argument) {
if (is_array($argument)) {
if ($this->hasReference($id, $argument, $deep, $visited)) {
@ -1595,7 +1637,7 @@ EOF;
return true;
}
if (!$deep || isset($visited[$argumentId]) || 'service_container' === $argumentId) {
if (!$deep || isset($visited[$argumentId]) || !isset($this->circularReferences[$id][$argumentId])) {
continue;
}

View File

@ -95,13 +95,7 @@ class ProjectServiceContainer extends Container
require_once $this->targetDirs[1].'/includes/HotPath/C2.php';
require_once $this->targetDirs[1].'/includes/HotPath/C3.php';
$a = ${($_ = isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3']) ? $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3'] : $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3()) && false ?: '_'};
if (isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C2'])) {
return $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C2'];
}
return $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C2'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C2($a);
return $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C2'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C2(${($_ = isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3']) ? $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3'] : $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3()) && false ?: '_'});
}
/**

View File

@ -82,10 +82,6 @@ class ProjectServiceContainer extends Container
{
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'};
if (isset($this->services['bar'])) {
return $this->services['bar'];
}
$this->services['bar'] = $instance = new \Bar\FooClass('foo', $a, $this->getParameter('foo_bar'));
$a->configure($instance);
@ -186,13 +182,7 @@ class ProjectServiceContainer extends Container
*/
protected function getFactoryServiceService()
{
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'};
if (isset($this->services['factory_service'])) {
return $this->services['factory_service'];
}
return $this->services['factory_service'] = $a->getInstance();
return $this->services['factory_service'] = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'}->getInstance();
}
/**
@ -202,13 +192,7 @@ class ProjectServiceContainer extends Container
*/
protected function getFactoryServiceSimpleService()
{
$a = ${($_ = isset($this->services['factory_simple']) ? $this->services['factory_simple'] : $this->getFactorySimpleService()) && false ?: '_'};
if (isset($this->services['factory_service_simple'])) {
return $this->services['factory_service_simple'];
}
return $this->services['factory_service_simple'] = $a->getInstance();
return $this->services['factory_service_simple'] = ${($_ = isset($this->services['factory_simple']) ? $this->services['factory_simple'] : $this->getFactorySimpleService()) && false ?: '_'}->getInstance();
}
/**
@ -220,10 +204,6 @@ class ProjectServiceContainer extends Container
{
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'};
if (isset($this->services['foo'])) {
return $this->services['foo'];
}
$this->services['foo'] = $instance = \Bar\FooClass::getInstance('foo', $a, array($this->getParameter('foo') => 'foo is '.$this->getParameter('foo').'', 'foobar' => $this->getParameter('foo')), true, $this);
$instance->foo = 'bar';
@ -341,13 +321,7 @@ class ProjectServiceContainer extends Container
*/
protected function getNewFactoryServiceService()
{
$a = ${($_ = isset($this->services['new_factory']) ? $this->services['new_factory'] : $this->getNewFactoryService()) && false ?: '_'};
if (isset($this->services['new_factory_service'])) {
return $this->services['new_factory_service'];
}
$this->services['new_factory_service'] = $instance = $a->getInstance();
$this->services['new_factory_service'] = $instance = ${($_ = isset($this->services['new_factory']) ? $this->services['new_factory'] : $this->getNewFactoryService()) && false ?: '_'}->getInstance();
$instance->foo = 'bar';

View File

@ -33,18 +33,12 @@ use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
// Returns the public 'configured_service' shared service.
$a = ${($_ = isset($this->services['baz']) ? $this->services['baz'] : $this->load(__DIR__.'/getBazService.php')) && false ?: '_'};
if (isset($this->services['configured_service'])) {
return $this->services['configured_service'];
}
$b = new \ConfClass();
$b->setFoo($a);
$a = new \ConfClass();
$a->setFoo(${($_ = isset($this->services['baz']) ? $this->services['baz'] : $this->load(__DIR__.'/getBazService.php')) && false ?: '_'});
$this->services['configured_service'] = $instance = new \stdClass();
$b->configureStdClass($instance);
$a->configureStdClass($instance);
return $instance;
@ -97,13 +91,7 @@ use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
// Returns the public 'factory_service' shared service.
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->load(__DIR__.'/getFoo_BazService.php')) && false ?: '_'};
if (isset($this->services['factory_service'])) {
return $this->services['factory_service'];
}
return $this->services['factory_service'] = $a->getInstance();
return $this->services['factory_service'] = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->load(__DIR__.'/getFoo_BazService.php')) && false ?: '_'}->getInstance();
[Container%s/getFactoryServiceSimpleService.php] => <?php
@ -112,13 +100,7 @@ use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
// Returns the public 'factory_service_simple' shared service.
$a = ${($_ = isset($this->services['factory_simple']) ? $this->services['factory_simple'] : $this->load(__DIR__.'/getFactorySimpleService.php')) && false ?: '_'};
if (isset($this->services['factory_service_simple'])) {
return $this->services['factory_service_simple'];
}
return $this->services['factory_service_simple'] = $a->getInstance();
return $this->services['factory_service_simple'] = ${($_ = isset($this->services['factory_simple']) ? $this->services['factory_simple'] : $this->load(__DIR__.'/getFactorySimpleService.php')) && false ?: '_'}->getInstance();
[Container%s/getFactorySimpleService.php] => <?php
@ -140,10 +122,6 @@ use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->load(__DIR__.'/getFoo_BazService.php')) && false ?: '_'};
if (isset($this->services['foo'])) {
return $this->services['foo'];
}
$this->services['foo'] = $instance = \Bar\FooClass::getInstance('foo', $a, array('bar' => 'foo is bar', 'foobar' => 'bar'), true, $this);
$instance->foo = 'bar';
@ -383,10 +361,6 @@ class ProjectServiceContainer extends Container
{
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->load(__DIR__.'/getFoo_BazService.php')) && false ?: '_'};
if (isset($this->services['bar'])) {
return $this->services['bar'];
}
$this->services['bar'] = $instance = new \Bar\FooClass('foo', $a, $this->getParameter('foo_bar'));
$a->configure($instance);

View File

@ -101,10 +101,6 @@ class ProjectServiceContainer extends Container
{
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'};
if (isset($this->services['bar'])) {
return $this->services['bar'];
}
$this->services['bar'] = $instance = new \Bar\FooClass('foo', $a, $this->getParameter('foo_bar'));
$a->configure($instance);
@ -133,18 +129,12 @@ class ProjectServiceContainer extends Container
*/
protected function getConfiguredServiceService()
{
$a = ${($_ = isset($this->services['baz']) ? $this->services['baz'] : $this->getBazService()) && false ?: '_'};
if (isset($this->services['configured_service'])) {
return $this->services['configured_service'];
}
$b = new \ConfClass();
$b->setFoo($a);
$a = new \ConfClass();
$a->setFoo(${($_ = isset($this->services['baz']) ? $this->services['baz'] : $this->getBazService()) && false ?: '_'});
$this->services['configured_service'] = $instance = new \stdClass();
$b->configureStdClass($instance);
$a->configureStdClass($instance);
return $instance;
}
@ -204,13 +194,7 @@ class ProjectServiceContainer extends Container
*/
protected function getFactoryServiceService()
{
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'};
if (isset($this->services['factory_service'])) {
return $this->services['factory_service'];
}
return $this->services['factory_service'] = $a->getInstance();
return $this->services['factory_service'] = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'}->getInstance();
}
/**
@ -220,13 +204,7 @@ class ProjectServiceContainer extends Container
*/
protected function getFactoryServiceSimpleService()
{
$a = ${($_ = isset($this->services['factory_simple']) ? $this->services['factory_simple'] : $this->getFactorySimpleService()) && false ?: '_'};
if (isset($this->services['factory_service_simple'])) {
return $this->services['factory_service_simple'];
}
return $this->services['factory_service_simple'] = $a->getInstance();
return $this->services['factory_service_simple'] = ${($_ = isset($this->services['factory_simple']) ? $this->services['factory_simple'] : $this->getFactorySimpleService()) && false ?: '_'}->getInstance();
}
/**
@ -238,10 +216,6 @@ class ProjectServiceContainer extends Container
{
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'};
if (isset($this->services['foo'])) {
return $this->services['foo'];
}
$this->services['foo'] = $instance = \Bar\FooClass::getInstance('foo', $a, array('bar' => 'foo is bar', 'foobar' => 'bar'), true, $this);
$instance->foo = 'bar';

View File

@ -90,13 +90,7 @@ class Symfony_DI_PhpDumper_Test_Legacy_Privates extends Container
*/
protected function getBarService()
{
$a = ${($_ = isset($this->services['private_not_inlined']) ? $this->services['private_not_inlined'] : $this->services['private_not_inlined'] = new \stdClass()) && false ?: '_'};
if (isset($this->services['bar'])) {
return $this->services['bar'];
}
return $this->services['bar'] = new \stdClass($a);
return $this->services['bar'] = new \stdClass(${($_ = isset($this->services['private_not_inlined']) ? $this->services['private_not_inlined'] : $this->services['private_not_inlined'] = new \stdClass()) && false ?: '_'});
}
/**

View File

@ -75,13 +75,7 @@ class ProjectServiceContainer extends Container
*/
protected function getBarServiceService()
{
$a = ${($_ = isset($this->services['baz_service']) ? $this->services['baz_service'] : $this->services['baz_service'] = new \stdClass()) && false ?: '_'};
if (isset($this->services['bar_service'])) {
return $this->services['bar_service'];
}
return $this->services['bar_service'] = new \stdClass($a);
return $this->services['bar_service'] = new \stdClass(${($_ = isset($this->services['baz_service']) ? $this->services['baz_service'] : $this->services['baz_service'] = new \stdClass()) && false ?: '_'});
}
/**
@ -167,10 +161,6 @@ class ProjectServiceContainer extends Container
{
$a = ${($_ = isset($this->services['translator.loader_3']) ? $this->services['translator.loader_3'] : $this->services['translator.loader_3'] = new \stdClass()) && false ?: '_'};
if (isset($this->services['translator_3'])) {
return $this->services['translator_3'];
}
$this->services['translator_3'] = $instance = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(array('translator.loader_3' => function () {
return ${($_ = isset($this->services['translator.loader_3']) ? $this->services['translator.loader_3'] : $this->services['translator.loader_3'] = new \stdClass()) && false ?: '_'};
})));

View File

@ -66,13 +66,7 @@ class ProjectServiceContainer extends Container
*/
protected function getBarServiceService()
{
$a = ${($_ = isset($this->services['baz_service']) ? $this->services['baz_service'] : $this->services['baz_service'] = new \stdClass()) && false ?: '_'};
if (isset($this->services['bar_service'])) {
return $this->services['bar_service'];
}
return $this->services['bar_service'] = new \stdClass($a);
return $this->services['bar_service'] = new \stdClass(${($_ = isset($this->services['baz_service']) ? $this->services['baz_service'] : $this->services['baz_service'] = new \stdClass()) && false ?: '_'});
}
/**
@ -82,13 +76,7 @@ class ProjectServiceContainer extends Container
*/
protected function getFooServiceService()
{
$a = ${($_ = isset($this->services['baz_service']) ? $this->services['baz_service'] : $this->services['baz_service'] = new \stdClass()) && false ?: '_'};
if (isset($this->services['foo_service'])) {
return $this->services['foo_service'];
}
return $this->services['foo_service'] = new \stdClass($a);
return $this->services['foo_service'] = new \stdClass(${($_ = isset($this->services['baz_service']) ? $this->services['baz_service'] : $this->services['baz_service'] = new \stdClass()) && false ?: '_'});
}
/**