diff --git a/src/Symfony/Component/DependencyInjection/Argument/BoundArgument.php b/src/Symfony/Component/DependencyInjection/Argument/BoundArgument.php index b2fc28d8d56db..4889c8e7cb02d 100644 --- a/src/Symfony/Component/DependencyInjection/Argument/BoundArgument.php +++ b/src/Symfony/Component/DependencyInjection/Argument/BoundArgument.php @@ -16,13 +16,19 @@ */ 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 @@ public function __construct($value, bool $trackUsage = true) } else { $this->used = true; } + $this->type = $type; + $this->file = $file; } /** @@ -37,7 +45,7 @@ public function __construct($value, bool $trackUsage = true) */ 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 @@ public function getValues() */ 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; } } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php index db6c588f91c5a..5cee0023060c4 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php @@ -37,8 +37,39 @@ public function process(ContainerBuilder $container) 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 @@ protected function processValue($value, $isRoot = false) } 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)) { diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/DefaultsConfigurator.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/DefaultsConfigurator.php index b928d75860296..b527fd51da153 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/Configurator/DefaultsConfigurator.php +++ b/src/Symfony/Component/DependencyInjection/Loader/Configurator/DefaultsConfigurator.php @@ -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. * diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/InstanceofConfigurator.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/InstanceofConfigurator.php index ad9a6872b6800..f75e17614476f 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/Configurator/InstanceofConfigurator.php +++ b/src/Symfony/Component/DependencyInjection/Loader/Configurator/InstanceofConfigurator.php @@ -11,6 +11,8 @@ namespace Symfony\Component\DependencyInjection\Loader\Configurator; +use Symfony\Component\DependencyInjection\Definition; + /** * @author Nicolas Grekas */ @@ -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. */ diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServiceConfigurator.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServiceConfigurator.php index 8fc60fbd376bd..13584973af5ff 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServiceConfigurator.php +++ b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServiceConfigurator.php @@ -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); } diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php index 366828ea4cbdc..b693fc2d66e7e 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php +++ b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php @@ -29,6 +29,7 @@ class ServicesConfigurator extends AbstractConfigurator private $container; private $loader; private $instanceof; + private $path; private $anonymousHash; private $anonymousCount; @@ -38,6 +39,7 @@ public function __construct(ContainerBuilder $container, PhpFileLoader $loader, $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 @@ public function __construct(ContainerBuilder $container, PhpFileLoader $loader, */ 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 @@ final public function instanceof(string $fqcn): InstanceofConfigurator { $this->instanceof[$fqcn] = $definition = new ChildDefinition(''); - return new InstanceofConfigurator($this, $definition, $fqcn); + return new InstanceofConfigurator($this, $definition, $fqcn, $this->path); } /** @@ -90,7 +92,7 @@ final public function set(?string $id, string $class = null): ServiceConfigurato $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; } diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/Traits/BindTrait.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/Traits/BindTrait.php index 5d6c57cb3b9b2..621158fdee277 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/Configurator/Traits/BindTrait.php +++ b/src/Symfony/Component/DependencyInjection/Loader/Configurator/Traits/BindTrait.php @@ -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 @@ final public function bind($nameOrFqcn, $valueOrRef) 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; diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index 6a07c613e2cdd..bbe2d5569579b 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -175,9 +175,15 @@ private function getServiceDefaults(\DOMDocument $xml, $file) 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 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults) } $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); diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index 198f00d64cb8e..703620c8a67e3 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -284,7 +284,9 @@ private function parseDefaults(array &$content, string $file): array 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 @@ private function parseDefinition($id, $service, $file, array $defaults) } $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); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php index 7bbecf6207f46..97855e7bd913b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php @@ -50,7 +50,7 @@ public function testProcess() /** * @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() { diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php index c5a823cc7ee1e..a3f5012e3268f 100644 --- a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php +++ b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php @@ -139,8 +139,8 @@ public function process(ContainerBuilder $container) } 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));