From 540567625065b604be1e89f6393b552dbf99e058 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 13 Dec 2018 17:15:21 +0100 Subject: [PATCH] [DI] fix reporting bindings on overriden services as unused --- .../DependencyInjection/ContainerBuilder.php | 32 +++++++++++++++---- .../Compiler/ResolveBindingsPassTest.php | 18 +++++++++++ .../ResolveChildDefinitionsPassTest.php | 2 +- .../Tests/ContainerBuilderTest.php | 2 +- .../Fixtures/config/instanceof.expected.yml | 6 ++-- .../Fixtures/config/prototype.expected.yml | 10 +++--- 6 files changed, 53 insertions(+), 17 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 5fe6f2ae6fdfd..1dbe69a437de8 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -531,7 +531,8 @@ public function set($id, $service) throw new BadMethodCallException(sprintf('Setting service "%s" for an unknown or non-synthetic service definition on a compiled container is not allowed.', $id)); } - unset($this->definitions[$id], $this->aliasDefinitions[$id], $this->removedIds[$id]); + $this->removeId($id); + unset($this->removedIds[$id]); parent::set($id, $service); } @@ -544,8 +545,7 @@ public function set($id, $service) public function removeDefinition($id) { if (isset($this->definitions[$id = $this->normalizeId($id)])) { - unset($this->definitions[$id]); - $this->removedIds[$id] = true; + $this->removeId($id); } } @@ -876,7 +876,8 @@ public function setAlias($alias, $id) throw new InvalidArgumentException(sprintf('An alias can not reference itself, got a circular reference on "%s".', $alias)); } - unset($this->definitions[$alias], $this->removedIds[$alias]); + $this->removeId($alias); + unset($this->removedIds[$alias]); return $this->aliasDefinitions[$alias] = $id; } @@ -889,8 +890,7 @@ public function setAlias($alias, $id) public function removeAlias($alias) { if (isset($this->aliasDefinitions[$alias = $this->normalizeId($alias)])) { - unset($this->aliasDefinitions[$alias]); - $this->removedIds[$alias] = true; + $this->removeId($alias); } } @@ -1019,7 +1019,8 @@ public function setDefinition($id, Definition $definition) $id = $this->normalizeId($id); - unset($this->aliasDefinitions[$id], $this->removedIds[$id]); + $this->removeId($id); + unset($this->removedIds[$id]); return $this->definitions[$id] = $definition; } @@ -1656,4 +1657,21 @@ private function inVendors($path) return false; } + + private function removeId($id) + { + $this->removedIds[$id] = true; + unset($this->aliasDefinitions[$id]); + + if (!isset($this->definitions[$id])) { + return; + } + + foreach ($this->definitions[$id]->getBindings() as $binding) { + list($value, $identifier) = $binding->getValues(); + $binding->setValues(array($value, $identifier, true)); + } + + unset($this->definitions[$id]); + } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php index d59b95af5c406..edca01ea7f16e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php @@ -111,4 +111,22 @@ public function testScalarSetter() $this->assertEquals(array(array('setDefaultLocale', array('fr'))), $definition->getMethodCalls()); } + + public function testOverriddenBindings() + { + $container = new ContainerBuilder(); + + $binding = new BoundArgument('bar'); + + $container->register('foo', 'stdClass') + ->setBindings(array('$foo' => $binding)); + $container->register('bar', 'stdClass') + ->setBindings(array('$foo' => $binding)); + + $container->register('foo', 'stdClass'); + + (new ResolveBindingsPass())->process($container); + + $this->assertInstanceOf('stdClass', $container->get('foo')); + } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveChildDefinitionsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveChildDefinitionsPassTest.php index 1575bd7b07751..863f2833e5f7e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveChildDefinitionsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveChildDefinitionsPassTest.php @@ -434,7 +434,7 @@ protected function process(ContainerBuilder $container) /** * @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException - * @expectedExceptionMessageRegExp /^Circular reference detected for service "c", path: "c -> b -> a -> c"./ + * @expectedExceptionMessageRegExp /^Circular reference detected for service "a", path: "a -> c -> b -> a"./ */ public function testProcessDetectsChildDefinitionIndirectCircularReference() { diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php index 0bf1befa3f84d..354b44187d6bc 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php @@ -559,7 +559,7 @@ public function testMerge() $config->setDefinition('baz', new Definition('BazClass')); $config->setAlias('alias_for_foo', 'foo'); $container->merge($config); - $this->assertEquals(array('service_container', 'foo', 'bar', 'baz'), array_keys($container->getDefinitions()), '->merge() merges definitions already defined ones'); + $this->assertEquals(array('foo', 'bar', 'service_container', 'baz'), array_keys($container->getDefinitions()), '->merge() merges definitions already defined ones'); $aliases = $container->getAliases(); $this->assertArrayHasKey('alias_for_foo', $aliases); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.expected.yml index b12a304221dd8..9f0bfbba78a2e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.expected.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.expected.yml @@ -4,6 +4,9 @@ services: class: Symfony\Component\DependencyInjection\ContainerInterface public: true synthetic: true + foo: + class: App\FooService + public: true Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo: class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo public: true @@ -16,6 +19,3 @@ services: shared: false configurator: c - foo: - class: App\FooService - public: true diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.expected.yml index ebfe087d779cf..0af4d530a0879 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.expected.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.expected.yml @@ -4,22 +4,22 @@ services: class: Symfony\Component\DependencyInjection\ContainerInterface public: true synthetic: true - Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo: - class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar: + class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar public: true tags: - { name: foo } - { name: baz } deprecated: '%service_id%' + lazy: true arguments: [1] factory: f - Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar: - class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo: + class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo public: true tags: - { name: foo } - { name: baz } deprecated: '%service_id%' - lazy: true arguments: [1] factory: f