Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit b90172a

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
[DI] compute autowiring error messages lazily
1 parent 5e0d3f0 commit b90172a
Copy full SHA for b90172a

File tree

11 files changed

+96
-45
lines changed
Filter options

11 files changed

+96
-45
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TestServiceContainerWeakRefPass.php
+2-2Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
+30-22Lines 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 ($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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/DefinitionErrorExceptionPass.php
+1-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php
+1-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/ResolveChildDefinitionsPass.php
+2-3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/ContainerBuilder.php
+1-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Definition.php
+20-3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
+4-4Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Exception/AutowiringFailedException.php
+32-2Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,42 @@
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+
$this->messageCallback = $message;
28+
parent::__construct('', $code, $previous);
29+
30+
$this->message = new class($this->message, $this->messageCallback) {
31+
public $message;
32+
public $messageCallback;
33+
34+
public function __construct(&$message, &$messageCallback)
35+
{
36+
$this->message = &$message;
37+
$this->messageCallback = &$messageCallback;
38+
}
39+
40+
public function __toString()
41+
{
42+
$messageCallback = $this->messageCallback;
43+
$this->messageCallback = null;
44+
45+
return $this->message = $messageCallback();
46+
}
47+
};
48+
} else {
49+
parent::__construct($message, $code, $previous);
50+
}
51+
}
52+
53+
public function getMessageCallback(): ?\Closure
54+
{
55+
return $this->messageCallback;
2656
}
2757

2858
public function getServiceId()

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
+2-5Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php
+1-1Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.