Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
bug #29597 [DI] fix reporting bindings on overriden services as unuse…
…d (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[DI] fix reporting bindings on overriden services as unused

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #28326
| License       | MIT
| Doc PR        | -

Commits
-------

e07ad2b [DI] fix reporting bindings on overriden services as unused
  • Loading branch information
nicolas-grekas committed Dec 24, 2018
2 parents 91b28ff + e07ad2b commit 44e9a91
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 17 deletions.
Expand Up @@ -34,6 +34,8 @@ class ResolveBindingsPass extends AbstractRecursivePass
*/
public function process(ContainerBuilder $container)
{
$this->usedBindings = $container->getRemovedBindingIds();

try {
parent::process($container);

Expand Down
46 changes: 39 additions & 7 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
Expand Up @@ -123,6 +123,8 @@ class ContainerBuilder extends Container implements TaggedContainerInterface

private $removedIds = array();

private $removedBindingIds = array();

private static $internalTypes = array(
'int' => true,
'float' => true,
Expand Down Expand Up @@ -531,7 +533,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);
}
Expand All @@ -544,8 +547,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);
}
}

Expand Down Expand Up @@ -876,7 +878,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;
}
Expand All @@ -889,8 +892,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);
}
}

Expand Down Expand Up @@ -1019,7 +1021,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;
}
Expand Down Expand Up @@ -1552,6 +1555,18 @@ public static function getInitializedConditionals($value)
return $services;
}

/**
* Gets removed binding ids.
*
* @return array
*
* @internal
*/
public function getRemovedBindingIds()
{
return $this->removedBindingIds;
}

/**
* Computes a reasonably unique hash of a value.
*
Expand Down Expand Up @@ -1656,4 +1671,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(, $identifier) = $binding->getValues();
$this->removedBindingIds[$identifier] = true;
}

unset($this->definitions[$id]);
}
}
Expand Up @@ -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' => clone $binding));
$container->register('bar', 'stdClass')
->setBindings(array('$foo' => clone $binding));

$container->register('foo', 'stdClass');

(new ResolveBindingsPass())->process($container);

$this->assertInstanceOf('stdClass', $container->get('foo'));
}
}
Expand Up @@ -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()
{
Expand Down
Expand Up @@ -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);
Expand Down
Expand Up @@ -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
Expand All @@ -16,6 +19,3 @@ services:

shared: false
configurator: c
foo:
class: App\FooService
public: true
Expand Up @@ -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

0 comments on commit 44e9a91

Please sign in to comment.