Skip to content

Commit 739a6a4

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '4.1'
* 4.1: [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 4.1.8 updated VERSION for 4.1.7 updated CHANGELOG for 4.1.7 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 36cea45 + e0bd358 commit 739a6a4

19 files changed

+272
-103
lines changed

‎Compiler/AutowirePass.php

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

370370
private function createTypeNotFoundMessage(TypedReference $reference, $label)
371371
{
372-
if (!$r = $this->container->getReflectionClass($type = $reference->getType(), false)) {
372+
$trackResources = $this->container->isTrackingResources();
373+
$this->container->setResourceTracking(false);
374+
try {
375+
if ($r = $this->container->getReflectionClass($type = $reference->getType(), false)) {
376+
$alternatives = $this->createTypeAlternatives($reference);
377+
}
378+
} finally {
379+
$this->container->setResourceTracking($trackResources);
380+
}
381+
382+
if (!$r) {
373383
// either $type does not exist or a parent class does not exist
374384
try {
375385
$resource = new ClassExistenceResource($type, false);
@@ -382,7 +392,6 @@ private function createTypeNotFoundMessage(TypedReference $reference, $label)
382392

383393
$message = sprintf('has type "%s" but this class %s.', $type, $parentMsg ? sprintf('is missing a parent class (%s)', $parentMsg) : 'was not found');
384394
} else {
385-
$alternatives = $this->createTypeAlternatives($reference);
386395
$message = $this->container->has($type) ? 'this service is abstract' : 'no such service exists';
387396
$message = sprintf('references %s "%s" but %s.%s', $r->isInterface() ? 'interface' : 'class', $type, $message, $alternatives);
388397

‎ContainerBuilder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ public function getReflectionClass(?string $class, bool $throw = true): ?\Reflec
346346
try {
347347
if (isset($this->classReflectors[$class])) {
348348
$classReflector = $this->classReflectors[$class];
349-
} elseif ($this->trackResources) {
349+
} elseif (class_exists(ClassExistenceResource::class)) {
350350
$resource = new ClassExistenceResource($class, false);
351351
$classReflector = $resource->isFresh(0) ? false : new \ReflectionClass($class);
352352
} 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
@@ -342,9 +342,13 @@ private function analyzeCircularReferences(array $edges, &$checkedNodes, &$curre
342342
if ($node->getValue() && ($edge->isLazy() || $edge->isWeak())) {
343343
// no-op
344344
} elseif (isset($currentPath[$id])) {
345+
$currentId = $id;
345346
foreach (array_reverse($currentPath) as $parentId) {
346-
$this->circularReferences[$parentId][$id] = $id;
347-
$id = $parentId;
347+
$this->circularReferences[$parentId][$currentId] = $currentId;
348+
if ($parentId === $id) {
349+
break;
350+
}
351+
$currentId = $parentId;
348352
}
349353
} elseif (!isset($checkedNodes[$id])) {
350354
$checkedNodes[$id] = true;
@@ -459,7 +463,7 @@ private function addServiceInclude(string $cId, Definition $definition): string
459463
* @throws InvalidArgumentException
460464
* @throws RuntimeException
461465
*/
462-
private function addServiceInstance(string $id, Definition $definition, string $isSimpleInstance): string
466+
private function addServiceInstance(string $id, Definition $definition, bool $isSimpleInstance): string
463467
{
464468
$class = $this->dumpValue($definition->getClass());
465469

@@ -585,7 +589,7 @@ private function addService(string $id, Definition $definition): array
585589
$this->definitionVariables = new \SplObjectStorage();
586590
$this->referenceVariables = array();
587591
$this->variableCount = 0;
588-
$this->definitionVariables[$definition] = $this->referenceVariables[$id] = new Variable('instance');
592+
$this->referenceVariables[$id] = new Variable('instance');
589593

590594
$return = array();
591595

@@ -658,22 +662,7 @@ protected function {$methodName}($lazyInitialization)
658662
$code .= sprintf(" @trigger_error(%s, E_USER_DEPRECATED);\n\n", $this->export($definition->getDeprecationMessage($id)));
659663
}
660664

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

678667
if ($asFile) {
679668
$code = implode("\n", array_map(function ($line) { return $line ? substr($line, 8) : $line; }, explode("\n", $code)));
@@ -687,35 +676,41 @@ protected function {$methodName}($lazyInitialization)
687676
return array($file, $code);
688677
}
689678

690-
private function addInlineVariables(string &$head, string &$tail, string $id, array $arguments, bool $forConstructor): bool
679+
private function addInlineVariables(string $id, Definition $definition, array $arguments, bool $forConstructor): string
691680
{
692-
$hasSelfRef = false;
681+
$code = '';
693682

694683
foreach ($arguments as $argument) {
695684
if (\is_array($argument)) {
696-
$hasSelfRef = $this->addInlineVariables($head, $tail, $id, $argument, $forConstructor) || $hasSelfRef;
685+
$code .= $this->addInlineVariables($id, $definition, $argument, $forConstructor);
697686
} elseif ($argument instanceof Reference) {
698-
$hasSelfRef = $this->addInlineReference($head, $id, $argument, $forConstructor) || $hasSelfRef;
687+
$code .= $this->addInlineReference($id, $definition, $argument, $forConstructor);
699688
} elseif ($argument instanceof Definition) {
700-
$hasSelfRef = $this->addInlineService($head, $tail, $id, $argument, $forConstructor) || $hasSelfRef;
689+
$code .= $this->addInlineService($id, $definition, $argument, $forConstructor);
701690
}
702691
}
703692

704-
return $hasSelfRef;
693+
return $code;
705694
}
706695

707-
private function addInlineReference(string &$code, string $id, string $targetId, bool $forConstructor): bool
696+
private function addInlineReference(string $id, Definition $definition, string $targetId, bool $forConstructor): string
708697
{
709-
$hasSelfRef = isset($this->circularReferences[$id][$targetId]);
698+
if ($id === $targetId) {
699+
return $this->addInlineService($id, $definition, $definition, $forConstructor);
700+
}
710701

711702
if ('service_container' === $targetId || isset($this->referenceVariables[$targetId])) {
712-
return $hasSelfRef;
703+
return '';
713704
}
714705

706+
$hasSelfRef = isset($this->circularReferences[$id][$targetId]);
707+
$forConstructor = $forConstructor && !isset($this->definitionVariables[$definition]);
715708
list($callCount, $behavior) = $this->serviceCalls[$targetId];
716709

717-
if (2 > $callCount && (!$hasSelfRef || !$forConstructor)) {
718-
return $hasSelfRef;
710+
$code = $hasSelfRef && !$forConstructor ? $this->addInlineService($id, $definition, $definition, $forConstructor) : '';
711+
712+
if (isset($this->referenceVariables[$targetId]) || (2 > $callCount && (!$hasSelfRef || !$forConstructor))) {
713+
return $code;
719714
}
720715

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

727722
if (!$hasSelfRef || !$forConstructor) {
728-
return $hasSelfRef;
723+
return $code;
729724
}
730725

731726
$code .= sprintf(<<<'EOTXT'
@@ -740,46 +735,56 @@ private function addInlineReference(string &$code, string $id, string $targetId,
740735
$id
741736
);
742737

743-
return false;
738+
return $code;
744739
}
745740

746-
private function addInlineService(string &$head, string &$tail, string $id, Definition $definition, bool $forConstructor): bool
741+
private function addInlineService(string $id, Definition $definition, Definition $inlineDef = null, bool $forConstructor = true): string
747742
{
748-
if (isset($this->definitionVariables[$definition])) {
749-
return false;
743+
$isSimpleInstance = $isRootInstance = null === $inlineDef;
744+
745+
if (isset($this->definitionVariables[$inlineDef = $inlineDef ?: $definition])) {
746+
return '';
750747
}
751748

752-
$arguments = array($definition->getArguments(), $definition->getFactory());
749+
$arguments = array($inlineDef->getArguments(), $inlineDef->getFactory());
753750

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

758-
$name = $this->getNextVariableName();
759-
$this->definitionVariables[$definition] = new Variable($name);
753+
if ($arguments = array_filter(array($inlineDef->getProperties(), $inlineDef->getMethodCalls(), $inlineDef->getConfigurator()))) {
754+
$isSimpleInstance = false;
755+
} elseif ($definition !== $inlineDef && 2 > $this->inlinedDefinitions[$inlineDef]) {
756+
return $code;
757+
}
760758

761-
$code = '';
762-
if ($forConstructor) {
763-
$hasSelfRef = $this->addInlineVariables($code, $tail, $id, $arguments, $forConstructor);
759+
if (isset($this->definitionVariables[$inlineDef])) {
760+
$isSimpleInstance = false;
764761
} else {
765-
$hasSelfRef = $this->addInlineVariables($code, $code, $id, $arguments, $forConstructor);
766-
}
767-
$code .= $this->addNewInstance($definition, ' $'.$name.' = ', $id);
768-
$hasSelfRef && !$forConstructor ? $tail .= ('' !== $tail ? "\n" : '').$code : $head .= ('' !== $head ? "\n" : '').$code;
762+
$name = $definition === $inlineDef ? 'instance' : $this->getNextVariableName();
763+
$this->definitionVariables[$inlineDef] = new Variable($name);
764+
$code .= '' !== $code ? "\n" : '';
769765

770-
$code = '';
771-
$arguments = array($definition->getProperties(), $definition->getMethodCalls(), $definition->getConfigurator());
772-
$hasSelfRef = $this->addInlineVariables($code, $code, $id, $arguments, false) || $hasSelfRef;
766+
if ('instance' === $name) {
767+
$code .= $this->addServiceInstance($id, $definition, $isSimpleInstance);
768+
} else {
769+
$code .= $this->addNewInstance($inlineDef, ' $'.$name.' = ', $id);
770+
}
773771

774-
$code .= '' !== $code ? "\n" : '';
775-
$code .= $this->addServiceProperties($definition, $name);
776-
$code .= $this->addServiceMethodCalls($definition, $name);
777-
$code .= $this->addServiceConfigurator($definition, $name);
778-
if ('' !== $code) {
779-
$hasSelfRef ? $tail .= ('' !== $tail ? "\n" : '').$code : $head .= $code;
772+
if ('' !== $inline = $this->addInlineVariables($id, $definition, $arguments, false)) {
773+
$code .= "\n".$inline."\n";
774+
} elseif ($arguments && 'instance' === $name) {
775+
$code .= "\n";
776+
}
777+
778+
$code .= $this->addServiceProperties($inlineDef, $name);
779+
$code .= $this->addServiceMethodCalls($inlineDef, $name);
780+
$code .= $this->addServiceConfigurator($inlineDef, $name);
781+
}
782+
783+
if ($isRootInstance && !$isSimpleInstance) {
784+
$code .= "\n return \$instance;\n";
780785
}
781786

782-
return $hasSelfRef;
787+
return $code;
783788
}
784789

785790
private function addServices(array &$services = null): 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
@@ -880,6 +880,8 @@ public function testAlmostCircular($visibility)
880880

881881
$foo6 = $container->get('foo6');
882882
$this->assertEquals((object) array('bar6' => (object) array()), $foo6);
883+
884+
$this->assertInstanceOf(\stdClass::class, $container->get('root'));
883885
}
884886

885887
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)