[DI] compute autowiring error messages lazily
This commit is contained in:
parent
8d277ce3e5
commit
3b3a1bd3cc
@ -31,7 +31,7 @@ class TestServiceContainerWeakRefPass implements CompilerPassInterface
|
||||
$definitions = $container->getDefinitions();
|
||||
|
||||
foreach ($definitions as $id => $definition) {
|
||||
if ($id && '.' !== $id[0] && (!$definition->isPublic() || $definition->isPrivate()) && !$definition->getErrors() && !$definition->isAbstract()) {
|
||||
if ($id && '.' !== $id[0] && (!$definition->isPublic() || $definition->isPrivate()) && !$definition->hasErrors() && !$definition->isAbstract()) {
|
||||
$privateServices[$id] = new Reference($id, ContainerBuilder::IGNORE_ON_UNINITIALIZED_REFERENCE);
|
||||
}
|
||||
}
|
||||
@ -43,7 +43,7 @@ class TestServiceContainerWeakRefPass implements CompilerPassInterface
|
||||
while (isset($aliases[$target = (string) $alias])) {
|
||||
$alias = $aliases[$target];
|
||||
}
|
||||
if (isset($definitions[$target]) && !$definitions[$target]->getErrors() && !$definitions[$target]->isAbstract()) {
|
||||
if (isset($definitions[$target]) && !$definitions[$target]->hasErrors() && !$definitions[$target]->isAbstract()) {
|
||||
$privateServices[$id] = new Reference($target, ContainerBuilder::IGNORE_ON_UNINITIALIZED_REFERENCE);
|
||||
}
|
||||
}
|
||||
|
@ -74,7 +74,7 @@ class AutowirePass extends AbstractRecursivePass
|
||||
throw $e;
|
||||
}
|
||||
|
||||
$this->container->getDefinition($this->currentId)->addError($e->getMessage());
|
||||
$this->container->getDefinition($this->currentId)->addError($e->getMessageCallback() ?? $e->getMessage());
|
||||
|
||||
return parent::processValue($value, $isRoot);
|
||||
}
|
||||
@ -86,16 +86,15 @@ class AutowirePass extends AbstractRecursivePass
|
||||
if ($ref = $this->getAutowiredReference($value)) {
|
||||
return $ref;
|
||||
}
|
||||
$message = $this->createTypeNotFoundMessage($value, 'it');
|
||||
|
||||
if (ContainerBuilder::RUNTIME_EXCEPTION_ON_INVALID_REFERENCE === $value->getInvalidBehavior()) {
|
||||
$message = $this->createTypeNotFoundMessageCallback($value, 'it');
|
||||
|
||||
// since the error message varies by referenced id and $this->currentId, so should the id of the dummy errored definition
|
||||
$this->container->register($id = sprintf('.errored.%s.%s', $this->currentId, (string) $value), $value->getType())
|
||||
->addError($message);
|
||||
|
||||
return new TypedReference($id, $value->getType(), $value->getInvalidBehavior(), $value->getName());
|
||||
}
|
||||
$this->container->log($this, $message);
|
||||
}
|
||||
$value = parent::processValue($value, $isRoot);
|
||||
|
||||
@ -222,14 +221,13 @@ class AutowirePass extends AbstractRecursivePass
|
||||
|
||||
$getValue = function () use ($type, $parameter, $class, $method) {
|
||||
if (!$value = $this->getAutowiredReference($ref = new TypedReference($type, $type, ContainerBuilder::EXCEPTION_ON_INVALID_REFERENCE, $parameter->name))) {
|
||||
$failureMessage = $this->createTypeNotFoundMessage($ref, sprintf('argument "$%s" of method "%s()"', $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
|
||||
$failureMessage = $this->createTypeNotFoundMessageCallback($ref, sprintf('argument "$%s" of method "%s()"', $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
|
||||
|
||||
if ($parameter->isDefaultValueAvailable()) {
|
||||
$value = $parameter->getDefaultValue();
|
||||
} elseif (!$parameter->allowsNull()) {
|
||||
throw new AutowiringFailedException($this->currentId, $failureMessage);
|
||||
}
|
||||
$this->container->log($this, $failureMessage);
|
||||
}
|
||||
|
||||
return $value;
|
||||
@ -307,27 +305,27 @@ class AutowirePass extends AbstractRecursivePass
|
||||
/**
|
||||
* Populates the list of available types.
|
||||
*/
|
||||
private function populateAvailableTypes()
|
||||
private function populateAvailableTypes(ContainerBuilder $container)
|
||||
{
|
||||
$this->types = array();
|
||||
$this->ambiguousServiceTypes = array();
|
||||
|
||||
foreach ($this->container->getDefinitions() as $id => $definition) {
|
||||
$this->populateAvailableType($id, $definition);
|
||||
foreach ($container->getDefinitions() as $id => $definition) {
|
||||
$this->populateAvailableType($container, $id, $definition);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Populates the list of available types for a given definition.
|
||||
*/
|
||||
private function populateAvailableType(string $id, Definition $definition)
|
||||
private function populateAvailableType(ContainerBuilder $container, string $id, Definition $definition)
|
||||
{
|
||||
// Never use abstract services
|
||||
if ($definition->isAbstract()) {
|
||||
return;
|
||||
}
|
||||
|
||||
if ('' === $id || '.' === $id[0] || $definition->isDeprecated() || !$reflectionClass = $this->container->getReflectionClass($definition->getClass(), false)) {
|
||||
if ('' === $id || '.' === $id[0] || $definition->isDeprecated() || !$reflectionClass = $container->getReflectionClass($definition->getClass(), false)) {
|
||||
return;
|
||||
}
|
||||
|
||||
@ -367,19 +365,21 @@ class AutowirePass extends AbstractRecursivePass
|
||||
$this->ambiguousServiceTypes[$type][] = $id;
|
||||
}
|
||||
|
||||
private function createTypeNotFoundMessage(TypedReference $reference, $label)
|
||||
private function createTypeNotFoundMessageCallback(TypedReference $reference, $label)
|
||||
{
|
||||
$trackResources = $this->container->isTrackingResources();
|
||||
$this->container->setResourceTracking(false);
|
||||
try {
|
||||
if ($r = $this->container->getReflectionClass($type = $reference->getType(), false)) {
|
||||
$alternatives = $this->createTypeAlternatives($reference);
|
||||
}
|
||||
} finally {
|
||||
$this->container->setResourceTracking($trackResources);
|
||||
$container = new ContainerBuilder($this->container->getParameterBag());
|
||||
$container->setAliases($this->container->getAliases());
|
||||
$container->setDefinitions($this->container->getDefinitions());
|
||||
$container->setResourceTracking(false);
|
||||
|
||||
return function () use ($container, $reference, $label) {
|
||||
return $this->createTypeNotFoundMessage($container, $reference, $label);
|
||||
};
|
||||
}
|
||||
|
||||
if (!$r) {
|
||||
private function createTypeNotFoundMessage(ContainerBuilder $container, TypedReference $reference, $label)
|
||||
{
|
||||
if (!$r = $container->getReflectionClass($type = $reference->getType(), false)) {
|
||||
// either $type does not exist or a parent class does not exist
|
||||
try {
|
||||
$resource = new ClassExistenceResource($type, false);
|
||||
@ -392,7 +392,8 @@ class AutowirePass extends AbstractRecursivePass
|
||||
|
||||
$message = sprintf('has type "%s" but this class %s.', $type, $parentMsg ? sprintf('is missing a parent class (%s)', $parentMsg) : 'was not found');
|
||||
} else {
|
||||
$message = $this->container->has($type) ? 'this service is abstract' : 'no such service exists';
|
||||
$alternatives = $this->createTypeAlternatives($container, $reference);
|
||||
$message = $container->has($type) ? 'this service is abstract' : 'no such service exists';
|
||||
$message = sprintf('references %s "%s" but %s.%s', $r->isInterface() ? 'interface' : 'class', $type, $message, $alternatives);
|
||||
|
||||
if ($r->isInterface() && !$alternatives) {
|
||||
@ -410,18 +411,18 @@ class AutowirePass extends AbstractRecursivePass
|
||||
return $message;
|
||||
}
|
||||
|
||||
private function createTypeAlternatives(TypedReference $reference)
|
||||
private function createTypeAlternatives(ContainerBuilder $container, TypedReference $reference)
|
||||
{
|
||||
// try suggesting available aliases first
|
||||
if ($message = $this->getAliasesSuggestionForType($type = $reference->getType())) {
|
||||
if ($message = $this->getAliasesSuggestionForType($container, $type = $reference->getType())) {
|
||||
return ' '.$message;
|
||||
}
|
||||
if (null === $this->ambiguousServiceTypes) {
|
||||
$this->populateAvailableTypes();
|
||||
$this->populateAvailableTypes($container);
|
||||
}
|
||||
|
||||
$servicesAndAliases = $this->container->getServiceIds();
|
||||
if (!$this->container->has($type) && false !== $key = array_search(strtolower($type), array_map('strtolower', $servicesAndAliases))) {
|
||||
$servicesAndAliases = $container->getServiceIds();
|
||||
if (!$container->has($type) && false !== $key = array_search(strtolower($type), array_map('strtolower', $servicesAndAliases))) {
|
||||
return sprintf(' Did you mean "%s"?', $servicesAndAliases[$key]);
|
||||
} elseif (isset($this->ambiguousServiceTypes[$type])) {
|
||||
$message = sprintf('one of these existing services: "%s"', implode('", "', $this->ambiguousServiceTypes[$type]));
|
||||
@ -434,11 +435,11 @@ class AutowirePass extends AbstractRecursivePass
|
||||
return sprintf(' You should maybe alias this %s to %s.', class_exists($type, false) ? 'class' : 'interface', $message);
|
||||
}
|
||||
|
||||
private function getAliasesSuggestionForType($type, $extraContext = null)
|
||||
private function getAliasesSuggestionForType(ContainerBuilder $container, $type, $extraContext = null)
|
||||
{
|
||||
$aliases = array();
|
||||
foreach (class_parents($type) + class_implements($type) as $parent) {
|
||||
if ($this->container->has($parent) && !$this->container->findDefinition($parent)->isAbstract()) {
|
||||
if ($container->has($parent) && !$container->findDefinition($parent)->isAbstract()) {
|
||||
$aliases[] = $parent;
|
||||
}
|
||||
}
|
||||
|
@ -28,7 +28,7 @@ class DefinitionErrorExceptionPass extends AbstractRecursivePass
|
||||
*/
|
||||
protected function processValue($value, $isRoot = false)
|
||||
{
|
||||
if (!$value instanceof Definition || empty($value->getErrors())) {
|
||||
if (!$value instanceof Definition || !$value->hasErrors()) {
|
||||
return parent::processValue($value, $isRoot);
|
||||
}
|
||||
|
||||
|
@ -157,7 +157,7 @@ class InlineServiceDefinitionsPass extends AbstractRecursivePass implements Repe
|
||||
*/
|
||||
private function isInlineableDefinition($id, Definition $definition)
|
||||
{
|
||||
if ($definition->getErrors() || $definition->isDeprecated() || $definition->isLazy() || $definition->isSynthetic()) {
|
||||
if ($definition->hasErrors() || $definition->isDeprecated() || $definition->isLazy() || $definition->isSynthetic()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
|
@ -176,9 +176,8 @@ class ResolveChildDefinitionsPass extends AbstractRecursivePass
|
||||
$def->setMethodCalls(array_merge($def->getMethodCalls(), $calls));
|
||||
}
|
||||
|
||||
foreach (array_merge($parentDef->getErrors(), $definition->getErrors()) as $v) {
|
||||
$def->addError($v);
|
||||
}
|
||||
$def->addError($parentDef);
|
||||
$def->addError($definition);
|
||||
|
||||
// these attributes are always taken from the child
|
||||
$def->setAbstract($definition->isAbstract());
|
||||
|
@ -592,7 +592,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
|
||||
throw $e;
|
||||
}
|
||||
|
||||
if ($e = $definition->getErrors()) {
|
||||
if ($definition->hasErrors() && $e = $definition->getErrors()) {
|
||||
throw new RuntimeException(reset($e));
|
||||
}
|
||||
|
||||
|
@ -876,13 +876,17 @@ class Definition
|
||||
/**
|
||||
* Add an error that occurred when building this Definition.
|
||||
*
|
||||
* @param string $error
|
||||
* @param string|\Closure|self $error
|
||||
*
|
||||
* @return $this
|
||||
*/
|
||||
public function addError($error)
|
||||
{
|
||||
if ($error instanceof self) {
|
||||
$this->errors = array_merge($this->errors, $error->errors);
|
||||
} else {
|
||||
$this->errors[] = $error;
|
||||
}
|
||||
|
||||
return $this;
|
||||
}
|
||||
@ -894,6 +898,19 @@ class Definition
|
||||
*/
|
||||
public function getErrors()
|
||||
{
|
||||
foreach ($this->errors as $i => $error) {
|
||||
if ($error instanceof \Closure) {
|
||||
$this->errors[$i] = (string) $error();
|
||||
} elseif (!\is_string($error)) {
|
||||
$this->errors[$i] = (string) $error;
|
||||
}
|
||||
}
|
||||
|
||||
return $this->errors;
|
||||
}
|
||||
|
||||
public function hasErrors(): bool
|
||||
{
|
||||
return (bool) $this->errors;
|
||||
}
|
||||
}
|
||||
|
@ -492,7 +492,7 @@ EOF;
|
||||
|
||||
private function isTrivialInstance(Definition $definition): bool
|
||||
{
|
||||
if ($definition->getErrors()) {
|
||||
if ($definition->hasErrors()) {
|
||||
return true;
|
||||
}
|
||||
if ($definition->isSynthetic() || $definition->getFile() || $definition->getMethodCalls() || $definition->getProperties() || $definition->getConfigurator()) {
|
||||
@ -1465,7 +1465,7 @@ EOF;
|
||||
continue;
|
||||
}
|
||||
$definition = $this->container->findDefinition($id = (string) $v);
|
||||
$load = !($e = $definition->getErrors()) ? $this->asFiles && !$this->isHotPath($definition) : reset($e);
|
||||
$load = !($definition->hasErrors() && $e = $definition->getErrors()) ? $this->asFiles && !$this->isHotPath($definition) : reset($e);
|
||||
$serviceMap .= sprintf("\n %s => array(%s, %s, %s, %s),",
|
||||
$this->export($k),
|
||||
$this->export($definition->isShared() ? ($definition->isPublic() ? 'services' : 'privates') : false),
|
||||
@ -1483,7 +1483,7 @@ EOF;
|
||||
list($this->definitionVariables, $this->referenceVariables) = $scope;
|
||||
}
|
||||
} elseif ($value instanceof Definition) {
|
||||
if ($e = $value->getErrors()) {
|
||||
if ($value->hasErrors() && $e = $value->getErrors()) {
|
||||
$this->addThrow = true;
|
||||
|
||||
return sprintf('$this->throw(%s)', $this->export(reset($e)));
|
||||
@ -1592,7 +1592,7 @@ EOF;
|
||||
return $code;
|
||||
}
|
||||
} elseif ($this->isTrivialInstance($definition)) {
|
||||
if ($e = $definition->getErrors()) {
|
||||
if ($definition->hasErrors() && $e = $definition->getErrors()) {
|
||||
$this->addThrow = true;
|
||||
|
||||
return sprintf('$this->throw(%s)', $this->export(reset($e)));
|
||||
|
@ -17,12 +17,44 @@ namespace Symfony\Component\DependencyInjection\Exception;
|
||||
class AutowiringFailedException extends RuntimeException
|
||||
{
|
||||
private $serviceId;
|
||||
private $messageCallback;
|
||||
|
||||
public function __construct(string $serviceId, string $message = '', int $code = 0, \Exception $previous = null)
|
||||
public function __construct(string $serviceId, $message = '', int $code = 0, \Exception $previous = null)
|
||||
{
|
||||
$this->serviceId = $serviceId;
|
||||
|
||||
if (!$message instanceof \Closure) {
|
||||
parent::__construct($message, $code, $previous);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
$this->messageCallback = $message;
|
||||
parent::__construct('', $code, $previous);
|
||||
|
||||
$this->message = new class($this->message, $this->messageCallback) {
|
||||
private $message;
|
||||
private $messageCallback;
|
||||
|
||||
public function __construct(&$message, &$messageCallback)
|
||||
{
|
||||
$this->message = &$message;
|
||||
$this->messageCallback = &$messageCallback;
|
||||
}
|
||||
|
||||
public function __toString(): string
|
||||
{
|
||||
$messageCallback = $this->messageCallback;
|
||||
$this->messageCallback = null;
|
||||
|
||||
return $this->message = $messageCallback();
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
public function getMessageCallback(): ?\Closure
|
||||
{
|
||||
return $this->messageCallback;
|
||||
}
|
||||
|
||||
public function getServiceId()
|
||||
|
@ -20,7 +20,6 @@ use Symfony\Component\DependencyInjection\Compiler\AutowireRequiredMethodsPass;
|
||||
use Symfony\Component\DependencyInjection\Compiler\DecoratorServicePass;
|
||||
use Symfony\Component\DependencyInjection\Compiler\ResolveClassPass;
|
||||
use Symfony\Component\DependencyInjection\ContainerBuilder;
|
||||
use Symfony\Component\DependencyInjection\Definition;
|
||||
use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
|
||||
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
|
||||
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
|
||||
@ -620,7 +619,7 @@ class AutowirePassTest extends TestCase
|
||||
}
|
||||
|
||||
$this->assertNotNull($e);
|
||||
$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());
|
||||
$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());
|
||||
}
|
||||
|
||||
/**
|
||||
@ -903,9 +902,7 @@ class AutowirePassTest extends TestCase
|
||||
|
||||
(new AutowirePass())->process($container);
|
||||
|
||||
$erroredDefinition = new Definition(MissingClass::class);
|
||||
|
||||
$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));
|
||||
$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());
|
||||
}
|
||||
|
||||
public function testNamedArgumentAliasResolveCollisions()
|
||||
|
@ -375,7 +375,7 @@ class DefinitionTest extends TestCase
|
||||
public function testAddError()
|
||||
{
|
||||
$def = new Definition('stdClass');
|
||||
$this->assertEmpty($def->getErrors());
|
||||
$this->assertFalse($def->hasErrors());
|
||||
$def->addError('First error');
|
||||
$def->addError('Second error');
|
||||
$this->assertSame(array('First error', 'Second error'), $def->getErrors());
|
||||
|
Reference in New Issue
Block a user