Skip to content

Commit 44e9a91

Browse files
nicolas-grekasnicolas-grekas
committed
bug #29597 [DI] fix reporting bindings on overriden services as unused (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
2 parents 91b28ff + e07ad2b commit 44e9a91

File tree

7 files changed

+69
-17
lines changed

7 files changed

+69
-17
lines changed

‎src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php

+2
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ class ResolveBindingsPass extends AbstractRecursivePass
3434
*/
3535
public function process(ContainerBuilder $container)
3636
{
37+
$this->usedBindings = $container->getRemovedBindingIds();
38+
3739
try {
3840
parent::process($container);
3941

‎src/Symfony/Component/DependencyInjection/ContainerBuilder.php

+39-7
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
123123

124124
private $removedIds = array();
125125

126+
private $removedBindingIds = array();
127+
126128
private static $internalTypes = array(
127129
'int' => true,
128130
'float' => true,
@@ -531,7 +533,8 @@ public function set($id, $service)
531533
throw new BadMethodCallException(sprintf('Setting service "%s" for an unknown or non-synthetic service definition on a compiled container is not allowed.', $id));
532534
}
533535

534-
unset($this->definitions[$id], $this->aliasDefinitions[$id], $this->removedIds[$id]);
536+
$this->removeId($id);
537+
unset($this->removedIds[$id]);
535538

536539
parent::set($id, $service);
537540
}
@@ -544,8 +547,7 @@ public function set($id, $service)
544547
public function removeDefinition($id)
545548
{
546549
if (isset($this->definitions[$id = $this->normalizeId($id)])) {
547-
unset($this->definitions[$id]);
548-
$this->removedIds[$id] = true;
550+
$this->removeId($id);
549551
}
550552
}
551553

@@ -876,7 +878,8 @@ public function setAlias($alias, $id)
876878
throw new InvalidArgumentException(sprintf('An alias can not reference itself, got a circular reference on "%s".', $alias));
877879
}
878880

879-
unset($this->definitions[$alias], $this->removedIds[$alias]);
881+
$this->removeId($alias);
882+
unset($this->removedIds[$alias]);
880883

881884
return $this->aliasDefinitions[$alias] = $id;
882885
}
@@ -889,8 +892,7 @@ public function setAlias($alias, $id)
889892
public function removeAlias($alias)
890893
{
891894
if (isset($this->aliasDefinitions[$alias = $this->normalizeId($alias)])) {
892-
unset($this->aliasDefinitions[$alias]);
893-
$this->removedIds[$alias] = true;
895+
$this->removeId($alias);
894896
}
895897
}
896898

@@ -1019,7 +1021,8 @@ public function setDefinition($id, Definition $definition)
10191021

10201022
$id = $this->normalizeId($id);
10211023

1022-
unset($this->aliasDefinitions[$id], $this->removedIds[$id]);
1024+
$this->removeId($id);
1025+
unset($this->removedIds[$id]);
10231026

10241027
return $this->definitions[$id] = $definition;
10251028
}
@@ -1552,6 +1555,18 @@ public static function getInitializedConditionals($value)
15521555
return $services;
15531556
}
15541557

1558+
/**
1559+
* Gets removed binding ids.
1560+
*
1561+
* @return array
1562+
*
1563+
* @internal
1564+
*/
1565+
public function getRemovedBindingIds()
1566+
{
1567+
return $this->removedBindingIds;
1568+
}
1569+
15551570
/**
15561571
* Computes a reasonably unique hash of a value.
15571572
*
@@ -1656,4 +1671,21 @@ private function inVendors($path)
16561671

16571672
return false;
16581673
}
1674+
1675+
private function removeId($id)
1676+
{
1677+
$this->removedIds[$id] = true;
1678+
unset($this->aliasDefinitions[$id]);
1679+
1680+
if (!isset($this->definitions[$id])) {
1681+
return;
1682+
}
1683+
1684+
foreach ($this->definitions[$id]->getBindings() as $binding) {
1685+
list(, $identifier) = $binding->getValues();
1686+
$this->removedBindingIds[$identifier] = true;
1687+
}
1688+
1689+
unset($this->definitions[$id]);
1690+
}
16591691
}

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php

+18
Original file line numberDiff line numberDiff line change
@@ -111,4 +111,22 @@ public function testScalarSetter()
111111

112112
$this->assertEquals(array(array('setDefaultLocale', array('fr'))), $definition->getMethodCalls());
113113
}
114+
115+
public function testOverriddenBindings()
116+
{
117+
$container = new ContainerBuilder();
118+
119+
$binding = new BoundArgument('bar');
120+
121+
$container->register('foo', 'stdClass')
122+
->setBindings(array('$foo' => clone $binding));
123+
$container->register('bar', 'stdClass')
124+
->setBindings(array('$foo' => clone $binding));
125+
126+
$container->register('foo', 'stdClass');
127+
128+
(new ResolveBindingsPass())->process($container);
129+
130+
$this->assertInstanceOf('stdClass', $container->get('foo'));
131+
}
114132
}

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveChildDefinitionsPassTest.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ protected function process(ContainerBuilder $container)
434434

435435
/**
436436
* @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException
437-
* @expectedExceptionMessageRegExp /^Circular reference detected for service "c", path: "c -> b -> a -> c"./
437+
* @expectedExceptionMessageRegExp /^Circular reference detected for service "a", path: "a -> c -> b -> a"./
438438
*/
439439
public function testProcessDetectsChildDefinitionIndirectCircularReference()
440440
{

‎src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,7 @@ public function testMerge()
559559
$config->setDefinition('baz', new Definition('BazClass'));
560560
$config->setAlias('alias_for_foo', 'foo');
561561
$container->merge($config);
562-
$this->assertEquals(array('service_container', 'foo', 'bar', 'baz'), array_keys($container->getDefinitions()), '->merge() merges definitions already defined ones');
562+
$this->assertEquals(array('foo', 'bar', 'service_container', 'baz'), array_keys($container->getDefinitions()), '->merge() merges definitions already defined ones');
563563

564564
$aliases = $container->getAliases();
565565
$this->assertArrayHasKey('alias_for_foo', $aliases);

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.expected.yml

+3-3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ services:
44
class: Symfony\Component\DependencyInjection\ContainerInterface
55
public: true
66
synthetic: true
7+
foo:
8+
class: App\FooService
9+
public: true
710
Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo:
811
class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo
912
public: true
@@ -16,6 +19,3 @@ services:
1619

1720
shared: false
1821
configurator: c
19-
foo:
20-
class: App\FooService
21-
public: true

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.expected.yml

+5-5
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,22 @@ services:
44
class: Symfony\Component\DependencyInjection\ContainerInterface
55
public: true
66
synthetic: true
7-
Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo:
8-
class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo
7+
Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar:
8+
class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar
99
public: true
1010
tags:
1111
- { name: foo }
1212
- { name: baz }
1313
deprecated: '%service_id%'
14+
lazy: true
1415
arguments: [1]
1516
factory: f
16-
Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar:
17-
class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar
17+
Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo:
18+
class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo
1819
public: true
1920
tags:
2021
- { name: foo }
2122
- { name: baz }
2223
deprecated: '%service_id%'
23-
lazy: true
2424
arguments: [1]
2525
factory: f

0 commit comments

Comments
 (0)