Skip to content

Commit e0bd358

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '3.4' into 4.1
* 3.4: [VarDumper] fix dump of closures created from callables [DI] fix dumping inlined services Add framework asset changes to upgrade 3.0 guide [Travis] Bump ext-mongodb to 1.5.2 on Travis [DI] dont track classes/interfaces used to compute autowiring error messages [DI] fix GraphvizDumper ignoring inline definitions bumped Symfony version to 3.4.19 updated VERSION for 3.4.18 updated CHANGELOG for 3.4.18 bumped Symfony version to 2.8.48 updated VERSION for 2.8.47 update CONTRIBUTORS for 2.8.47 updated CHANGELOG for 2.8.47 Fix ini_get() for boolean values
2 parents e72ee2c + 9d2e5ef commit e0bd358

19 files changed

+272
-103
lines changed

‎Compiler/AutowirePass.php

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,17 @@ private function set(string $type, string $id)
351351

352352
private function createTypeNotFoundMessage(TypedReference $reference, $label)
353353
{
354-
if (!$r = $this->container->getReflectionClass($type = $reference->getType(), false)) {
354+
$trackResources = $this->container->isTrackingResources();
355+
$this->container->setResourceTracking(false);
356+
try {
357+
if ($r = $this->container->getReflectionClass($type = $reference->getType(), false)) {
358+
$alternatives = $this->createTypeAlternatives($reference);
359+
}
360+
} finally {
361+
$this->container->setResourceTracking($trackResources);
362+
}
363+
364+
if (!$r) {
355365
// either $type does not exist or a parent class does not exist
356366
try {
357367
$resource = new ClassExistenceResource($type, false);
@@ -364,7 +374,6 @@ private function createTypeNotFoundMessage(TypedReference $reference, $label)
364374

365375
$message = sprintf('has type "%s" but this class %s.', $type, $parentMsg ? sprintf('is missing a parent class (%s)', $parentMsg) : 'was not found');
366376
} else {
367-
$alternatives = $this->createTypeAlternatives($reference);
368377
$message = $this->container->has($type) ? 'this service is abstract' : 'no such service exists';
369378
$message = sprintf('references %s "%s" but %s.%s', $r->isInterface() ? 'interface' : 'class', $type, $message, $alternatives);
370379

‎ContainerBuilder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ public function getReflectionClass(?string $class, bool $throw = true): ?\Reflec
344344
try {
345345
if (isset($this->classReflectors[$class])) {
346346
$classReflector = $this->classReflectors[$class];
347-
} elseif ($this->trackResources) {
347+
} elseif (class_exists(ClassExistenceResource::class)) {
348348
$resource = new ClassExistenceResource($class, false);
349349
$classReflector = $resource->isFresh(0) ? false : new \ReflectionClass($class);
350350
} else {

‎Dumper/GraphvizDumper.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,14 @@ private function findEdges(string $id, array $arguments, bool $required, string
132132
$edges[] = array('name' => $name, 'required' => $required, 'to' => $argument, 'lazy' => $lazyEdge);
133133
} elseif ($argument instanceof ArgumentInterface) {
134134
$edges = array_merge($edges, $this->findEdges($id, $argument->getValues(), $required, $name, true));
135+
} elseif ($argument instanceof Definition) {
136+
$edges = array_merge($edges,
137+
$this->findEdges($id, $argument->getArguments(), $required, ''),
138+
$this->findEdges($id, $argument->getProperties(), false, '')
139+
);
140+
foreach ($argument->getMethodCalls() as $call) {
141+
$edges = array_merge($edges, $this->findEdges($id, $call[1], false, $call[0].'()'));
142+
}
135143
} elseif (\is_array($argument)) {
136144
$edges = array_merge($edges, $this->findEdges($id, $argument, $required, $name, $lazy));
137145
}

‎Dumper/PhpDumper.php

Lines changed: 64 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -312,9 +312,13 @@ private function analyzeCircularReferences(array $edges, &$checkedNodes, &$curre
312312
if ($node->getValue() && ($edge->isLazy() || $edge->isWeak())) {
313313
// no-op
314314
} elseif (isset($currentPath[$id])) {
315+
$currentId = $id;
315316
foreach (array_reverse($currentPath) as $parentId) {
316-
$this->circularReferences[$parentId][$id] = $id;
317-
$id = $parentId;
317+
$this->circularReferences[$parentId][$currentId] = $currentId;
318+
if ($parentId === $id) {
319+
break;
320+
}
321+
$currentId = $parentId;
318322
}
319323
} elseif (!isset($checkedNodes[$id])) {
320324
$checkedNodes[$id] = true;
@@ -429,7 +433,7 @@ private function addServiceInclude(string $cId, Definition $definition): string
429433
* @throws InvalidArgumentException
430434
* @throws RuntimeException
431435
*/
432-
private function addServiceInstance(string $id, Definition $definition, string $isSimpleInstance): string
436+
private function addServiceInstance(string $id, Definition $definition, bool $isSimpleInstance): string
433437
{
434438
$class = $this->dumpValue($definition->getClass());
435439

@@ -552,7 +556,7 @@ private function addService(string $id, Definition $definition, string &$file =
552556
$this->definitionVariables = new \SplObjectStorage();
553557
$this->referenceVariables = array();
554558
$this->variableCount = 0;
555-
$this->definitionVariables[$definition] = $this->referenceVariables[$id] = new Variable('instance');
559+
$this->referenceVariables[$id] = new Variable('instance');
556560

557561
$return = array();
558562

@@ -630,22 +634,7 @@ protected function {$methodName}($lazyInitialization)
630634
$code .= sprintf(" @trigger_error(%s, E_USER_DEPRECATED);\n\n", $this->export($definition->getDeprecationMessage($id)));
631635
}
632636

633-
$head = $tail = '';
634-
$arguments = array($definition->getArguments(), $definition->getFactory());
635-
$this->addInlineVariables($head, $tail, $id, $arguments, true);
636-
$code .= '' !== $head ? $head."\n" : '';
637-
638-
if ($arguments = array_filter(array($definition->getProperties(), $definition->getMethodCalls(), $definition->getConfigurator()))) {
639-
$this->addInlineVariables($tail, $tail, $id, $arguments, false);
640-
641-
$tail .= '' !== $tail ? "\n" : '';
642-
$tail .= $this->addServiceProperties($definition);
643-
$tail .= $this->addServiceMethodCalls($definition);
644-
$tail .= $this->addServiceConfigurator($definition);
645-
}
646-
647-
$code .= $this->addServiceInstance($id, $definition, '' === $tail)
648-
.('' !== $tail ? "\n".$tail."\n return \$instance;\n" : '');
637+
$code .= $this->addInlineService($id, $definition);
649638

650639
if ($asFile) {
651640
$code = implode("\n", array_map(function ($line) { return $line ? substr($line, 8) : $line; }, explode("\n", $code)));
@@ -659,35 +648,41 @@ protected function {$methodName}($lazyInitialization)
659648
return $code;
660649
}
661650

662-
private function addInlineVariables(string &$head, string &$tail, string $id, array $arguments, bool $forConstructor): bool
651+
private function addInlineVariables(string $id, Definition $definition, array $arguments, bool $forConstructor): string
663652
{
664-
$hasSelfRef = false;
653+
$code = '';
665654

666655
foreach ($arguments as $argument) {
667656
if (\is_array($argument)) {
668-
$hasSelfRef = $this->addInlineVariables($head, $tail, $id, $argument, $forConstructor) || $hasSelfRef;
657+
$code .= $this->addInlineVariables($id, $definition, $argument, $forConstructor);
669658
} elseif ($argument instanceof Reference) {
670-
$hasSelfRef = $this->addInlineReference($head, $id, $argument, $forConstructor) || $hasSelfRef;
659+
$code .= $this->addInlineReference($id, $definition, $argument, $forConstructor);
671660
} elseif ($argument instanceof Definition) {
672-
$hasSelfRef = $this->addInlineService($head, $tail, $id, $argument, $forConstructor) || $hasSelfRef;
661+
$code .= $this->addInlineService($id, $definition, $argument, $forConstructor);
673662
}
674663
}
675664

676-
return $hasSelfRef;
665+
return $code;
677666
}
678667

679-
private function addInlineReference(string &$code, string $id, string $targetId, bool $forConstructor): bool
668+
private function addInlineReference(string $id, Definition $definition, string $targetId, bool $forConstructor): string
680669
{
681-
$hasSelfRef = isset($this->circularReferences[$id][$targetId]);
670+
if ($id === $targetId) {
671+
return $this->addInlineService($id, $definition, $definition, $forConstructor);
672+
}
682673

683674
if ('service_container' === $targetId || isset($this->referenceVariables[$targetId])) {
684-
return $hasSelfRef;
675+
return '';
685676
}
686677

678+
$hasSelfRef = isset($this->circularReferences[$id][$targetId]);
679+
$forConstructor = $forConstructor && !isset($this->definitionVariables[$definition]);
687680
list($callCount, $behavior) = $this->serviceCalls[$targetId];
688681

689-
if (2 > $callCount && (!$hasSelfRef || !$forConstructor)) {
690-
return $hasSelfRef;
682+
$code = $hasSelfRef && !$forConstructor ? $this->addInlineService($id, $definition, $definition, $forConstructor) : '';
683+
684+
if (isset($this->referenceVariables[$targetId]) || (2 > $callCount && (!$hasSelfRef || !$forConstructor))) {
685+
return $code;
691686
}
692687

693688
$name = $this->getNextVariableName();
@@ -697,7 +692,7 @@ private function addInlineReference(string &$code, string $id, string $targetId,
697692
$code .= sprintf(" \$%s = %s;\n", $name, $this->getServiceCall($targetId, $reference));
698693

699694
if (!$hasSelfRef || !$forConstructor) {
700-
return $hasSelfRef;
695+
return $code;
701696
}
702697

703698
$code .= sprintf(<<<'EOTXT'
@@ -712,46 +707,56 @@ private function addInlineReference(string &$code, string $id, string $targetId,
712707
$id
713708
);
714709

715-
return false;
710+
return $code;
716711
}
717712

718-
private function addInlineService(string &$head, string &$tail, string $id, Definition $definition, bool $forConstructor): bool
713+
private function addInlineService(string $id, Definition $definition, Definition $inlineDef = null, bool $forConstructor = true): string
719714
{
720-
if (isset($this->definitionVariables[$definition])) {
721-
return false;
715+
$isSimpleInstance = $isRootInstance = null === $inlineDef;
716+
717+
if (isset($this->definitionVariables[$inlineDef = $inlineDef ?: $definition])) {
718+
return '';
722719
}
723720

724-
$arguments = array($definition->getArguments(), $definition->getFactory());
721+
$arguments = array($inlineDef->getArguments(), $inlineDef->getFactory());
725722

726-
if (2 > $this->inlinedDefinitions[$definition] && !$definition->getMethodCalls() && !$definition->getProperties() && !$definition->getConfigurator()) {
727-
return $this->addInlineVariables($head, $tail, $id, $arguments, $forConstructor);
728-
}
723+
$code = $this->addInlineVariables($id, $definition, $arguments, $forConstructor);
729724

730-
$name = $this->getNextVariableName();
731-
$this->definitionVariables[$definition] = new Variable($name);
725+
if ($arguments = array_filter(array($inlineDef->getProperties(), $inlineDef->getMethodCalls(), $inlineDef->getConfigurator()))) {
726+
$isSimpleInstance = false;
727+
} elseif ($definition !== $inlineDef && 2 > $this->inlinedDefinitions[$inlineDef]) {
728+
return $code;
729+
}
732730

733-
$code = '';
734-
if ($forConstructor) {
735-
$hasSelfRef = $this->addInlineVariables($code, $tail, $id, $arguments, $forConstructor);
731+
if (isset($this->definitionVariables[$inlineDef])) {
732+
$isSimpleInstance = false;
736733
} else {
737-
$hasSelfRef = $this->addInlineVariables($code, $code, $id, $arguments, $forConstructor);
738-
}
739-
$code .= $this->addNewInstance($definition, '$'.$name, ' = ', $id);
740-
$hasSelfRef && !$forConstructor ? $tail .= ('' !== $tail ? "\n" : '').$code : $head .= ('' !== $head ? "\n" : '').$code;
734+
$name = $definition === $inlineDef ? 'instance' : $this->getNextVariableName();
735+
$this->definitionVariables[$inlineDef] = new Variable($name);
736+
$code .= '' !== $code ? "\n" : '';
741737

742-
$code = '';
743-
$arguments = array($definition->getProperties(), $definition->getMethodCalls(), $definition->getConfigurator());
744-
$hasSelfRef = $this->addInlineVariables($code, $code, $id, $arguments, false) || $hasSelfRef;
738+
if ('instance' === $name) {
739+
$code .= $this->addServiceInstance($id, $definition, $isSimpleInstance);
740+
} else {
741+
$code .= $this->addNewInstance($inlineDef, '$'.$name, ' = ', $id);
742+
}
745743

746-
$code .= '' !== $code ? "\n" : '';
747-
$code .= $this->addServiceProperties($definition, $name);
748-
$code .= $this->addServiceMethodCalls($definition, $name);
749-
$code .= $this->addServiceConfigurator($definition, $name);
750-
if ('' !== $code) {
751-
$hasSelfRef ? $tail .= ('' !== $tail ? "\n" : '').$code : $head .= $code;
744+
if ('' !== $inline = $this->addInlineVariables($id, $definition, $arguments, false)) {
745+
$code .= "\n".$inline."\n";
746+
} elseif ($arguments && 'instance' === $name) {
747+
$code .= "\n";
748+
}
749+
750+
$code .= $this->addServiceProperties($inlineDef, $name);
751+
$code .= $this->addServiceMethodCalls($inlineDef, $name);
752+
$code .= $this->addServiceConfigurator($inlineDef, $name);
753+
}
754+
755+
if ($isRootInstance && !$isSimpleInstance) {
756+
$code .= "\n return \$instance;\n";
752757
}
753758

754-
return $hasSelfRef;
759+
return $code;
755760
}
756761

757762
private function addServices(): string

‎Tests/ContainerBuilderTest.php

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

13631363
$foo6 = $container->get('foo6');
13641364
$this->assertEquals((object) array('bar6' => (object) array()), $foo6);
1365+
1366+
$this->assertInstanceOf(\stdClass::class, $container->get('root'));
13651367
}
13661368

13671369
public function provideAlmostCircular()

‎Tests/Dumper/GraphvizDumperTest.php

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\DependencyInjection\ContainerBuilder;
16+
use Symfony\Component\DependencyInjection\Definition;
1617
use Symfony\Component\DependencyInjection\Dumper\GraphvizDumper;
18+
use Symfony\Component\DependencyInjection\Reference;
1719

1820
class GraphvizDumperTest extends TestCase
1921
{
@@ -32,11 +34,11 @@ public function testDump()
3234

3335
$container = include self::$fixturesPath.'/containers/container9.php';
3436
$dumper = new GraphvizDumper($container);
35-
$this->assertEquals(str_replace('%path%', __DIR__, file_get_contents(self::$fixturesPath.'/graphviz/services9.dot')), $dumper->dump(), '->dump() dumps services');
37+
$this->assertStringEqualsFile(self::$fixturesPath.'/graphviz/services9.dot', $dumper->dump(), '->dump() dumps services');
3638

3739
$container = include self::$fixturesPath.'/containers/container10.php';
3840
$dumper = new GraphvizDumper($container);
39-
$this->assertEquals(str_replace('%path%', __DIR__, file_get_contents(self::$fixturesPath.'/graphviz/services10.dot')), $dumper->dump(), '->dump() dumps services');
41+
$this->assertStringEqualsFile(self::$fixturesPath.'/graphviz/services10.dot', $dumper->dump(), '->dump() dumps services');
4042

4143
$container = include self::$fixturesPath.'/containers/container10.php';
4244
$dumper = new GraphvizDumper($container);
@@ -47,28 +49,40 @@ public function testDump()
4749
'node.instance' => array('fillcolor' => 'green', 'style' => 'empty'),
4850
'node.definition' => array('fillcolor' => 'grey'),
4951
'node.missing' => array('fillcolor' => 'red', 'style' => 'empty'),
50-
)), str_replace('%path%', __DIR__, file_get_contents(self::$fixturesPath.'/graphviz/services10-1.dot')), '->dump() dumps services');
52+
)), file_get_contents(self::$fixturesPath.'/graphviz/services10-1.dot'), '->dump() dumps services');
5153
}
5254

5355
public function testDumpWithFrozenContainer()
5456
{
5557
$container = include self::$fixturesPath.'/containers/container13.php';
5658
$dumper = new GraphvizDumper($container);
57-
$this->assertEquals(str_replace('%path%', __DIR__, file_get_contents(self::$fixturesPath.'/graphviz/services13.dot')), $dumper->dump(), '->dump() dumps services');
59+
$this->assertStringEqualsFile(self::$fixturesPath.'/graphviz/services13.dot', $dumper->dump(), '->dump() dumps services');
5860
}
5961

6062
public function testDumpWithFrozenCustomClassContainer()
6163
{
6264
$container = include self::$fixturesPath.'/containers/container14.php';
6365
$dumper = new GraphvizDumper($container);
64-
$this->assertEquals(str_replace('%path%', __DIR__, file_get_contents(self::$fixturesPath.'/graphviz/services14.dot')), $dumper->dump(), '->dump() dumps services');
66+
$this->assertStringEqualsFile(self::$fixturesPath.'/graphviz/services14.dot', $dumper->dump(), '->dump() dumps services');
6567
}
6668

6769
public function testDumpWithUnresolvedParameter()
6870
{
6971
$container = include self::$fixturesPath.'/containers/container17.php';
7072
$dumper = new GraphvizDumper($container);
7173

72-
$this->assertEquals(str_replace('%path%', __DIR__, file_get_contents(self::$fixturesPath.'/graphviz/services17.dot')), $dumper->dump(), '->dump() dumps services');
74+
$this->assertStringEqualsFile(self::$fixturesPath.'/graphviz/services17.dot', $dumper->dump(), '->dump() dumps services');
75+
}
76+
77+
public function testDumpWithInlineDefinition()
78+
{
79+
$container = new ContainerBuilder();
80+
$container->register('foo', 'stdClass')->addArgument(
81+
(new Definition('stdClass'))->addArgument(new Reference('bar'))
82+
);
83+
$container->register('bar', 'stdClass');
84+
$dumper = new GraphvizDumper($container);
85+
86+
$this->assertStringEqualsFile(self::$fixturesPath.'/graphviz/services_inline.dot', $dumper->dump(), '->dump() dumps nested references');
7387
}
7488
}

‎Tests/Dumper/PhpDumperTest.php

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

874874
$foo6 = $container->get('foo6');
875875
$this->assertEquals((object) array('bar6' => (object) array()), $foo6);
876+
877+
$this->assertInstanceOf(\stdClass::class, $container->get('root'));
876878
}
877879

878880
public function provideAlmostCircular()

‎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+
}

0 commit comments

Comments
 (0)