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

Commit d4f407d

Browse files
nicolas-grekasnicolas-grekas
committed
[DI] compute autowiring error messages lazily
1 parent 5e0d3f0 commit d4f407d

File tree

11 files changed

+98
-45
lines changed

11 files changed

+98
-45
lines changed

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

Lines changed: 2 additions & 2 deletions
< B422 /colgroup>
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: 30 additions & 22 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,20 @@ 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+
$message = function () use ($container, $value) {
94+
return $this->createTypeNotFoundMessage($container, $value, 'it');
95+
};
96+
9297
// since the error message varies by referenced id and $this->currentId, so should the id of the dummy errored definition
9398
$this->container->register($id = sprintf('.errored.%s.%s', $this->currentId, (string) $value), $value->getType())
9499
->addError($message);
95100

96101
return new TypedReference($id, $value->getType(), $value->getInvalidBehavior(), $value->getName());
97102
}
98-
$this->container->log($this, $message);
99103
}
100104
$value = parent::processValue($value, $isRoot);
101105

@@ -222,14 +226,18 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a
222226

223227
$getValue = function () use ($type, $parameter, $class, $method) {
224228
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));
229+
$container = new ContainerBuilder($this->container->getParameterBag());
230+
$container->setAliases($this->container->getAliases());
231+
$container->setDefinitions($this->container->getDefinitions());
232+
$failureMessage = function () use ($container, $ref, $parameter, $class, $method) {
233+
return $this->createTypeNotFoundMessage($container, $ref, sprintf('argument "$%s" of method "%s()"', $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
234+
};
226235

227236
if ($parameter->isDefaultValueAvailable()) {
228237
$value = $parameter->getDefaultValue();
229238
} elseif (!$parameter->allowsNull()) {
230239
throw new AutowiringFailedException($this->currentId, $failureMessage);
231240
}
232-
$this->container->log($this, $failureMessage);
233241
}
234242

235243
return $value;
@@ -307,27 +315,27 @@ private function getAutowiredReference(TypedReference $reference)
307315
/**
308316
* Populates the list of available types.
309317
*/
310-
private function populateAvailableTypes()
318+
private function populateAvailableTypes(ContainerBuilder $container)
311319
{
312320
$this->types = array();
313321
$this->ambiguousServiceTypes = array();
314322

315-
foreach ($this->container->getDefinitions() as $id => $definition) {
316-
$this->populateAvailableType($id, $definition);
323+
foreach ($container->getDefinitions() as $id => $definition) {
324+
$this->populateAvailableType($container, $id, $definition);
317325
}
318326
}
319327

320328
/**
321329
* Populates the list of available types for a given definition.
322330
*/
323-
private function populateAvailableType(string $id, Definition $definition)
331+
private function populateAvailableType(ContainerBuilder $container, string $id, Definition $definition)
324332
{
325333
// Never use abstract services
326334
if ($definition->isAbstract()) {
327335
return;
328336
}
329337

330-
if ('' === $id || '.' === $id[0] || $definition->isDeprecated() || !$reflectionClass = $this->container->getReflectionClass($definition->getClass(), false)) {
338+
if ('' === $id || '.' === $id[0] || $definition->isDeprecated() || !$reflectionClass = $container->getReflectionClass($definition->getClass(), false)) {
331339
return;
332340
}
333341

@@ -367,9 +375,9 @@ private function set(string $type, string $id)
367375
$this->ambiguousServiceTypes[$type][] = $id;
368376
}
369377

370-
private function createTypeNotFoundMessage(TypedReference $reference, $label)
378+
private function createTypeNotFoundMessage(ContainerBuilder $container, TypedReference $reference, $label)
371379
{
372-
if (!$r = $this->container->getReflectionClass($type = $reference->getType(), false)) {
380+
if (!$r = $container->getReflectionClass($type = $reference->getType(), false)) {
373381
// either $type does not exist or a parent class does not exist
374382
try {
375383
$resource = new ClassExistenceResource($type, false);
@@ -382,8 +390,8 @@ private function createTypeNotFoundMessage(TypedReference $reference, $label)
382390

383391
$message = sprintf('has type "%s" but this class %s.', $type, $parentMsg ? sprintf('is missing a parent class (%s)', $parentMsg) : 'was not found');
384392
} else {
385-
$alternatives = $this->createTypeAlternatives($reference);
386-
$message = $this->container->has($type) ? 'this service is abstract' : 'no such service exists';
393+
$alternatives = $this->createTypeAlternatives($container, $reference);
394+
$message = $container->has($type) ? 'this service is abstract' : 'no such service exists';
387395
$message = sprintf('references %s "%s" but %s.%s', $r->isInterface() ? 'interface' : 'class', $type, $message, $alternatives);
388396

389397
if F438 ($r->isInterface() && !$alternatives) {
@@ -401,18 +409,18 @@ private function createTypeNotFoundMessage(TypedReference $reference, $label)
401409
return $message;
402410
}
403411

404-
private function createTypeAlternatives(TypedReference $reference)
412+
private function createTypeAlternatives(ContainerBuilder $container, TypedReference $reference)
405413
{
406414
// try suggesting available aliases first
407-
if ($message = $this->getAliasesSuggestionForType($type = $reference->getType())) {
415+
if ($message = $this->getAliasesSuggestionForType($container, $type = $reference->getType())) {
408416
return ' '.$message;
409417
}
410418
if (null === $this->ambiguousServiceTypes) {
411-
$this->populateAvailableTypes();
419+
$this->populateAvailableTypes($container);
412420
}
413421

414-
$servicesAndAliases = $this->container->getServiceIds();
415-
if (!$this->container->has($type) && false !== $key = array_search(strtolower($type), array_map('strtolower', $servicesAndAliases))) {
422+
$servicesAndAliases = $container->getServiceIds();
423+
if (!$container->has($type) && false !== $key = array_search(strtolower($type), array_map('strtolower', $servicesAndAliases))) {
416424
return sprintf(' Did you mean "%s"?', $servicesAndAliases[$key]);
417425
} elseif (isset($this->ambiguousServiceTypes[$type])) {
418426
$message = sprintf('one of these existing services: "%s"', implode('", "', $this->ambiguousServiceTypes[$type]));
@@ -425,11 +433,11 @@ private function createTypeAlternatives(TypedReference $reference)
425433
return sprintf(' You should maybe alias this %s to %s.', class_exists($type, false) ? 'class' : 'interface', $message);
426434
}
427435

428-
private function getAliasesSuggestionForType($type, $extraContext = null)
436+
private function getAliasesSuggestionForType(ContainerBuilder $container, $type, $extraContext = null)
429437
{
430438
$aliases = array();
431439
foreach (class_parents($type) + class_implements($type) as $parent) {
432-
if ($this->container->has($parent) && !$this->container->findDefinition($parent)->isAbstract()) {
440+
if ($container->has($parent) && !$container->findDefinition($parent)->isAbstract()) {
433441
$aliases[] = $parent;
434442
}
435443
}

‎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: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -876,13 +876,19 @@ 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+
foreach ($error->errors as $error) {
887+
$this->errors[] = $error;
888+
}
889+
} else {
890+
$this->errors[] = $error;
891+
}
886892

887893
return $this;
888894
}
@@ -894,6 +900,17 @@ public function addError($error)
894900
*/
895901
public function getErrors()
896902
{
897-
return $this->errors;
903+
$errors = array();
904+
905+
foreach ($this->errors as $error) {
906+
$errors[] = (string) ($error instanceof \Closure ? $error() : $error);
907+
}
908+
909+
return $errors;
910+
}
911+
912+
public function hasErrors(): bool
913+
{
914+
return (bool) $this->errors;
898915
}
899916
}

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

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

489489
private function isTrivialInstance(Definition $definition): bool
490490
{
491-
if ($definition->getErrors()) {
491+
if ($definition->hasErrors()) {
492492
return true;
493493
}
494494
if ($definition->isSynthetic() || $definition->getFile() || $definition->getMethodCalls() || $definition->getProperties() || $definition->getConfigurator()) {
@@ -1460,7 +1460,7 @@ private function dumpValue($value, bool $interpolate = true): string
14601460
continue;
14611461
}
14621462
$definition = $this->container->findDefinition($id = (string) $v);
1463-
$load = !($e = $definition->getErrors()) ? $this->asFiles && !$this->isHotPath($definition) : reset($e);
1463+
$load = !($definition->hasErrors() && $e = $definition->getErrors()) ? $this->asFiles && !$this->isHotPath($definition) : reset($e);
14641464
$serviceMap .= sprintf("\n %s => array(%s, %s, %s, %s),",
14651465
$this->export($k),
14661466
$this->export($definition->isShared() ? ($definition->isPublic() ? 'services' : 'privates') : false),
@@ -1478,7 +1478,7 @@ private function dumpValue($value, bool $interpolate = true): string
14781478
list($this->definitionVariables, $this->referenceVariables) = $scope;
14791479
}
14801480
} elseif ($value instanceof Definition) {
1481-
if ($e = $value->getErrors()) {
1481+
if ($value->hasErrors() && $e = $value->getErrors()) {
14821482
$this->addThrow = true;
14831483

14841484
return sprintf('$this->throw(%s)', $this->export(reset($e)));
@@ -1587,7 +1587,7 @@ private function getServiceCall(string $id, Reference $reference = null): string
15871587
return $code;
15881588
}
15891589
} elseif ($this->isTrivialInstance($definition)) {
1590-
if ($e = $definition->getErrors()) {
1590+
if ($definition->hasErrors() && $e = $definition->getErrors()) {
15911591
$this->addThrow = true;
15921592

15931593
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