8000 [DI] compute autowiring error messages lazily · symfony/symfony@614e793 · GitHub
[go: up one dir, main page]
Skip to content

Commit 614e793

Browse files
nicolas-grekasnicolas-grekas
committed
[DI] compute autowiring error messages lazily
1 parent 8d277ce commit 614e793

File tree

11 files changed

+98
-54
lines changed

11 files changed

+98
-54
lines changed

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TestServiceContainerWeakRefPass.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function process(ContainerBuilder $container)
3131
$definitions = $container->getDefinitions();
3232

3333
foreach ($definitions as $id => $definition) {
34-
if ($id && '.' !== $id[0] && (!$definition->isPublic() || $definition->isPrivate()) && !$definition->getErrors() && !$definition->isAbstract()) {
34+
if ($id && '.' !== $id[0] && (!$definition->isPublic() || $definition->isPrivate()) && !$definition->hasErrors() && !$definition->isAbstract()) {
3535
$privateServices[$id] = new Reference($id, ContainerBuilder::IGNORE_ON_UNINITIALIZED_REFERENCE);
3636
}
3737
}
@@ -43,7 +43,7 @@ public function process(ContainerBuilder $container)
4343
while (isset($aliases[$target = (string) $alias])) {
4444
$alias = $aliases[$target];
4545
}
46-
if (isset($definitions[$target]) && !$definitions[$target]->getErrors() && !$definitions[$target]->isAbstract()) {
46+
if (isset($definitions[$target]) && !$definitions[$target]->hasErrors() && !$definitions[$target]->isAbstract()) {
4747
$privateServices[$id] = new Reference($target, ContainerBuilder::IGNORE_ON_UNINITIALIZED_REFERENCE);
4848
}
4949
}

‎src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ protected function processValue($value, $isRoot = false)
7474
throw $e;
7575
}
7676

77-
$this->container->getDefinition($this->currentId)->addError($e->getMessage());
77+
$this->container->getDefinition($this->currentId)->addError($e->getMessageCallback() ?? $e->getMessage());
7878

7979
return parent::processValue($value, $isRoot);
8080
}
@@ -86,16 +86,21 @@ private function doProcessValue($value, $isRoot = false)
8686
if ($ref = $this->getAutowiredReference($value)) {
8787
return $ref;
8888
}
89-
$message = $this->createTypeNotFoundMessage($value, 'it');
90-
9189
if (ContainerBuilder::RUNTIME_EXCEPTION_ON_INVALID_REFERENCE === $value->getInvalidBehavior()) {
90+
$container = new ContainerBuilder($this->container->getParameterBag());
91+
$container->setAliases($this->container->getAliases());
92+
$container->setDefinitions($this->container->getDefinitions());
93+
$container->setResourceTracking(false);
94+
$message = function () use ($container, $value) {
95+
return $this->createTypeNotFoundMessage($container, $value, 'it');
96+
};
97+
9298
// since the error message varies by referenced id and $this->currentId, so should the id of the dummy errored definition
9399
$this->container->register($id = sprintf('.errored.%s.%s', $this->currentId, (string) $value), $value->getType())
94100
->addError($message);
95101

96102
return new TypedReference($id, $value->getType(), $value->getInvalidBehavior(), $value->getName());
97103
}
98-
$this->container->log($this, $message);
99104
}
100105
$value = parent::processValue($value, $isRoot);
101106

@@ -222,14 +227,19 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a
222227

 10000
223228
$getValue = function () use ($type, $parameter, $class, $method) {
224229
if (!$value = $this->getAutowiredReference($ref = new TypedReference($type, $type, ContainerBuilder::EXCEPTION_ON_INVALID_REFERENCE, $parameter->name))) {
225-
$failureMessage = $this->createTypeNotFoundMessage($ref, sprintf('argument "$%s" of method "%s()"', $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
230+
$container = new ContainerBuilder($this->container->getParameterBag());
231+
$container->setAliases($this->container->getAliases());
232+
$container->setDefinitions($this->container->getDefinitions());
233+
$container->setResourceTracking(false);
234+
$failureMessage = function () use ($container, $ref, $parameter, $class, $method) {
235+
return $this->createTypeNotFoundMessage($container, $ref, sprintf('argument "$%s" of method "%s()"', $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
236+
};
226237

227238
if ($parameter->isDefaultValueAvailable()) {
228239
$value = $parameter->getDefaultValue();
229240
} elseif (!$parameter->allowsNull()) {
230241
throw new AutowiringFailedException($this->currentId, $failureMessage);
231242
}
232-
$this->container->log($this, $failureMessage);
233243
}
234244

235245
return $value;
@@ -307,27 +317,27 @@ private function getAutowiredReference(TypedReference $reference)
307317
/**
308318
* Populates the list of available types.
309319
*/
310-
private function populateAvailableTypes()
320+
private function populateAvailableTypes(ContainerBuilder $container)
311321
{
312322
$this->types = array();
313323
$this->ambiguousServiceTypes = array();
314324

315-
foreach ($this->container->getDefinitions() as $id => $definition) {
316-
$this->populateAvailableType($id, $definition);
325+
foreach ($container->getDefinitions() as $id => $definition) {
326+
$this->populateAvailableType($container, $id, $definition);
317327
}
318328
}
319329

320330
/**
321331
* Populates the list of available types for a given definition.
322332
*/
323-
private function populateAvailableType(string $id, Definition $definition)
333+
private function populateAvailableType(ContainerBuilder $container, string $id, Definition $definition)
324334
{
325335
// Never use abstract services
326336
if ($definition->isAbstract()) {
327337
return;
328338
}
329339

330-
if ('' === $id || '.' === $id[0] || $definition->isDeprecated() || !$reflectionClass = $this->container->getReflectionClass($definition->getClass(), false)) {
340+
if ('' === $id || '.' === $id[0] || $definition->isDeprecated() || !$reflectionClass = $container->getReflectionClass($definition->getClass(), false)) {
331341
return;
332342
}
333343

@@ -367,19 +377,9 @@ private function set(string $type, string $id)
367377
$this->ambiguousServiceTypes[$type][] = $id;
368378
}
369379

370-
private function createTypeNotFoundMessage(TypedReference $reference, $label)
380+
private function createTypeNotFoundMessage(ContainerBuilder $container, TypedReference $reference, $label)
371381
{
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) {
382+
if (!$r = $container->getReflectionClass($type = $reference->getType(), false)) {
383383
// either $type does not exist or a parent class does not exist
384384
try {
385385
$resource = new ClassExistenceResource($type, false);
@@ -392,7 +392,8 @@ private function createTypeNotFoundMessage(TypedReference $reference, $label)
392392

393393
$message = sprintf('has type "%s" but this class %s.', $type, $parentMsg ? sprintf('is missing a parent class (%s)', $parentMsg) : 'was not found');
394394
} else {
395-
$message = $this->container->has($type) ? 'this service is abstract' : 'no such service exists';
395+
$alternatives = $this->createTypeAlternatives($container, $reference);
396+
$message = $container->has($type) ? 'this service is abstract' : 'no such service exists';
396397
$message = sprintf('references %s "%s" but %s.%s', $r->isInterface() ? 'interface' : 'class', $type, $message, $alternatives);
397398

398399
if ($r->isInterface() && !$alternatives) {
@@ -410,18 +411,18 @@ private function createTypeNotFoundMessage(TypedReference $reference, $label)
410411
return $message;
411412
}
412413

413-
private function createTypeAlternatives(TypedReference $reference)
414+
private function createTypeAlternatives(ContainerBuilder $container, TypedReference $reference)
414415
{
415416
// try suggesting available aliases first
416-
if ($message = $this->getAliasesSuggestionForType($type = $reference->getType())) {
417+
if ($message = $this->getAliasesSuggestionForType($container, $type = $reference->getType())) {
417418
return ' '.$message;
418419
}
419420
if (null === $this->ambiguousServiceTypes) {
420-
$this->populateAvailableTypes();
421+
$this->populateAvailableTypes($container);
421422
}
422423

423-
$servicesAndAliases = $this->container->getServiceIds();
424-
if (!$this->container->has($type) && false !== $key = array_search(strtolower($type), array_map('strtolower', $servicesAndAliases))) {
424+
$servicesAndAliases = $container->getServiceIds();
425+
if (!$container->has($type) && false !== $key = array_search(strtolower($type), array_map('strtolower', $servicesAndAliases))) {
425426
return sprintf(' Did you mean "%s"?', $servicesAndAliases[$key]);
426427
} elseif (isset($this->ambiguousServiceTypes[$type])) {
427428
$message = sprintf('one of these existing services: "%s"', implode('", "', $this->ambiguousServiceTypes[$type]));
@@ -434,11 +435,11 @@ private function createTypeAlternatives(TypedReference $reference)
434435
return sprintf(' You should maybe alias this %s to %s.', class_exists($type, false) ? 'class' : 'interface', $message);
435436
}
436437

437-
private function getAliasesSuggestionForType($type, $extraContext = null)
438+
private function getAliasesSuggestionForType(ContainerBuilder $container, $type, $extraContext = null)
438439
{
439440
$aliases = array();
440441
foreach (class_parents($type) + class_implements($type) as $parent) {
441-
if ($this->container->has($parent) && !$this->container->findDefinition($parent)->isAbstract()) {
442+
if ($container->has($parent) && !$container->findDefinition($parent)->isAbstract()) {
442443
$aliases[] = $parent;
443444
}
444445
}

‎src/Symfony/Component/DependencyInjection/Compiler/DefinitionErrorExceptionPass.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class DefinitionErrorExceptionPass extends AbstractRecursivePass
2828
*/
2929
protected function processValue($value, $isRoot = false)
3030
{
31-
if (!$value instanceof Definition || empty($value->getErrors())) {
31+
if (!$value instanceof Definition || !$value->hasErrors()) {
3232
return parent::processValue($value, $isRoot);
3333
}
3434

‎src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ protected function processValue($value, $isRoot = false)
157157
*/
158158
private function isInlineableDefinition($id, Definition $definition)
159159
{
160-
if ($definition->getErrors() || $definition->isDeprecated() || $definition->isLazy() || $definition->isSynthetic()) {
160+
if ($definition->hasErrors() || $definition->isDeprecated() || $definition->isLazy() || $definition->isSynthetic()) {
161161
return false;
162162
}
163163

‎src/Symfony/Component/DependencyInjection/Compiler/ResolveChildDefinitionsPass.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,8 @@ private function doResolveDefinition(ChildDefinition $definition)
176176
$def->setMethodCalls(array_merge($def->getMethodCalls(), $calls));
177177
}
178178

179-
foreach (array_merge($parentDef->getErrors(), $definition->getErrors()) as $v) {
180-
$def->addError($v);
181-
}
179+
$def->addError($parentDef);
180+
$def->addError($definition);
182181

183182
// these attributes are always taken from the child
184183
$def->setAbstract($definition->isAbstract());

‎src/Symfony/Component/DependencyInjection/ContainerBuilder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,7 @@ private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_
592592
throw $e;
593593
}
594594

595-
if ($e = $definition->getErrors()) {
595+
if ($definition->hasErrors() && $e = $definition->getErrors()) {
596596
throw new RuntimeException(reset($e));
597597
}
598598

‎src/Symfony/Component/DependencyInjection/Definition.php

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -876,13 +876,17 @@ public function setBindings(array $bindings)
876876
/**
877877
* Add an error that occurred when building this Definition.
878878
*
879-
* @param string $error
879+
* @param string|\Closure|self $error
880880
*
881881
* @return $this
882882
*/
883883
public function addError($error)
884884
{
885-
$this->errors[] = $error;
885+
if ($error instanceof self) {
886+
$this->errors = array_merge($this->errors, $error->errors);
887+
} else {
888+
$this->errors[] = $error;
889+
}
886890

887891
return $this;
888892
}
@@ -894,6 +898,17 @@ public function addError($error)
894898
*/
895899
public function getErrors()
896900
{
897-
return $this->errors;
901+
$errors = array();
902+
903+
foreach ($this->errors as $error) {
904+
$errors[] = (string) ($error instanceof \Closure ? $error() : $error);
905+
}
906+
907+
return $errors;
908+
}
909+
910+
public function hasErrors(): bool
911+
{
912+
return (bool) $this->errors;
898913
}
899914
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ private function addServiceInstance(string $id, Definition $definition, bool $is
492492

493493
private function isTrivialInstance(Definition $definition): bool
494494
{
495-
if ($definition->getErrors()) {
495+
if ($definition->hasErrors()) {
496496
return true;
497497
}
498498
if ($definition->isSynthetic() || $definition->getFile() || $definition->getMethodCalls() || $definition->getProperties() || $definition->getConfigurator()) {
@@ -1465,7 +1465,7 @@ private function dumpValue($value, bool $interpolate = true): string
14651465
continue;
14661466
}
14671467
$definition = $this->container->findDefinition($id = (string) $v);
1468-
$load = !($e = $definition->getErrors()) ? $this->asFiles && !$this->isHotPath($definition) : reset($e);
1468+
$load = !($definition->hasErrors() && $e = $definition->getErrors()) ? $this->asFiles && !$this->isHotPath($definition) : reset($e);
14691469
$serviceMap .= sprintf("\n %s => array(%s, %s, %s, %s),",
14701470
$this->export($k),
14711471
$this->export($definition->isShared() ? ($definition->isPublic() ? 'services' : 'privates') : false),
@@ -1483,7 +1483,7 @@ private function dumpValue($value, bool $interpolate = true): string
14831483
list($this->definitionVariables, $this->referenceVariables) = $scope;
14841484
}
14851485
} elseif ($value instanceof Definition) {
1486-
if ($e = $value->getErrors()) {
1486+
if ($value->hasErrors() && $e = $value->getErrors()) {
14871487
$this->addThrow = true;
14881488

14891489
return sprintf('$this->throw(%s)', $this->export(reset($e)));
@@ -1592,7 +1592,7 @@ private function getServiceCall(string $id, Reference $reference = null): string
15921592
return $code;
15931593
}
15941594
} elseif ($this->isTrivialInstance($definition)) {
1595-
if ($e = $definition->getErrors()) {
1595+
if ($definition->hasErrors() && $e = $definition->getErrors()) {
15961596
$this->addThrow = true;
15971597

15981598
return sprintf('$this->throw(%s)', $this->export(reset($e)));

‎src/Symfony/Component/DependencyInjection/Exception/AutowiringFailedException.php

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,44 @@
1717
class AutowiringFailedException extends RuntimeException
1818
{
1919
private $serviceId;
20+
private $messageCallback;
2021

21-
public function __construct(string $serviceId, string $message = '', int $code = 0, \Exception $previous = null)
22+
public function __construct(string $serviceId, $message = '', int $code = 0, \Exception $previous = null)
2223
{
2324
$this->serviceId = $serviceId;
2425

25-
parent::__construct($message, $code, $previous);
26+
if (!$message instanceof \Closure) {
27+
parent::__construct($message, $code, $previous);
28+
29+
return;
30+
}
31+
32+
$this->messageCallback = $message;
33+
parent::__construct('', $code, $previous);
34+
35+
$this->message = new class($this->message, $this->messageCallback) {
36+
private $message;
37+
private $messageCallback;
38+
39+
public function __construct(&$message, &$messageCallback)
40+
{
41+
$this->message = &$message;
42+
$this->messageCallback = &$messageCallback;
43+
}
44+
45+
public function __toString(): string
46+
{
47+
$messageCallback = $this->messageCallback;
48+
$this->messageCallback = null;
49+
50+
return $this->message = $messageCallback();
51+
}
52+
};
53+
}
54+
55+
public function getMessageCallback(): ?\Closure
56+
{
57+
return $this->messageCallback;
2658
}
2759

2860
public function getServiceId()

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
use Symfony\Component\DependencyInjection\Compiler\DecoratorServicePass;
2121
use Symfony\Component\DependencyInjection\Compiler\ResolveClassPass;
2222
use Symfony\Component\DependencyInjection\ContainerBuilder;
23-
use Symfony\Component\DependencyInjection\Definition;
2423
use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
2524
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
2625
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
@@ -620,7 +619,7 @@ public function testSetterInjectionCollisionThrowsException()
620619
}
621620

622621
$this->assertNotNull($e);
623-
$this->assertSame('Cannot autowire service "setter_injection_collision": argument "$collision" of method "Symfony\Component\DependencyInjection\Tests\Compiler\SetterInjectionCollision::setMultipleInstancesForOneArg()" references interface "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" but no such service exists. You should maybe alias this interface to one of these existing services: "c1", "c2".', $e->getMessage());
622+
$this->assertSame('Cannot autowire service "setter_injection_collision": argument "$collision" of method "Symfony\Component\DependencyInjection\Tests\Compiler\SetterInjectionCollision::setMultipleInstancesForOneArg()" references interface "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" but no such service exists. You should maybe alias this interface to one of these existing services: "c1", "c2".', (string) $e->getMessage());
624623
}
625624

626625
/**
@@ -903,9 +902,7 @@ public function testErroredServiceLocator()
903902

904903
(new AutowirePass())->process($container);
905904

906-
$erroredDefinition = new Definition(MissingClass::class);
907-
908-
$this->assertEquals($erroredDefinition->addError('Cannot autowire service "some_locator": it has type "Symfony\Component\DependencyInjection\Tests\Compiler\MissingClass" but this class was not found.'), $container->getDefinition('.errored.some_locator.'.MissingClass::class));
905+
$this->assertSame(array('Cannot autowire service "some_locator": it has type "Symfony\Component\DependencyInjection\Tests\Compiler\MissingClass" but this class was not found.'), $container->getDefinition('.errored.some_locator.'.MissingClass::class)->getErrors());
909906
}
910907

911908
public function testNamedArgumentAliasResolveCollisions()

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ public function testShouldAutoconfigure()
375375
public function testAddError()
376376
{
377377
$def = new Definition('stdClass');
378-
$this->assertEmpty($def->getErrors());
378+
$this->assertFalse($def->hasErrors());
379379
$def->addError('First error');
380380
$def->addError('Second error');
381381
$this->assertSame(array('First error', 'Second error'), $def->getErrors());

0 commit comments

Comments
 (0)
0