[DI] Fix bad error message for unused bind under _defaults
This commit is contained in:
parent
4447f87c2e
commit
35bf4203e8
@ -16,13 +16,19 @@ namespace Symfony\Component\DependencyInjection\Argument;
|
||||
*/
|
||||
final class BoundArgument implements ArgumentInterface
|
||||
{
|
||||
const SERVICE_BINDING = 0;
|
||||
const DEFAULTS_BINDING = 1;
|
||||
const INSTANCEOF_BINDING = 2;
|
||||
|
||||
private static $sequence = 0;
|
||||
|
||||
private $value;
|
||||
private $identifier;
|
||||
private $used;
|
||||
private $type;
|
||||
private $file;
|
||||
|
||||
public function __construct($value, bool $trackUsage = true)
|
||||
public function __construct($value, bool $trackUsage = true, int $type = 0, string $file = null)
|
||||
{
|
||||
$this->value = $value;
|
||||
if ($trackUsage) {
|
||||
@ -30,6 +36,8 @@ final class BoundArgument implements ArgumentInterface
|
||||
} else {
|
||||
$this->used = true;
|
||||
}
|
||||
$this->type = $type;
|
||||
$this->file = $file;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -37,7 +45,7 @@ final class BoundArgument implements ArgumentInterface
|
||||
*/
|
||||
public function getValues()
|
||||
{
|
||||
return [$this->value, $this->identifier, $this->used];
|
||||
return [$this->value, $this->identifier, $this->used, $this->type, $this->file];
|
||||
}
|
||||
|
||||
/**
|
||||
@ -45,6 +53,6 @@ final class BoundArgument implements ArgumentInterface
|
||||
*/
|
||||
public function setValues(array $values)
|
||||
{
|
||||
list($this->value, $this->identifier, $this->used) = $values;
|
||||
list($this->value, $this->identifier, $this->used, $this->type, $this->file) = $values;
|
||||
}
|
||||
}
|
||||
|
@ -37,8 +37,39 @@ class ResolveBindingsPass extends AbstractRecursivePass
|
||||
try {
|
||||
parent::process($container);
|
||||
|
||||
foreach ($this->unusedBindings as list($key, $serviceId)) {
|
||||
$message = sprintf('Unused binding "%s" in service "%s".', $key, $serviceId);
|
||||
foreach ($this->unusedBindings as list($key, $serviceId, $bindingType, $file)) {
|
||||
$argumentType = $argumentName = $message = null;
|
||||
|
||||
if (false !== strpos($key, ' ')) {
|
||||
list($argumentType, $argumentName) = explode(' ', $key, 2);
|
||||
} elseif ('$' === $key[0]) {
|
||||
$argumentName = $key;
|
||||
} else {
|
||||
$argumentType = $key;
|
||||
}
|
||||
|
||||
if ($argumentType) {
|
||||
$message .= sprintf('of type "%s" ', $argumentType);
|
||||
}
|
||||
|
||||
if ($argumentName) {
|
||||
$message .= sprintf('named "%s" ', $argumentName);
|
||||
}
|
||||
|
||||
if (BoundArgument::DEFAULTS_BINDING === $bindingType) {
|
||||
$message .= 'under "_defaults"';
|
||||
} elseif (BoundArgument::INSTANCEOF_BINDING === $bindingType) {
|
||||
$message .= 'under "_instanceof"';
|
||||
} else {
|
||||
$message .= sprintf('for service "%s"', $serviceId);
|
||||
}
|
||||
|
||||
if ($file) {
|
||||
$message .= sprintf(' in file "%s"', $file);
|
||||
}
|
||||
|
||||
$message = sprintf('A binding is configured for an argument %s, but no corresponding argument has been found. It may be unused and should be removed, or it may have a typo.', $message);
|
||||
|
||||
if ($this->errorMessages) {
|
||||
$message .= sprintf("\nCould be related to%s:", 1 < \count($this->errorMessages) ? ' one of' : '');
|
||||
}
|
||||
@ -75,12 +106,12 @@ class ResolveBindingsPass extends AbstractRecursivePass
|
||||
}
|
||||
|
||||
foreach ($bindings as $key => $binding) {
|
||||
list($bindingValue, $bindingId, $used) = $binding->getValues();
|
||||
list($bindingValue, $bindingId, $used, $bindingType, $file) = $binding->getValues();
|
||||
if ($used) {
|
||||
$this->usedBindings[$bindingId] = true;
|
||||
unset($this->unusedBindings[$bindingId]);
|
||||
} elseif (!isset($this->usedBindings[$bindingId])) {
|
||||
$this->unusedBindings[$bindingId] = [$key, $this->currentId];
|
||||
$this->unusedBindings[$bindingId] = [$key, $this->currentId, $bindingType, $file];
|
||||
}
|
||||
|
||||
if (preg_match('/^(?:(?:array|bool|float|int|string) )?\$/', $key)) {
|
||||
|
@ -11,6 +11,7 @@
|
||||
|
||||
namespace Symfony\Component\DependencyInjection\Loader\Configurator;
|
||||
|
||||
use Symfony\Component\DependencyInjection\Definition;
|
||||
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
|
||||
|
||||
/**
|
||||
@ -25,6 +26,15 @@ class DefaultsConfigurator extends AbstractServiceConfigurator
|
||||
use Traits\BindTrait;
|
||||
use Traits\PublicTrait;
|
||||
|
||||
private $path;
|
||||
|
||||
public function __construct(ServicesConfigurator $parent, Definition $definition, string $path = null)
|
||||
{
|
||||
parent::__construct($parent, $definition, null, []);
|
||||
|
||||
$this->path = $path;
|
||||
}
|
||||
|
||||
/**
|
||||
* Adds a tag for this definition.
|
||||
*
|
||||
|
@ -11,6 +11,8 @@
|
||||
|
||||
namespace Symfony\Component\DependencyInjection\Loader\Configurator;
|
||||
|
||||
use Symfony\Component\DependencyInjection\Definition;
|
||||
|
||||
/**
|
||||
* @author Nicolas Grekas <p@tchwork.com>
|
||||
*/
|
||||
@ -28,6 +30,15 @@ class InstanceofConfigurator extends AbstractServiceConfigurator
|
||||
use Traits\TagTrait;
|
||||
use Traits\BindTrait;
|
||||
|
||||
private $path;
|
||||
|
||||
public function __construct(ServicesConfigurator $parent, Definition $definition, string $id, string $path = null)
|
||||
{
|
||||
parent::__construct($parent, $definition, $id, []);
|
||||
|
||||
$this->path = $path;
|
||||
}
|
||||
|
||||
/**
|
||||
* Defines an instanceof-conditional to be applied to following service definitions.
|
||||
*/
|
||||
|
@ -45,12 +45,14 @@ class ServiceConfigurator extends AbstractServiceConfigurator
|
||||
private $container;
|
||||
private $instanceof;
|
||||
private $allowParent;
|
||||
private $path;
|
||||
|
||||
public function __construct(ContainerBuilder $container, array $instanceof, bool $allowParent, ServicesConfigurator $parent, Definition $definition, $id, array $defaultTags)
|
||||
public function __construct(ContainerBuilder $container, array $instanceof, bool $allowParent, ServicesConfigurator $parent, Definition $definition, $id, array $defaultTags, string $path = null)
|
||||
{
|
||||
$this->container = $container;
|
||||
$this->instanceof = $instanceof;
|
||||
$this->allowParent = $allowParent;
|
||||
$this->path = $path;
|
||||
|
||||
parent::__construct($parent, $definition, $id, $defaultTags);
|
||||
}
|
||||
|
@ -29,6 +29,7 @@ class ServicesConfigurator extends AbstractConfigurator
|
||||
private $container;
|
||||
private $loader;
|
||||
private $instanceof;
|
||||
private $path;
|
||||
private $anonymousHash;
|
||||
private $anonymousCount;
|
||||
|
||||
@ -38,6 +39,7 @@ class ServicesConfigurator extends AbstractConfigurator
|
||||
$this->container = $container;
|
||||
$this->loader = $loader;
|
||||
$this->instanceof = &$instanceof;
|
||||
$this->path = $path;
|
||||
$this->anonymousHash = ContainerBuilder::hash($path ?: mt_rand());
|
||||
$this->anonymousCount = &$anonymousCount;
|
||||
$instanceof = [];
|
||||
@ -48,7 +50,7 @@ class ServicesConfigurator extends AbstractConfigurator
|
||||
*/
|
||||
final public function defaults(): DefaultsConfigurator
|
||||
{
|
||||
return new DefaultsConfigurator($this, $this->defaults = new Definition());
|
||||
return new DefaultsConfigurator($this, $this->defaults = new Definition(), $this->path);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -58,7 +60,7 @@ class ServicesConfigurator extends AbstractConfigurator
|
||||
{
|
||||
$this->instanceof[$fqcn] = $definition = new ChildDefinition('');
|
||||
|
||||
return new InstanceofConfigurator($this, $definition, $fqcn);
|
||||
return new InstanceofConfigurator($this, $definition, $fqcn, $this->path);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -90,7 +92,7 @@ class ServicesConfigurator extends AbstractConfigurator
|
||||
$definition->setBindings($defaults->getBindings());
|
||||
$definition->setChanges([]);
|
||||
|
||||
$configurator = new ServiceConfigurator($this->container, $this->instanceof, $allowParent, $this, $definition, $id, $defaults->getTags());
|
||||
$configurator = new ServiceConfigurator($this->container, $this->instanceof, $allowParent, $this, $definition, $id, $defaults->getTags(), $this->path);
|
||||
|
||||
return null !== $class ? $configurator->class($class) : $configurator;
|
||||
}
|
||||
|
@ -11,7 +11,10 @@
|
||||
|
||||
namespace Symfony\Component\DependencyInjection\Loader\Configurator\Traits;
|
||||
|
||||
use Symfony\Component\DependencyInjection\Argument\BoundArgument;
|
||||
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
|
||||
use Symfony\Component\DependencyInjection\Loader\Configurator\DefaultsConfigurator;
|
||||
use Symfony\Component\DependencyInjection\Loader\Configurator\InstanceofConfigurator;
|
||||
use Symfony\Component\DependencyInjection\Reference;
|
||||
|
||||
trait BindTrait
|
||||
@ -35,7 +38,8 @@ trait BindTrait
|
||||
throw new InvalidArgumentException(sprintf('Invalid binding for service "%s": named arguments must start with a "$", and FQCN must map to references. Neither applies to binding "%s".', $this->id, $nameOrFqcn));
|
||||
}
|
||||
$bindings = $this->definition->getBindings();
|
||||
$bindings[$nameOrFqcn] = $valueOrRef;
|
||||
$type = $this instanceof DefaultsConfigurator ? BoundArgument::DEFAULTS_BINDING : ($this instanceof InstanceofConfigurator ? BoundArgument::INSTANCEOF_BINDING : BoundArgument::SERVICE_BINDING);
|
||||
$bindings[$nameOrFqcn] = new BoundArgument($valueOrRef, true, $type, $this->path ?? null);
|
||||
$this->definition->setBindings($bindings);
|
||||
|
||||
return $this;
|
||||
|
@ -175,9 +175,15 @@ class XmlFileLoader extends FileLoader
|
||||
if (null === $defaultsNode = $xpath->query('//container:services/container:defaults')->item(0)) {
|
||||
return [];
|
||||
}
|
||||
|
||||
$bindings = [];
|
||||
foreach ($this->getArgumentsAsPhp($defaultsNode, 'bind', $file) as $argument => $value) {
|
||||
$bindings[$argument] = new BoundArgument($value, true, BoundArgument::DEFAULTS_BINDING, $file);
|
||||
}
|
||||
|
||||
$defaults = [
|
||||
'tags' => $this->getChildren($defaultsNode, 'tag'),
|
||||
'bind' => array_map(function ($v) { return new BoundArgument($v); }, $this->getArgumentsAsPhp($defaultsNode, 'bind', $file)),
|
||||
'bind' => $bindings,
|
||||
];
|
||||
|
||||
foreach ($defaults['tags'] as $tag) {
|
||||
@ -368,6 +374,11 @@ class XmlFileLoader extends FileLoader
|
||||
}
|
||||
|
||||
$bindings = $this->getArgumentsAsPhp($service, 'bind', $file);
|
||||
$bindingType = $this->isLoadingInstanceof ? BoundArgument::INSTANCEOF_BINDING : BoundArgument::SERVICE_BINDING;
|
||||
foreach ($bindings as $argument => $value) {
|
||||
$bindings[$argument] = new BoundArgument($value, true, $bindingType, $file);
|
||||
}
|
||||
|
||||
if (isset($defaults['bind'])) {
|
||||
// deep clone, to avoid multiple process of the same instance in the passes
|
||||
$bindings = array_merge(unserialize(serialize($defaults['bind'])), $bindings);
|
||||
|
@ -284,7 +284,9 @@ class YamlFileLoader extends FileLoader
|
||||
throw new InvalidArgumentException(sprintf('Parameter "bind" in "_defaults" must be an array in %s. Check your YAML syntax.', $file));
|
||||
}
|
||||
|
||||
$defaults['bind'] = array_map(function ($v) { return new BoundArgument($v); }, $this->resolveServices($defaults['bind'], $file));
|
||||
foreach ($this->resolveServices($defaults['bind'], $file) as $argument => $value) {
|
||||
$defaults['bind'][$argument] = new BoundArgument($value, true, BoundArgument::DEFAULTS_BINDING, $file);
|
||||
}
|
||||
}
|
||||
|
||||
return $defaults;
|
||||
@ -534,6 +536,12 @@ class YamlFileLoader extends FileLoader
|
||||
}
|
||||
|
||||
$bindings = array_merge($bindings, $this->resolveServices($service['bind'], $file));
|
||||
$bindingType = $this->isLoadingInstanceof ? BoundArgument::INSTANCEOF_BINDING : BoundArgument::SERVICE_BINDING;
|
||||
foreach ($bindings as $argument => $value) {
|
||||
if (!$value instanceof BoundArgument) {
|
||||
$bindings[$argument] = new BoundArgument($value, true, $bindingType, $file);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
$definition->setBindings($bindings);
|
||||
|
@ -50,7 +50,7 @@ class ResolveBindingsPassTest extends TestCase
|
||||
|
||||
/**
|
||||
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
|
||||
* @expectedExceptionMessage Unused binding "$quz" in service "Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy".
|
||||
* @expectedExceptionMessage A binding is configured for an argument named "$quz" for service "Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy", but no corresponding argument has been found. It may be unused and should be removed, or it may have a typo.
|
||||
*/
|
||||
public function testUnusedBinding()
|
||||
{
|
||||
|
@ -139,8 +139,8 @@ class RegisterControllerArgumentLocatorsPass implements CompilerPassInterface
|
||||
} elseif (isset($bindings[$bindingName = $type.' $'.$p->name]) || isset($bindings[$bindingName = '$'.$p->name]) || isset($bindings[$bindingName = $type])) {
|
||||
$binding = $bindings[$bindingName];
|
||||
|
||||
list($bindingValue, $bindingId) = $binding->getValues();
|
||||
$binding->setValues([$bindingValue, $bindingId, true]);
|
||||
list($bindingValue, $bindingId, , $bindingType, $bindingFile) = $binding->getValues();
|
||||
$binding->setValues([$bindingValue, $bindingId, true, $bindingType, $bindingFile]);
|
||||
|
||||
if (!$bindingValue instanceof Reference) {
|
||||
$args[$p->name] = new Reference('.value.'.$container->hash($bindingValue));
|
||||
|
Reference in New Issue
Block a user