Skip to content

Commit a97606d

Browse files
nicolas-grekasnicolas-grekas
committed
[DI] fix dumping inlined services
1 parent 3ed98de commit a97606d

12 files changed

+344
-86
lines changed

‎src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php‎

Lines changed: 63 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -306,9 +306,13 @@ private function analyzeCircularReferences(array $edges, &$checkedNodes, &$curre
306306
if ($node->getValue() && ($edge->isLazy() || $edge->isWeak())) {
307307
// no-op
308308
} elseif (isset($currentPath[$id])) {
309+
$currentId = $id;
309310
foreach (array_reverse($currentPath) as $parentId) {
310-
$this->circularReferences[$parentId][$id] = $id;
311-
$id = $parentId;
311+
$this->circularReferences[$parentId][$currentId] = $currentId;
312+
if ($parentId === $id) {
313+
break;
314+
}
315+
$currentId = $parentId;
312316
}
313317
} elseif (!isset($checkedNodes[$id])) {
314318
$checkedNodes[$id] = true;
@@ -591,7 +595,7 @@ private function addService($id, Definition $definition, &$file = null)
591595
$this->definitionVariables = new \SplObjectStorage();
592596
$this->referenceVariables = array();
593597
$this->variableCount = 0;
594-
$this->definitionVariables[$definition] = $this->referenceVariables[$id] = new Variable('instance');
598+
$this->referenceVariables[$id] = new Variable('instance');
595599

596600
$return = array();
597601

@@ -663,22 +667,7 @@ protected function {$methodName}($lazyInitialization)
663667
$code .= sprintf(" @trigger_error(%s, E_USER_DEPRECATED);\n\n", $this->export($definition->getDeprecationMessage($id)));
664668
}
665669

666-
$head = $tail = '';
667-
$arguments = array($definition->getArguments(), $definition->getFactory());
668-
$this->addInlineVariables($head, $tail, $id, $arguments, true);
669-
$code .= '' !== $head ? $head."\n" : '';
670-
671-
if ($arguments = array_filter(array($definition->getProperties(), $definition->getMethodCalls(), $definition->getConfigurator()))) {
672-
$this->addInlineVariables($tail, $tail, $id, $arguments, false);
673-
674-
$tail .= '' !== $tail ? "\n" : '';
675-
$tail .= $this->addServiceProperties($definition);
676-
$tail .= $this->addServiceMethodCalls($definition);
677-
$tail .= $this->addServiceConfigurator($definition);
678-
}
679-
680-
$code .= $this->addServiceInstance($id, $definition, '' === $tail)
681-
.('' !== $tail ? "\n".$tail."\n return \$instance;\n" : '');
670+
$code .= $this->addInlineService($id, $definition);
682671

683672
if ($asFile) {
684673
$code = implode("\n", array_map(function ($line) { return $line ? substr($line, 8) : $line; }, explode("\n", $code)));
@@ -692,35 +681,41 @@ protected function {$methodName}($lazyInitialization)
692681
return $code;
693682
}
694683

695-
private function addInlineVariables(&$head, &$tail, $id, array $arguments, $forConstructor)
684+
private function addInlineVariables($id, Definition $definition, array $arguments, $forConstructor)
696685
{
697-
$hasSelfRef = false;
686+
$code = '';
698687

699688
foreach ($arguments as $argument) {
700689
if (\is_array($argument)) {
701-
$hasSelfRef = $this->addInlineVariables($head, $tail, $id, $argument, $forConstructor) || $hasSelfRef;
690+
$code .= $this->addInlineVariables($id, $definition, $argument, $forConstructor);
702691
} elseif ($argument instanceof Reference) {
703-
$hasSelfRef = $this->addInlineReference($head, $id, $this->container->normalizeId($argument), $forConstructor) || $hasSelfRef;
692+
$code .= $this->addInlineReference($id, $definition, $this->container->normalizeId($argument), $forConstructor);
704693
} elseif ($argument instanceof Definition) {
705-
$hasSelfRef = $this->addInlineService($head, $tail, $id, $argument, $forConstructor) || $hasSelfRef;
694+
$code .= $this->addInlineService($id, $definition, $argument, $forConstructor);
706695
}
707696
}
708697

709-
return $hasSelfRef;
698+
return $code;
710699
}
711700

712-
private function addInlineReference(&$code, $id, $targetId, $forConstructor)
701+
private function addInlineReference($id, Definition $definition, $targetId, $forConstructor)
713702
{
714-
$hasSelfRef = isset($this->circularReferences[$id][$targetId]);
703+
if ($id === $targetId) {
704+
return $this->addInlineService($id, $definition, $definition, $forConstructor);
705+
}
715706

716707
if ('service_container' === $targetId || isset($this->referenceVariables[$targetId])) {
717-
return $hasSelfRef;
708+
return '';
718709
}
719710

711+
$hasSelfRef = isset($this->circularReferences[$id][$targetId]);
712+
$forConstructor = $forConstructor && !isset($this->definitionVariables[$definition]);
720713
list($callCount, $behavior) = $this->serviceCalls[$targetId];
721714

722-
if (2 > $callCount && (!$hasSelfRef || !$forConstructor)) {
723-
return $hasSelfRef;
715+
$code = $hasSelfRef && !$forConstructor ? $this->addInlineService($id, $definition, $definition, $forConstructor) : '';
716+
717+
if (isset($this->referenceVariables[$targetId]) || (2 > $callCount && (!$hasSelfRef || !$forConstructor))) {
718+
return $code;
724719
}
725720

726721
$name = $this->getNextVariableName();
@@ -730,7 +725,7 @@ private function addInlineReference(&$code, $id, $targetId, $forConstructor)
730725
$code .= sprintf(" \$%s = %s;\n", $name, $this->getServiceCall($targetId, $reference));
731726

732727
if (!$hasSelfRef || !$forConstructor) {
733-
return $hasSelfRef;
728+
return $code;
734729
}
735730

736731
$code .= sprintf(<<<'EOTXT'
@@ -745,46 +740,56 @@ private function addInlineReference(&$code, $id, $targetId, $forConstructor)
745740
$id
746741
);
747742

748-
return false;
743+
return $code;
749744
}
750745

751-
private function addInlineService(&$head, &$tail, $id, Definition $definition, $forConstructor)
746+
private function addInlineService($id, Definition $definition, Definition $inlineDef = null, $forConstructor = true)
752747
{
753-
if (isset($this->definitionVariables[$definition])) {
754-
return false;
748+
$isSimpleInstance = $isRootInstance = null === $inlineDef;
749+
750+
if (isset($this->definitionVariables[$inlineDef = $inlineDef ?: $definition])) {
751+
return '';
755752
}
756753

757-
$arguments = array($definition->getArguments(), $definition->getFactory());
754+
$arguments = array($inlineDef->getArguments(), $inlineDef->getFactory());
758755

759-
if (2 > $this->inlinedDefinitions[$definition] && !$definition->getMethodCalls() && !$definition->getProperties() && !$definition->getConfigurator()) {
760-
return $this->addInlineVariables($head, $tail, $id, $arguments, $forConstructor);
761-
}
756+
$code = $this->addInlineVariables($id, $definition, $arguments, $forConstructor);
762757

763-
$name = $this->getNextVariableName();
764-
$this->definitionVariables[$definition] = new Variable($name);
758+
if ($arguments = array_filter(array($inlineDef->getProperties(), $inlineDef->getMethodCalls(), $inlineDef->getConfigurator()))) {
759+
$isSimpleInstance = false;
760+
} elseif ($definition !== $inlineDef && 2 > $this->inlinedDefinitions[$inlineDef]) {
761+
return $code;
762+
}
765763

766-
$code = '';
767-
if ($forConstructor) {
768-
$hasSelfRef = $this->addInlineVariables($code, $tail, $id, $arguments, $forConstructor);
764+
if (isset($this->definitionVariables[$inlineDef])) {
765+
$isSimpleInstance = false;
769766
} else {
770-
$hasSelfRef = $this->addInlineVariables($code, $code, $id, $arguments, $forConstructor);
771-
}
772-
$code .= $this->addNewInstance($definition, '$'.$name, ' = ', $id);
773-
$hasSelfRef && !$forConstructor ? $tail .= ('' !== $tail ? "\n" : '').$code : $head .= ('' !== $head ? "\n" : '').$code;
767+
$name = $definition === $inlineDef ? 'instance' : $this->getNextVariableName();
768+
$this->definitionVariables[$inlineDef] = new Variable($name);
769+
$code .= '' !== $code ? "\n" : '';
774770

775-
$code = '';
776-
$arguments = array($definition->getProperties(), $definition->getMethodCalls(), $definition->getConfigurator());
777-
$hasSelfRef = $this->addInlineVariables($code, $code, $id, $arguments, false) || $hasSelfRef;
771+
if ('instance' === $name) {
772+
$code .= $this->addServiceInstance($id, $definition, $isSimpleInstance);
773+
} else {
774+
$code .= $this->addNewInstance($inlineDef, '$'.$name, ' = ', $id);
775+
}
778776

779-
$code .= '' !== $code ? "\n" : '';
780-
$code .= $this->addServiceProperties($definition, $name);
781-
$code .= $this->addServiceMethodCalls($definition, $name);
782-
$code .= $this->addServiceConfigurator($definition, $name);
783-
if ('' !== $code) {
784-
$hasSelfRef ? $tail .= ('' !== $tail ? "\n" : '').$code : $head .= $code;
777+
if ('' !== $inline = $this->addInlineVariables($id, $definition, $arguments, false)) {
778+
$code .= "\n".$inline."\n";
779+
} elseif ($arguments && 'instance' === $name) {
780+
$code .= "\n";
781+
}
782+
783+
$code .= $this->addServiceProperties($inlineDef, $name);
784+
$code .= $this->addServiceMethodCalls($inlineDef, $name);
785+
$code .= $this->addServiceConfigurator($inlineDef, $name);
786+
}
787+
788+
if ($isRootInstance && !$isSimpleInstance) {
789+
$code .= "\n return \$instance;\n";
785790
}
786791

787-
return $hasSelfRef;
792+
return $code;
788793
}
789794

790795
/**

‎src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1388,6 +1388,8 @@ public function testAlmostCircular($visibility)
13881388

13891389
$foo6 = $container->get('foo6');
13901390
$this->assertEquals((object) array('bar6' => (object) array()), $foo6);
1391+
1392+
$this->assertInstanceOf(\stdClass::class, $container->get('root'));
13911393
}
13921394

13931395
public function provideAlmostCircular()

‎src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,8 @@ public function testAlmostCircular($visibility)
833833

834834
$foo6 = $container->get('foo6');
835835
$this->assertEquals((object) array('bar6' => (object) array()), $foo6);
836+
837+
$this->assertInstanceOf(\stdClass::class, $container->get('root'));
836838
}
837839

838840
public function provideAlmostCircular()

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooForCircularWithAddCalls.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
13+
14+
class FooForCircularWithAddCalls
15+
{
16+
public function call(\stdClass $argument)
17+
{
18+
}
19+
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container_almost_circular.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use Symfony\Component\DependencyInjection\Definition;
55
use Symfony\Component\DependencyInjection\Dumper\PhpDumper;
66
use Symfony\Component\DependencyInjection\Reference;
7+
use Symfony\Component\DependencyInjection\Tests\Fixtures\FooForCircularWithAddCalls;
78

89
$public = 'public' === $visibility;
910
$container = new ContainerBuilder();
@@ -115,4 +116,33 @@
115116
->setPublic(true)
116117
->setProperty('bar6', new Reference('bar6'));
117118

119+
// provided by Christian Schiffler
120+
121+
$container
122+
->register('root', 'stdClass')
123+
->setArguments([new Reference('level2'), new Reference('multiuse1')])
124+
->setPublic(true);
125+
126+
$container
127+
->register('level2', FooForCircularWithAddCalls::class)
128+
->addMethodCall('call', [new Reference('level3')]);
129+
130+
$container->register('multiuse1', 'stdClass');
131+
132+
$container
133+
->register('level3', 'stdClass')
134+
->addArgument(new Reference('level4'));
135+
136+
$container
137+
->register('level4', 'stdClass')
138+
->setArguments([new Reference('multiuse1'), new Reference('level5')]);
139+
140+
$container
141+
->register('level5', 'stdClass')
142+
->addArgument(new Reference('level6'));
143+
144+
$container
145+
->register('level6', FooForCircularWithAddCalls::class)
146+
->addMethodCall('call', [new Reference('level5')]);
147+
118148
return $container;

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,6 @@ use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
158158
$this->services['foo_with_inline'] = $instance = new \Foo();
159159

160160
$a = new \Bar();
161-
162161
$a->pub = 'pub';
163162
$a->setBaz(${($_ = isset($this->services['baz']) ? $this->services['baz'] : $this->load('getBazService.php')) && false ?: '_'});
164163

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,6 @@ protected function getFooWithInlineService()
264264
$this->services['foo_with_inline'] = $instance = new \Foo();
265265

266266
$a = new \Bar();
267-
268267
$a->pub = 'pub';
269268
$a->setBaz(${($_ = isset($this->services['baz']) ? $this->services['baz'] : $this->getBazService()) && false ?: '_'});
270269

0 commit comments

Comments
 (0)