diff --git a/Compiler/AnalyzeServiceReferencesPass.php b/Compiler/AnalyzeServiceReferencesPass.php index 22976bbb1..27f2c21ad 100644 --- a/Compiler/AnalyzeServiceReferencesPass.php +++ b/Compiler/AnalyzeServiceReferencesPass.php @@ -34,15 +34,17 @@ class AnalyzeServiceReferencesPass extends AbstractRecursivePass implements Repe private $graph; private $currentDefinition; private $onlyConstructorArguments; + private $hasProxyDumper; private $lazy; private $expressionLanguage; /** * @param bool $onlyConstructorArguments Sets this Service Reference pass to ignore method calls */ - public function __construct(bool $onlyConstructorArguments = false) + public function __construct(bool $onlyConstructorArguments = false, bool $hasProxyDumper = true) { $this->onlyConstructorArguments = $onlyConstructorArguments; + $this->hasProxyDumper = $hasProxyDumper; } /** @@ -97,7 +99,7 @@ protected function processValue($value, $isRoot = false) $targetId, $targetDefinition, $value, - $this->lazy || ($targetDefinition && $targetDefinition->isLazy()), + $this->lazy || ($this->hasProxyDumper && $targetDefinition && $targetDefinition->isLazy()), ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $value->getInvalidBehavior() ); diff --git a/Compiler/AutowirePass.php b/Compiler/AutowirePass.php index 2d937f284..e858d14fa 100644 --- a/Compiler/AutowirePass.php +++ b/Compiler/AutowirePass.php @@ -144,11 +144,12 @@ private function doProcessValue($value, $isRoot = false) */ private function autowireCalls(\ReflectionClass $reflectionClass, bool $isRoot): array { + $this->decoratedId = null; + $this->decoratedClass = null; + $this->getPreviousValue = null; + if ($isRoot && ($definition = $this->container->getDefinition($this->currentId)) && $this->container->has($this->decoratedId = $definition->innerServiceId)) { $this->decoratedClass = $this->container->findDefinition($this->decoratedId)->getClass(); - } else { - $this->decoratedId = null; - $this->decoratedClass = null; } foreach ($this->methodCalls as $i => $call) { diff --git a/Compiler/ResolveReferencesToAliasesPass.php b/Compiler/ResolveReferencesToAliasesPass.php index 31a7a2789..73a12026e 100644 --- a/Compiler/ResolveReferencesToAliasesPass.php +++ b/Compiler/ResolveReferencesToAliasesPass.php @@ -58,7 +58,7 @@ private function getDefinitionId(string $id, ContainerBuilder $container): strin $seen = array(); while ($container->hasAlias($id)) { if (isset($seen[$id])) { - throw new ServiceCircularReferenceException($id, array_keys($seen)); + throw new ServiceCircularReferenceException($id, array_merge(array_keys($seen), array($id))); } $seen[$id] = true; $id = (string) $container->getAlias($id); diff --git a/Container.php b/Container.php index 98da2596c..9b012361a 100644 --- a/Container.php +++ b/Container.php @@ -230,7 +230,7 @@ public function get($id, $invalidBehavior = /* self::EXCEPTION_ON_INVALID_REFERE private function make(string $id, int $invalidBehavior) { if (isset($this->loading[$id])) { - throw new ServiceCircularReferenceException($id, array_keys($this->loading)); + throw new ServiceCircularReferenceException($id, array_merge(array_keys($this->loading), array($id))); } $this->loading[$id] = true; diff --git a/ContainerBuilder.php b/ContainerBuilder.php index 725503680..080b5de00 100644 --- a/ContainerBuilder.php +++ b/ContainerBuilder.php @@ -121,7 +121,6 @@ class ContainerBuilder extends Container implements TaggedContainerInterface private $autoconfiguredInstanceof = array(); private $removedIds = array(); - private $alreadyLoading = array(); private static $internalTypes = array( 'int' => true, @@ -555,20 +554,30 @@ public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INV return $this->doGet($id, $invalidBehavior); } - private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, array &$inlineServices = array()) + private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, array &$inlineServices = null, $isConstructorArgument = false) { if (isset($inlineServices[$id])) { return $inlineServices[$id]; } - if (ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior) { - return parent::get($id, $invalidBehavior); + if (null === $inlineServices) { + $isConstructorArgument = true; + $inlineServices = array(); } - if ($service = parent::get($id, ContainerInterface::NULL_ON_INVALID_REFERENCE)) { - return $service; + try { + if (ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior) { + return parent::get($id, $invalidBehavior); + } + if ($service = parent::get($id, ContainerInterface::NULL_ON_INVALID_REFERENCE)) { + return $service; + } + } catch (ServiceCircularReferenceException $e) { + if ($isConstructorArgument) { + throw $e; + } } if (!isset($this->definitions[$id]) && isset($this->aliasDefinitions[$id])) { - return $this->doGet((string) $this->aliasDefinitions[$id], $invalidBehavior, $inlineServices); + return $this->doGet((string) $this->aliasDefinitions[$id], $invalidBehavior, $inlineServices, $isConstructorArgument); } try { @@ -585,16 +594,17 @@ private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_ throw new RuntimeException(reset($e)); } - $loading = isset($this->alreadyLoading[$id]) ? 'loading' : 'alreadyLoading'; - $this->{$loading}[$id] = true; + if ($isConstructorArgument) { + $this->loading[$id] = true; + } try { - $service = $this->createService($definition, $inlineServices, $id); + return $this->createService($definition, $inlineServices, $isConstructorArgument, $id); } finally { - unset($this->{$loading}[$id]); + if ($isConstructorArgument) { + unset($this->loading[$id]); + } } - - return $service; } /** @@ -1050,7 +1060,7 @@ public function findDefinition($id) * @throws RuntimeException When the service is a synthetic service * @throws InvalidArgumentException When configure callable is not callable */ - private function createService(Definition $definition, array &$inlineServices, $id = null, $tryProxy = true) + private function createService(Definition $definition, array &$inlineServices, $isConstructorArgument = false, $id = null, $tryProxy = true) { if (null === $id && isset($inlineServices[$h = spl_object_hash($definition)])) { return $inlineServices[$h]; @@ -1068,16 +1078,14 @@ private function createService(Definition $definition, array &$inlineServices, $ @trigger_error($definition->getDeprecationMessage($id), E_USER_DEPRECATED); } - if ($tryProxy && $definition->isLazy()) { - $proxy = $this - ->getProxyInstantiator() - ->instantiateProxy( - $this, - $definition, - $id, function () use ($definition, &$inlineServices, $id) { - return $this->createService($definition, $inlineServices, $id, false); - } - ); + if ($tryProxy && $definition->isLazy() && !$tryProxy = !($proxy = $this->proxyInstantiator) || $proxy instanceof RealServiceInstantiator) { + $proxy = $proxy->instantiateProxy( + $this, + $definition, + $id, function () use ($definition, &$inlineServices, $id) { + return $this->createService($definition, $inlineServices, true, $id, false); + } + ); $this->shareService($definition, $proxy, $id, $inlineServices); return $proxy; @@ -1089,19 +1097,21 @@ private function createService(Definition $definition, array &$inlineServices, $ require_once $parameterBag->resolveValue($definition->getFile()); } - $arguments = $this->doResolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getArguments())), $inlineServices); - - if (null !== $id && $definition->isShared() && isset($this->services[$id]) && ($tryProxy || !$definition->isLazy())) { - return $this->services[$id]; - } + $arguments = $this->doResolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getArguments())), $inlineServices, $isConstructorArgument); if (null !== $factory = $definition->getFactory()) { if (\is_array($factory)) { - $factory = array($this->doResolveServices($parameterBag->resolveValue($factory[0]), $inlineServices), $factory[1]); + $factory = array($this->doResolveServices($parameterBag->resolveValue($factory[0]), $inlineServices, $isConstructorArgument), $factory[1]); } elseif (!\is_string($factory)) { throw new RuntimeException(sprintf('Cannot create service "%s" because of invalid factory', $id)); } + } + if (null !== $id && $definition->isShared() && isset($this->services[$id]) && ($tryProxy || !$definition->isLazy())) { + return $this->services[$id]; + } + + if (null !== $factory) { $service = \call_user_func_array($factory, $arguments); if (!$definition->isDeprecated() && \is_array($factory) && \is_string($factory[0])) { @@ -1169,11 +1179,11 @@ public function resolveServices($value) return $this->doResolveServices($value); } - private function doResolveServices($value, array &$inlineServices = array()) + private function doResolveServices($value, array &$inlineServices = array(), $isConstructorArgument = false) { if (\is_array($value)) { foreach ($value as $k => $v) { - $value[$k] = $this->doResolveServices($v, $inlineServices); + $value[$k] = $this->doResolveServices($v, $inlineServices, $isConstructorArgument); } } elseif ($value instanceof ServiceClosureArgument) { $reference = $value->getValues()[0]; @@ -1216,9 +1226,9 @@ private function doResolveServices($value, array &$inlineServices = array()) return $count; }); } elseif ($value instanceof Reference) { - $value = $this->doGet((string) $value, $value->getInvalidBehavior(), $inlineServices); + $value = $this->doGet((string) $value, $value->getInvalidBehavior(), $inlineServices, $isConstructorArgument); } elseif ($value instanceof Definition) { - $value = $this->createService($value, $inlineServices); + $value = $this->createService($value, $inlineServices, $isConstructorArgument); } elseif ($value instanceof Parameter) { $value = $this->getParameter((string) $value); } elseif ($value instanceof Expression) { @@ -1511,18 +1521,6 @@ protected function getEnv($name) } } - /** - * Retrieves the currently set proxy instantiator or instantiates one. - */ - private function getProxyInstantiator(): InstantiatorInterface - { - if (!$this->proxyInstantiator) { - $this->proxyInstantiator = new RealServiceInstantiator(); - } - - return $this->proxyInstantiator; - } - private function callMethod($service, $call, array &$inlineServices) { foreach (self::getServiceConditionals($call[1]) as $s) { @@ -1552,7 +1550,7 @@ private function shareService(Definition $definition, $service, $id, array &$inl if (null !== $id && $definition->isShared()) { $this->services[$id] = $service; - unset($this->loading[$id], $this->alreadyLoading[$id]); + unset($this->loading[$id]); } } diff --git a/Dumper/PhpDumper.php b/Dumper/PhpDumper.php index f9f9613f6..f37641030 100644 --- a/Dumper/PhpDumper.php +++ b/Dumper/PhpDumper.php @@ -15,6 +15,7 @@ use Symfony\Component\DependencyInjection\Argument\IteratorArgument; use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument; use Symfony\Component\DependencyInjection\Compiler\AnalyzeServiceReferencesPass; +use Symfony\Component\DependencyInjection\Compiler\CheckCircularReferencesPass; use Symfony\Component\DependencyInjection\Container; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -139,7 +140,20 @@ public function dump(array $options = array()) $this->initializeMethodNamesMap('Container' === $baseClass ? Container::class : $baseClass); - (new AnalyzeServiceReferencesPass())->process($this->container); + if ($this->getProxyDumper() instanceof NullDumper) { + (new AnalyzeServiceReferencesPass(true, false))->process($this->container); + try { + (new CheckCircularReferencesPass())->process($this->container); + } catch (ServiceCircularReferenceException $e) { + $path = $e->getPath(); + end($path); + $path[key($path)] .= '". Try running "composer require symfony/proxy-manager-bridge'; + + throw new ServiceCircularReferenceException($e->getServiceId(), $path); + } + } + + (new AnalyzeServiceReferencesPass(false, !$this->getProxyDumper() instanceof NullDumper))->process($this->container); $this->circularReferences = array(); $checkedNodes = array(); foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) { @@ -287,21 +301,18 @@ private function getProxyDumper(): ProxyDumper return $this->proxyDumper; } - private function addServiceLocalTempVariables(string $cId, Definition $definition, \SplObjectStorage $inlinedDefinitions, \SplObjectStorage $allInlinedDefinitions): string + private function addServiceLocalTempVariables(string $cId, Definition $definition, \SplObjectStorage $inlinedDefinitions, array $serviceCalls, bool $preInstance = false): string { - $allCalls = $calls = $behavior = array(); + $calls = array(); - foreach ($allInlinedDefinitions as $def) { - $arguments = array($def->getArguments(), $def->getFactory(), $def->getProperties(), $def->getMethodCalls(), $def->getConfigurator()); - $this->getServiceCallsFromArguments($arguments, $allCalls, false, $cId, $behavior, $allInlinedDefinitions[$def]); - } - - $isPreInstance = isset($inlinedDefinitions[$definition]) && isset($this->circularReferences[$cId]) && !$this->getProxyDumper()->isProxyCandidate($definition) && $definition->isShared(); foreach ($inlinedDefinitions as $def) { - $this->getServiceCallsFromArguments(array($def->getArguments(), $def->getFactory()), $calls, $isPreInstance, $cId); + if ($preInstance && !$inlinedDefinitions[$def][1]) { + continue; + } + $this->getServiceCallsFromArguments(array($def->getArguments(), $def->getFactory()), $calls, $preInstance, $cId); if ($def !== $definition) { $arguments = array($def->getProperties(), $def->getMethodCalls(), $def->getConfigurator()); - $this->getServiceCallsFromArguments($arguments, $calls, $isPreInstance && !$this->hasReference($cId, $arguments, true), $cId); + $this->getServiceCallsFromArguments($arguments, $calls, $preInstance && !$this->hasReference($cId, $arguments, true), $cId); } } if (!isset($inlinedDefinitions[$definition])) { @@ -310,23 +321,23 @@ private function addServiceLocalTempVariables(string $cId, Definition $definitio } $code = ''; - foreach ($calls as $id => $callCount) { + foreach ($calls as $id => list($callCount)) { if ('service_container' === $id || $id === $cId || isset($this->referenceVariables[$id])) { continue; } - if ($callCount <= 1 && $allCalls[$id] <= 1) { + if ($callCount <= 1 && $serviceCalls[$id][0] <= 1) { continue; } $name = $this->getNextVariableName(); $this->referenceVariables[$id] = new Variable($name); - $reference = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE >= $behavior[$id] ? new Reference($id, $behavior[$id]) : null; + $reference = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE >= $serviceCalls[$id][1] ? new Reference($id, $serviceCalls[$id][1]) : null; $code .= sprintf(" \$%s = %s;\n", $name, $this->getServiceCall($id, $reference)); } if ('' !== $code) { - if ($isPreInstance) { + if ($preInstance) { $code .= sprintf(<<<'EOTXT' if (isset($this->%s['%s'])) { @@ -422,23 +433,21 @@ private function generateProxyClasses() } } - private function addServiceInclude(string $cId, Definition $definition, \SplObjectStorage $inlinedDefinitions): string + private function addServiceInclude(string $cId, Definition $definition, \SplObjectStorage $inlinedDefinitions, array $serviceCalls): string { $code = ''; if ($this->inlineRequires && !$this->isHotPath($definition)) { - $lineage = $calls = $behavior = array(); + $lineage = array(); foreach ($inlinedDefinitions as $def) { if (!$def->isDeprecated() && \is_string($class = \is_array($factory = $def->getFactory()) && \is_string($factory[0]) ? $factory[0] : $def->getClass())) { $this->collectLineage($class, $lineage); } - $arguments = array($def->getArguments(), $def->getFactory(), $def->getProperties(), $def->getMethodCalls(), $def->getConfigurator()); - $this->getServiceCallsFromArguments($arguments, $calls, false, $cId, $behavior, $inlinedDefinitions[$def]); } - foreach ($calls as $id => $callCount) { + foreach ($serviceCalls as $id => list($callCount, $behavior)) { if ('service_container' !== $id && $id !== $cId - && ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE !== $behavior[$id] + && ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE !== $behavior && $this->container->has($id) && $this->isTrivialInstance($def = $this->container->findDefinition($id)) && \is_string($class = \is_array($factory = $def->getFactory()) && \is_string($factory[0]) ? $factory[0] : $def->getClass()) @@ -471,24 +480,24 @@ private function addServiceInclude(string $cId, Definition $definition, \SplObje * @throws RuntimeException When the factory definition is incomplete * @throws ServiceCircularReferenceException When a circular reference is detected */ - private function addServiceInlinedDefinitions(string $id, Definition $definition, \SplObjectStorage $inlinedDefinitions, bool &$isSimpleInstance): string + private function addServiceInlinedDefinitions(string $id, Definition $definition, \SplObjectStorage $inlinedDefinitions, bool &$isSimpleInstance, bool $preInstance = false): string { $code = ''; foreach ($inlinedDefinitions as $def) { - if ($definition === $def) { + if ($definition === $def || isset($this->definitionVariables[$def])) { continue; } - if ($inlinedDefinitions[$def] <= 1 && !$def->getMethodCalls() && !$def->getProperties() && !$def->getConfigurator() && false === strpos($this->dumpValue($def->getClass()), '$')) { + if ($inlinedDefinitions[$def][0] <= 1 && !$def->getMethodCalls() && !$def->getProperties() && !$def->getConfigurator() && false === strpos($this->dumpValue($def->getClass()), '$')) { continue; } - if (isset($this->definitionVariables[$def])) { - $name = $this->definitionVariables[$def]; - } else { - $name = $this->getNextVariableName(); - $this->definitionVariables[$def] = new Variable($name); + if ($preInstance && !$inlinedDefinitions[$def][1]) { + continue; } + $name = $this->getNextVariableName(); + $this->definitionVariables[$def] = new Variable($name); + // a construct like: // $a = new ServiceA(ServiceB $b); $b = new ServiceB(ServiceA $a); // this is an indication for a wrong implementation, you can circumvent this problem @@ -497,7 +506,7 @@ private function addServiceInlinedDefinitions(string $id, Definition $definition // $a = new ServiceA(ServiceB $b); // $b->setServiceA(ServiceA $a); if (isset($inlinedDefinitions[$definition]) && $this->hasReference($id, array($def->getArguments(), $def->getFactory()))) { - throw new ServiceCircularReferenceException($id, array($id)); + throw new ServiceCircularReferenceException($id, array($id, '...', $id)); } $code .= $this->addNewInstance($def, '$'.$name, ' = ', $id); @@ -545,6 +554,7 @@ private function addServiceInstance(string $id, Definition $definition, string $ } $code = $this->addNewInstance($definition, $return, $instantiation, $id); + $this->referenceVariables[$id] = new Variable('instance'); if (!$isSimpleInstance) { $code .= "\n"; @@ -625,8 +635,6 @@ private function addServiceProperties(Definition $definition, $variableName = 'i */ private function addServiceInlinedDefinitionsSetup(string $id, Definition $definition, \SplObjectStorage $inlinedDefinitions, bool $isSimpleInstance): string { - $this->referenceVariables[$id] = new Variable('instance'); - $code = ''; foreach ($inlinedDefinitions as $def) { if ($definition === $def || !$this->hasReference($id, array($def->getProperties(), $def->getMethodCalls(), $def->getConfigurator()), true)) { @@ -636,7 +644,7 @@ private function addServiceInlinedDefinitionsSetup(string $id, Definition $defin // if the instance is simple, the return statement has already been generated // so, the only possible way to get there is because of a circular reference if ($isSimpleInstance) { - throw new ServiceCircularReferenceException($id, array($id)); + throw new ServiceCircularReferenceException($id, array($id, '...', $id)); } $name = (string) $this->definitionVariables[$def]; @@ -753,6 +761,7 @@ protected function {$methodName}($lazyInitialization) $inlinedDefinitions = $this->getDefinitionsFromArguments(array($definition)); $constructorDefinitions = $this->getDefinitionsFromArguments(array($definition->getArguments(), $definition->getFactory())); $otherDefinitions = new \SplObjectStorage(); + $serviceCalls = array(); foreach ($inlinedDefinitions as $def) { if ($def === $definition || isset($constructorDefinitions[$def])) { @@ -760,11 +769,14 @@ protected function {$methodName}($lazyInitialization) } else { $otherDefinitions[$def] = $inlinedDefinitions[$def]; } + $arguments = array($def->getArguments(), $def->getFactory(), $def->getProperties(), $def->getMethodCalls(), $def->getConfigurator()); + $this->getServiceCallsFromArguments($arguments, $serviceCalls, false, $id); } $isSimpleInstance = !$definition->getProperties() && !$definition->getMethodCalls() && !$definition->getConfigurator(); + $preInstance = isset($this->circularReferences[$id]) && !$this->getProxyDumper()->isProxyCandidate($definition) && $definition->isShared(); - $code .= $this->addServiceInclude($id, $definition, $inlinedDefinitions); + $code .= $this->addServiceInclude($id, $definition, $inlinedDefinitions, $serviceCalls); if ($this->getProxyDumper()->isProxyCandidate($definition)) { $factoryCode = $asFile ? "\$this->load('%s.php', false)" : '$this->%s(false)'; @@ -776,10 +788,12 @@ protected function {$methodName}($lazyInitialization) } $code .= - $this->addServiceLocalTempVariables($id, $definition, $constructorDefinitions, $inlinedDefinitions). - $this->addServiceInlinedDefinitions($id, $definition, $constructorDefinitions, $isSimpleInstance). + $this->addServiceLocalTempVariables($id, $definition, $constructorDefinitions, $serviceCalls, $preInstance). + $this->addServiceInlinedDefinitions($id, $definition, $constructorDefinitions, $isSimpleInstance, $preInstance). $this->addServiceInstance($id, $definition, $isSimpleInstance). - $this->addServiceLocalTempVariables($id, $definition, $otherDefinitions, $inlinedDefinitions). + $this->addServiceLocalTempVariables($id, $definition, $constructorDefinitions->offsetUnset($definition) ?: $constructorDefinitions, $serviceCalls). + $this->addServiceLocalTempVariables($id, $definition, $otherDefinitions, $serviceCalls). + $this->addServiceInlinedDefinitions($id, $definition, $constructorDefinitions, $isSimpleInstance). $this->addServiceInlinedDefinitions($id, $definition, $otherDefinitions, $isSimpleInstance). $this->addServiceInlinedDefinitionsSetup($id, $definition, $inlinedDefinitions, $isSimpleInstance). $this->addServiceProperties($definition). @@ -1359,29 +1373,26 @@ private function getServiceConditionals($value): string return implode(' && ', $conditions); } - private function getServiceCallsFromArguments(array $arguments, array &$calls, bool $isPreInstance, string $callerId, array &$behavior = array(), int $step = 1) + private function getServiceCallsFromArguments(array $arguments, array &$calls, bool $preInstance, string $callerId) { foreach ($arguments as $argument) { if (\is_array($argument)) { - $this->getServiceCallsFromArguments($argument, $calls, $isPreInstance, $callerId, $behavior, $step); + $this->getServiceCallsFromArguments($argument, $calls, $preInstance, $callerId); } elseif ($argument instanceof Reference) { $id = (string) $argument; if (!isset($calls[$id])) { - $calls[$id] = (int) ($isPreInstance && isset($this->circularReferences[$callerId][$id])); - } - if (!isset($behavior[$id])) { - $behavior[$id] = $argument->getInvalidBehavior(); + $calls[$id] = array((int) ($preInstance && isset($this->circularReferences[$callerId][$id])), $argument->getInvalidBehavior()); } else { - $behavior[$id] = min($behavior[$id], $argument->getInvalidBehavior()); + $calls[$id][1] = min($calls[$id][1], $argument->getInvalidBehavior()); } - $calls[$id] += $step; + ++$calls[$id][0]; } } } - private function getDefinitionsFromArguments(array $arguments, \SplObjectStorage $definitions = null): \SplObjectStorage + private function getDefinitionsFromArguments(array $arguments, bool $isConstructorArgument = true, \SplObjectStorage $definitions = null): \SplObjectStorage { if (null === $definitions) { $definitions = new \SplObjectStorage(); @@ -1389,22 +1400,23 @@ private function getDefinitionsFromArguments(array $arguments, \SplObjectStorage foreach ($arguments as $argument) { if (\is_array($argument)) { - $this->getDefinitionsFromArguments($argument, $definitions); + $this->getDefinitionsFromArguments($argument, $isConstructorArgument, $definitions); } elseif (!$argument instanceof Definition) { // no-op } elseif (isset($definitions[$argument])) { - $definitions[$argument] = 1 + $definitions[$argument]; + $def = $definitions[$argument]; + $definitions[$argument] = array(1 + $def[0], $isConstructorArgument || $def[1]); } else { - $definitions[$argument] = 1; - $this->getDefinitionsFromArguments($argument->getArguments(), $definitions); - $this->getDefinitionsFromArguments(array($argument->getFactory()), $definitions); - $this->getDefinitionsFromArguments($argument->getProperties(), $definitions); - $this->getDefinitionsFromArguments($argument->getMethodCalls(), $definitions); - $this->getDefinitionsFromArguments(array($argument->getConfigurator()), $definitions); + $definitions[$argument] = array(1, $isConstructorArgument); + $this->getDefinitionsFromArguments($argument->getArguments(), $isConstructorArgument, $definitions); + $this->getDefinitionsFromArguments(array($argument->getFactory()), $isConstructorArgument, $definitions); + $this->getDefinitionsFromArguments($argument->getProperties(), false, $definitions); + $this->getDefinitionsFromArguments($argument->getMethodCalls(), false, $definitions); + $this->getDefinitionsFromArguments(array($argument->getConfigurator()), false, $definitions); // move current definition last in the list - $nbOccurences = $definitions[$argument]; + $def = $definitions[$argument]; unset($definitions[$argument]); - $definitions[$argument] = $nbOccurences; + $definitions[$argument] = $def; } } @@ -1430,7 +1442,7 @@ private function hasReference(string $id, array $arguments, bool $deep = false, return true; } - if (!$deep || isset($visited[$argumentId]) || !isset($this->circularReferences[$id][$argumentId])) { + if (!$deep || isset($visited[$argumentId]) || !isset($this->circularReferences[$argumentId])) { continue; } diff --git a/Tests/Compiler/AutowirePassTest.php b/Tests/Compiler/AutowirePassTest.php index cbbd88678..366583aba 100644 --- a/Tests/Compiler/AutowirePassTest.php +++ b/Tests/Compiler/AutowirePassTest.php @@ -834,6 +834,29 @@ public function testAutowireDecorator() $this->assertSame(Decorator::class.'.inner', (string) $definition->getArgument(1)); } + public function testAutowireDecoratorChain() + { + $container = new ContainerBuilder(); + $container->register(LoggerInterface::class, NullLogger::class); + $container->register(Decorated::class, Decorated::class); + $container + ->register(Decorator::class, Decorator::class) + ->setDecoratedService(Decorated::class) + ->setAutowired(true) + ; + $container + ->register(DecoratedDecorator::class, DecoratedDecorator::class) + ->setDecoratedService(Decorated::class) + ->setAutowired(true) + ; + + (new DecoratorServicePass())->process($container); + (new AutowirePass())->process($container); + + $definition = $container->getDefinition(DecoratedDecorator::class); + $this->assertSame(DecoratedDecorator::class.'.inner', (string) $definition->getArgument(0)); + } + public function testAutowireDecoratorRenamedId() { $container = new ContainerBuilder(); diff --git a/Tests/ContainerBuilderTest.php b/Tests/ContainerBuilderTest.php index 7ac3a4bb4..f6fa62fc3 100644 --- a/Tests/ContainerBuilderTest.php +++ b/Tests/ContainerBuilderTest.php @@ -1353,6 +1353,12 @@ public function testAlmostCircular($visibility) $foo5 = $container->get('foo5'); $this->assertSame($foo5, $foo5->bar->foo); + + $manager = $container->get('manager'); + $this->assertEquals(new \stdClass(), $manager); + + $manager = $container->get('manager2'); + $this->assertEquals(new \stdClass(), $manager); } public function provideAlmostCircular() diff --git a/Tests/Dumper/PhpDumperTest.php b/Tests/Dumper/PhpDumperTest.php index cf5a4b579..8d6d0781b 100644 --- a/Tests/Dumper/PhpDumperTest.php +++ b/Tests/Dumper/PhpDumperTest.php @@ -22,6 +22,7 @@ use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Dumper\PhpDumper; use Symfony\Component\DependencyInjection\EnvVarProcessorInterface; +use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException; use Symfony\Component\DependencyInjection\Loader\YamlFileLoader; use Symfony\Component\DependencyInjection\Parameter; use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag; @@ -570,50 +571,22 @@ public function testCircularReferenceAllowanceForLazyServices() $container->compile(); $dumper = new PhpDumper($container); + $dumper->setProxyDumper(new \DummyProxyDumper()); $dumper->dump(); $this->addToAssertionCount(1); - } - - public function testCircularReferenceAllowanceForInlinedDefinitionsForLazyServices() - { - /* - * test graph: - * [connection] -> [event_manager] --> [entity_manager](lazy) - * | - * --(call)- addEventListener ("@lazy_service") - * - * [lazy_service](lazy) -> [entity_manager](lazy) - * - */ - - $container = new ContainerBuilder(); - - $eventManagerDefinition = new Definition('stdClass'); - - $connectionDefinition = $container->register('connection', 'stdClass')->setPublic(true); - $connectionDefinition->addArgument($eventManagerDefinition); - - $container->register('entity_manager', 'stdClass') - ->setPublic(true) - ->setLazy(true) - ->addArgument(new Reference('connection')); - - $lazyServiceDefinition = $container->register('lazy_service', 'stdClass'); - $lazyServiceDefinition->setPublic(true); - $lazyServiceDefinition->setLazy(true); - $lazyServiceDefinition->addArgument(new Reference('entity_manager')); - - $eventManagerDefinition->addMethodCall('addEventListener', array(new Reference('lazy_service'))); - - $container->compile(); $dumper = new PhpDumper($container); - $dumper->setProxyDumper(new \DummyProxyDumper()); - $dumper->dump(); + $message = 'Circular reference detected for service "foo", path: "foo -> bar -> foo". Try running "composer require symfony/proxy-manager-bridge".'; + if (method_exists($this, 'expectException')) { + $this->expectException(ServiceCircularReferenceException::class); + $this->expectExceptionMessage($message); + } else { + $this->setExpectedException(ServiceCircularReferenceException::class, $message); + } - $this->addToAssertionCount(1); + $dumper->dump(); } public function testDedupLazyProxy() @@ -891,6 +864,12 @@ public function testAlmostCircular($visibility) $foo5 = $container->get('foo5'); $this->assertSame($foo5, $foo5->bar->foo); + + $manager = $container->get('manager'); + $this->assertEquals(new \stdClass(), $manager); + + $manager = $container->get('manager2'); + $this->assertEquals(new \stdClass(), $manager); } public function provideAlmostCircular() diff --git a/Tests/Fixtures/containers/container_almost_circular.php b/Tests/Fixtures/containers/container_almost_circular.php index dff937ccd..2079e136b 100644 --- a/Tests/Fixtures/containers/container_almost_circular.php +++ b/Tests/Fixtures/containers/container_almost_circular.php @@ -55,4 +55,50 @@ ->addArgument(new Reference('foo5')) ->setProperty('foo', new Reference('foo5')); +// doctrine-like event system + some extra + +$container->register('manager', 'stdClass')->setPublic(true) + ->addArgument(new Reference('connection')); + +$container->register('logger', 'stdClass')->setPublic(true) + ->addArgument(new Reference('connection')) + ->setProperty('handler', (new Definition('stdClass'))->addArgument(new Reference('manager'))) +; +$container->register('connection', 'stdClass')->setPublic(true) + ->addArgument(new Reference('dispatcher')) + ->addArgument(new Reference('config')); + +$container->register('config', 'stdClass')->setPublic(false) + ->setProperty('logger', new Reference('logger')); + +$container->register('dispatcher', 'stdClass')->setPublic($public) + ->setLazy($public) + ->setProperty('subscriber', new Reference('subscriber')); + +$container->register('subscriber', 'stdClass')->setPublic(true) + ->addArgument(new Reference('manager')); + +// doctrine-like event system + some extra (bis) + +$container->register('manager2', 'stdClass')->setPublic(true) + ->addArgument(new Reference('connection2')); + +$container->register('logger2', 'stdClass')->setPublic(false) + ->addArgument(new Reference('connection2')) + ->setProperty('handler2', (new Definition('stdClass'))->addArgument(new Reference('manager2'))) +; +$container->register('connection2', 'stdClass')->setPublic(true) + ->addArgument(new Reference('dispatcher2')) + ->addArgument(new Reference('config2')); + +$container->register('config2', 'stdClass')->setPublic(false) + ->setProperty('logger2', new Reference('logger2')); + +$container->register('dispatcher2', 'stdClass')->setPublic($public) + ->setLazy($public) + ->setProperty('subscriber2', new Reference('subscriber2')); + +$container->register('subscriber2', 'stdClass')->setPublic(false) + ->addArgument(new Reference('manager2')); + return $container; diff --git a/Tests/Fixtures/includes/autowiring_classes.php b/Tests/Fixtures/includes/autowiring_classes.php index d806f294d..93eab8c50 100644 --- a/Tests/Fixtures/includes/autowiring_classes.php +++ b/Tests/Fixtures/includes/autowiring_classes.php @@ -373,6 +373,13 @@ public function __construct(LoggerInterface $logger, DecoratorInterface $decorat } } +class DecoratedDecorator implements DecoratorInterface +{ + public function __construct(DecoratorInterface $decorator) + { + } +} + class NonAutowirableDecorator implements DecoratorInterface { public function __construct(LoggerInterface $logger, DecoratorInterface $decorated1, DecoratorInterface $decorated2) diff --git a/Tests/Fixtures/php/services_almost_circular_private.php b/Tests/Fixtures/php/services_almost_circular_private.php index 4de6bfc23..d6f1d6b11 100644 --- a/Tests/Fixtures/php/services_almost_circular_private.php +++ b/Tests/Fixtures/php/services_almost_circular_private.php @@ -30,10 +30,16 @@ public function __construct() $this->methodMap = array( 'bar2' => 'getBar2Service', 'bar3' => 'getBar3Service', + 'connection' => 'getConnectionService', + 'connection2' => 'getConnection2Service', 'foo' => 'getFooService', 'foo2' => 'getFoo2Service', 'foo5' => 'getFoo5Service', 'foobar4' => 'getFoobar4Service', + 'logger' => 'getLoggerService', + 'manager' => 'getManagerService', + 'manager2' => 'getManager2Service', + 'subscriber' => 'getSubscriberService', ); $this->aliases = array(); @@ -62,10 +68,16 @@ public function getRemovedIds() 'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true, 'bar' => true, 'bar5' => true, + 'config' => true, + 'config2' => true, + 'dispatcher' => true, + 'dispatcher2' => true, 'foo4' => true, 'foobar' => true, 'foobar2' => true, 'foobar3' => true, + 'logger2' => true, + 'subscriber2' => true, ); } @@ -99,6 +111,49 @@ protected function getBar3Service() return $instance; } + /** + * Gets the public 'connection' shared service. + * + * @return \stdClass + */ + protected function getConnectionService() + { + $a = new \stdClass(); + + $b = new \stdClass(); + + $this->services['connection'] = $instance = new \stdClass($a, $b); + + $a->subscriber = ($this->services['subscriber'] ?? $this->getSubscriberService()); + $b->logger = ($this->services['logger'] ?? $this->getLoggerService()); + + return $instance; + } + + /** + * Gets the public 'connection2' shared service. + * + * @return \stdClass + */ + protected function getConnection2Service() + { + $a = new \stdClass(); + + $b = new \stdClass(); + + $this->services['connection2'] = $instance = new \stdClass($a, $b); + + $c = ($this->services['manager2'] ?? $this->getManager2Service()); + + $d = new \stdClass($instance); + + $a->subscriber2 = new \stdClass($c); + $d->handler2 = new \stdClass($c); + $b->logger2 = $d; + + return $instance; + } + /** * Gets the public 'foo' shared service. * @@ -140,7 +195,7 @@ protected function getFoo5Service() { $this->services['foo5'] = $instance = new \stdClass(); - $a = new \stdClass(($this->services['foo5'] ?? $this->getFoo5Service())); + $a = new \stdClass($instance); $a->foo = $instance; @@ -164,4 +219,72 @@ protected function getFoobar4Service() return $instance; } + + /** + * Gets the public 'logger' shared service. + * + * @return \stdClass + */ + protected function getLoggerService() + { + $a = ($this->services['connection'] ?? $this->getConnectionService()); + + if (isset($this->services['logger'])) { + return $this->services['logger']; + } + + $this->services['logger'] = $instance = new \stdClass($a); + + $instance->handler = new \stdClass(($this->services['manager'] ?? $this->getManagerService())); + + return $instance; + } + + /** + * Gets the public 'manager' shared service. + * + * @return \stdClass + */ + protected function getManagerService() + { + $a = ($this->services['connection'] ?? $this->getConnectionService()); + + if (isset($this->services['manager'])) { + return $this->services['manager']; + } + + return $this->services['manager'] = new \stdClass($a); + } + + /** + * Gets the public 'manager2' shared service. + * + * @return \stdClass + */ + protected function getManager2Service() + { + $a = ($this->services['connection2'] ?? $this->getConnection2Service()); + + if (isset($this->services['manager2'])) { + return $this->services['manager2']; + } + + return $this->services['manager2'] = new \stdClass($a); + } + + /** + * Gets the public 'subscriber' shared service. + * + * @return \stdClass + */ + protected function getSubscriberService() + { + $a = ($this->services['manager'] ?? $this->getManagerService()); + + if (isset($this->services['subscriber'])) { + return $this->services['subscriber']; + } + + return $this->services['subscriber'] = new \stdClass($a); + } } diff --git a/Tests/Fixtures/php/services_almost_circular_public.php b/Tests/Fixtures/php/services_almost_circular_public.php index 79a0c11cc..42c09d33f 100644 --- a/Tests/Fixtures/php/services_almost_circular_public.php +++ b/Tests/Fixtures/php/services_almost_circular_public.php @@ -31,6 +31,10 @@ public function __construct() 'bar' => 'getBarService', 'bar3' => 'getBar3Service', 'bar5' => 'getBar5Service', + 'connection' => 'getConnectionService', + 'connection2' => 'getConnection2Service', + 'dispatcher' => 'getDispatcherService', + 'dispatcher2' => 'getDispatcher2Service', 'foo' => 'getFooService', 'foo2' => 'getFoo2Service', 'foo4' => 'getFoo4Service', @@ -39,6 +43,10 @@ public function __construct() 'foobar2' => 'getFoobar2Service', 'foobar3' => 'getFoobar3Service', 'foobar4' => 'getFoobar4Service', + 'logger' => 'getLoggerService', + 'manager' => 'getManagerService', + 'manager2' => 'getManager2Service', + 'subscriber' => 'getSubscriberService', ); $this->aliases = array(); @@ -66,6 +74,10 @@ public function getRemovedIds() 'Psr\\Container\\ContainerInterface' => true, 'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true, 'bar2' => true, + 'config' => true, + 'config2' => true, + 'logger2' => true, + 'subscriber2' => true, ); } @@ -119,6 +131,81 @@ protected function getBar5Service() return $instance; } + /** + * Gets the public 'connection' shared service. + * + * @return \stdClass + */ + protected function getConnectionService() + { + $a = ($this->services['dispatcher'] ?? $this->getDispatcherService()); + + if (isset($this->services['connection'])) { + return $this->services['connection']; + } + + $b = new \stdClass(); + + $this->services['connection'] = $instance = new \stdClass($a, $b); + + $b->logger = ($this->services['logger'] ?? $this->getLoggerService()); + + return $instance; + } + + /** + * Gets the public 'connection2' shared service. + * + * @return \stdClass + */ + protected function getConnection2Service() + { + $a = ($this->services['dispatcher2'] ?? $this->getDispatcher2Service()); + + if (isset($this->services['connection2'])) { + return $this->services['connection2']; + } + + $b = new \stdClass(); + + $this->services['connection2'] = $instance = new \stdClass($a, $b); + + $c = new \stdClass($instance); + + $c->handler2 = new \stdClass(($this->services['manager2'] ?? $this->getManager2Service())); + $b->logger2 = $c; + + return $instance; + } + + /** + * Gets the public 'dispatcher' shared service. + * + * @return \stdClass + */ + protected function getDispatcherService($lazyLoad = true) + { + $this->services['dispatcher'] = $instance = new \stdClass(); + + $instance->subscriber = ($this->services['subscriber'] ?? $this->getSubscriberService()); + + return $instance; + } + + /** + * Gets the public 'dispatcher2' shared service. + * + * @return \stdClass + */ + protected function getDispatcher2Service($lazyLoad = true) + { + $this->services['dispatcher2'] = $instance = new \stdClass(); + + $instance->subscriber2 = new \stdClass(($this->services['manager2'] ?? $this->getManager2Service())); + + return $instance; + } + /** * Gets the public 'foo' shared service. * @@ -236,4 +323,72 @@ protected function getFoobar4Service() return $instance; } + + /** + * Gets the public 'logger' shared service. + * + * @return \stdClass + */ + protected function getLoggerService() + { + $a = ($this->services['connection'] ?? $this->getConnectionService()); + + if (isset($this->services['logger'])) { + return $this->services['logger']; + } + + $this->services['logger'] = $instance = new \stdClass($a); + + $instance->handler = new \stdClass(($this->services['manager'] ?? $this->getManagerService())); + + return $instance; + } + + /** + * Gets the public 'manager' shared service. + * + * @return \stdClass + */ + protected function getManagerService() + { + $a = ($this->services['connection'] ?? $this->getConnectionService()); + + if (isset($this->services['manager'])) { + return $this->services['manager']; + } + + return $this->services['manager'] = new \stdClass($a); + } + + /** + * Gets the public 'manager2' shared service. + * + * @return \stdClass + */ + protected function getManager2Service() + { + $a = ($this->services['connection2'] ?? $this->getConnection2Service()); + + if (isset($this->services['manager2'])) { + return $this->services['manager2']; + } + + return $this->services['manager2'] = new \stdClass($a); + } + + /** + * Gets the public 'subscriber' shared service. + * + * @return \stdClass + */ + protected function getSubscriberService() + { + $a = ($this->services['manager'] ?? $this->getManagerService()); + + if (isset($this->services['subscriber'])) { + return $this->services['subscriber']; + } + + return $this->services['subscriber'] = new \stdClass($a); + } }