From 3378889cdef240146740fa2258858df7449f8f90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Bogusz?= Date: Wed, 16 Jan 2019 01:12:14 +0100 Subject: [PATCH] Fixes issues #27828 and #28326 --- .../Argument/BoundArgument.php | 17 ++++++++-- .../Compiler/ResolveBindingsPass.php | 31 ++++++++++++++++--- .../DependencyInjection/ContainerBuilder.php | 18 +++++++++++ .../Loader/YamlFileLoader.php | 10 +++++- .../Tests/Fixtures/Foo.php | 10 ++++++ .../Tests/Fixtures/yaml/defaults_bindings.yml | 14 +++++++++ .../Fixtures/yaml/defaults_bindings2.yml | 10 ++++++ .../Fixtures/yaml/defaults_bindings3.yml | 8 +++++ .../Tests/Loader/YamlFileLoaderTest.php | 19 ++++++++++++ 9 files changed, 130 insertions(+), 7 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/Foo.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings.yml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings2.yml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings3.yml diff --git a/src/Symfony/Component/DependencyInjection/Argument/BoundArgument.php b/src/Symfony/Component/DependencyInjection/Argument/BoundArgument.php index a20698440c3cf..54eb40f72f48f 100644 --- a/src/Symfony/Component/DependencyInjection/Argument/BoundArgument.php +++ b/src/Symfony/Component/DependencyInjection/Argument/BoundArgument.php @@ -16,16 +16,29 @@ */ final class BoundArgument implements ArgumentInterface { + const SERVICE_BIND = 0; + const DEFAULT_BIND = 1; + const INSTANCE_BIND = 2; + + const MESSAGES = [ + 1 => 'under "_defaults"', + 2 => 'under "_instanceof"', + ]; + private static $sequence = 0; private $value; private $identifier; private $used; + private $type; + private $file; - public function __construct($value) + public function __construct($value, $type = 0, $file = null) { $this->value = $value; $this->identifier = ++self::$sequence; + $this->type = (int) $type; + $this->file = (string) $file; } /** @@ -33,7 +46,7 @@ public function __construct($value) */ public function getValues() { - return [$this->value, $this->identifier, $this->used]; + return [$this->value, $this->identifier, $this->used, $this->type, $this->file]; } /** diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php index 0fad285e5aaf4..bbe76822a68d6 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php @@ -34,11 +34,34 @@ class ResolveBindingsPass extends AbstractRecursivePass */ public function process(ContainerBuilder $container) { + foreach ($container->getBindings() as $definition) { + foreach ($definition as $argument => $values) { + if (1 < \count($values)) { + foreach (\array_slice(array_keys($values), 0, -1) as $value) { + $this->usedBindings[$value] = true; + } + } + } + } + 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, $type, $file)) { + $message = sprintf('You have a "bind" configured for an argument named "%s" ', $key); + + if ($type) { + $message .= BoundArgument::MESSAGES[$type]; + } else { + $message .= sprintf('in service "%s"', $serviceId); + } + + if ($file) { + $message .= sprintf(' in file "%s"', $file); + } + + $message .= '. But, this argument was not found in any of the services it was applied to. It may be unused and can be removed, or it may have a typo.'; + if ($this->errorMessages) { $message .= sprintf("\nCould be related to%s:", 1 < \count($this->errorMessages) ? ' one of' : ''); } @@ -75,12 +98,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 (isset($key[0]) && '$' === $key[0]) { diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 0c1966a42e255..a7c54f8d82230 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -123,6 +123,11 @@ class ContainerBuilder extends Container implements TaggedContainerInterface private $removedIds = []; + /** + * @var bool[][][] a map of values bound to arguments in definitions + */ + private $bindings = []; + private static $internalTypes = [ 'int' => true, 'float' => true, @@ -1021,6 +1026,14 @@ public function setDefinition($id, Definition $definition) unset($this->aliasDefinitions[$id], $this->removedIds[$id]); + $bindings = $definition->getBindings(); + if (!empty($bindings)) { + foreach ($bindings as $argument => $binding) { + list(, $identifier) = $binding->getValues(); + $this->bindings[$id][$argument][$identifier] = true; + } + } + return $this->definitions[$id] = $definition; } @@ -1656,4 +1669,9 @@ private function inVendors($path) return false; } + + public function getBindings(): array + { + return $this->bindings; + } } diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index dccbae9b73934..5d7582e25030b 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -299,7 +299,9 @@ private function parseDefaults(array &$content, $file) 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, BoundArgument::DEFAULT_BIND, $file); + } } return $defaults; @@ -558,6 +560,12 @@ private function parseDefinition($id, $service, $file, array $defaults) } $bindings = array_merge($bindings, $this->resolveServices($service['bind'], $file)); + + $bindingType = $this->isLoadingInstanceof ? BoundArgument::INSTANCE_BIND : BoundArgument::SERVICE_BIND; + + foreach ($bindings as $argument => $value) { + $bindings[$argument] = new BoundArgument($value, $bindingType, $file); + } } $definition->setBindings($bindings); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Foo.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Foo.php new file mode 100644 index 0000000000000..5cba442d81198 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Foo.php @@ -0,0 +1,10 @@ + 'factory', ], array_map(function ($v) { return $v->getValues()[0]; }, $definition->getBindings())); } + + /** + * The pass may throw an exception, which will cause the test to fail. + */ + public function testOverriddenDefaultsBindings() + { + $container = new ContainerBuilder(); + + $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml')); + $loader->load('defaults_bindings.yml'); + $loader->load('defaults_bindings2.yml'); + $loader->load('defaults_bindings3.yml'); + + $pass = new ResolveBindingsPass(); + $pass->process($container); + + $this->assertTrue(true); + } }