Skip to content

Commit 3378889

Browse files
przemyslaw-boguszprzemyslaw-bogusz
committed
Fixes issues #27828 and #28326 
1 parent b6b5976 commit 3378889

File tree

9 files changed

+130
-7
lines changed

9 files changed

+130
-7
lines changed

‎src/Symfony/Component/DependencyInjection/Argument/BoundArgument.php

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,24 +16,37 @@
1616
*/
1717
final class BoundArgument implements ArgumentInterface
1818
{
19+
const SERVICE_BIND = 0;
20+
const DEFAULT_BIND = 1;
21+
const INSTANCE_BIND = 2;
22+
23+
const MESSAGES = [
24+
1 => 'under "_defaults"',
25+
2 => 'under "_instanceof"',
26+
];
27+
1928
private static $sequence = 0;
2029

2130
private $value;
2231
private $identifier;
2332
private $used;
33+
private $type;
34+
private $file;
2435

25-
public function __construct($value)
36+
public function __construct($value, $type = 0, $file = null)
2637
{
2738
$this->value = $value;
2839
$this->identifier = ++self::$sequence;
40+
$this->type = (int) $type;
41+
$this->file = (string) $file;
2942
}
3043

3144
/**
3245
* {@inheritdoc}
3346
*/
3447
public function getValues()
3548
{
36-
return [$this->value, $this->identifier, $this->used];
49+
return [$this->value, $this->identifier, $this->used, $this->type, $this->file];
3750
}
3851

3952
/**

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

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,34 @@ class ResolveBindingsPass extends AbstractRecursivePass
3434
*/
3535
public function process(ContainerBuilder $container)
3636
{
37+
foreach ($container->getBindings() as $definition) {
38+
foreach ($definition as $argument => $values) {
39+
if (1 < \count($values)) {
40+
foreach (\array_slice(array_keys($values), 0, -1) as $value) {
41+
$this->usedBindings[$value] = true;
42+
}
43+
}
44+
}
45+
}
46+
3747
try {
3848
parent::process($container);
3949

40-
foreach ($this->unusedBindings as list($key, $serviceId)) {
41-
$message = sprintf('Unused binding "%s" in service "%s".', $key, $serviceId);
50+
foreach ($this->unusedBindings as list($key, $serviceId, $type, $file)) {
51+
$message = sprintf('You have a "bind" configured for an argument named "%s" ', $key);
52+
53+
if ($type) {
54+
$message .= BoundArgument::MESSAGES[$type];
55+
} else {
56+
$message .= sprintf('in service "%s"', $serviceId);
57+
}
58+
59+
if ($file) {
60+
$message .= sprintf(' in file "%s"', $file);
61+
}
62+
63+
$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.';
64+
4265
if ($this->errorMessages) {
4366
$message .= sprintf("\nCould be related to%s:", 1 < \count($this->errorMessages) ? ' one of' : '');
4467
}
@@ -75,12 +98,12 @@ protected function processValue($value, $isRoot = false)
7598
}
7699

77100
foreach ($bindings as $key => $binding) {
78-
list($bindingValue, $bindingId, $used) = $binding->getValues();
101+
list($bindingValue, $bindingId, $used, $bindingType, $file) = $binding->getValues();
79102
if ($used) {
80103
$this->usedBindings[$bindingId] = true;
81104
unset($this->unusedBindings[$bindingId]);
82105
} elseif (!isset($this->usedBindings[$bindingId])) {
83-
$this->unusedBindings[$bindingId] = [$key, $this->currentId];
106+
$this->unusedBindings[$bindingId] = [$key, $this->currentId, $bindingType, $file];
84107
}
85108

86109
if (isset($key[0]) && '$' === $key[0]) {

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,11 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
123123

124124
private $removedIds = [];
125125

126+
/**
127+
* @var bool[][][] a map of values bound to arguments in definitions
128+
*/
129+
private $bindings = [];
130+
126131
private static $internalTypes = [
127132
'int' => true,
128133
'float' => true,
@@ -1021,6 +1026,14 @@ public function setDefinition($id, Definition $definition)
10211026

10221027
unset($this->aliasDefinitions[$id], $this->removedIds[$id]);
10231028

1029+
$bindings = $definition->getBindings();
1030+
if (!empty($bindings)) {
1031+
foreach ($bindings as $argument => $binding) {
1032+
list(, $identifier) = $binding->getValues();
1033+
$this->bindings[$id][$argument][$identifier] = true;
1034+
}
1035+
}
1036+
10241037
return $this->definitions[$id] = $definition;
10251038
}
10261039

@@ -1656,4 +1669,9 @@ private function inVendors($path)
16561669

16571670
return false;
16581671
}
1672+
1673+
public function getBindings(): array
1674+
{
1675+
return $this->bindings;
1676+
}
16591677
}

‎src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,9 @@ private function parseDefaults(array &$content, $file)
299299
throw new InvalidArgumentException(sprintf('Parameter "bind" in "_defaults" must be an array in %s. Check your YAML syntax.', $file));
300300
}
301301

302-
$defaults['bind'] = array_map(function ($v) { return new BoundArgument($v); }, $this->resolveServices($defaults['bind'], $file));
302+
foreach ($this->resolveServices($defaults['bind'], $file) as $argument => $value) {
303+
$defaults['bind'][$argument] = new BoundArgument($value, BoundArgument::DEFAULT_BIND, $file);
304+
}
303305
}
304306

305307
return $defaults;
@@ -558,6 +560,12 @@ private function parseDefinition($id, $service, $file, array $defaults)
558560
}
559561

560562
$bindings = array_merge($bindings, $this->resolveServices($service['bind'], $file));
563+
564+
$bindingType = $this->isLoadingInstanceof ? BoundArgument::INSTANCE_BIND : BoundArgument::SERVICE_BIND;
565+
566+
foreach ($bindings as $argument => $value) {
567+
$bindings[$argument] = new BoundArgument($value, $bindingType, $file);
568+
}
561569
}
562570

563571
$definition->setBindings($bindings);

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/Foo.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
4+
5+
class Foo
6+
{
7+
public function __construct($abc)
8+
{
9+
}
10+
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings.yml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
services:
2+
_defaults:
3+
bind:
4+
$quz: quzFirstValue
5+
$abc: abcFirstValue
6+
7+
bar:
8+
class: Symfony\Component\DependencyInjection\Tests\Fixtures\Bar
9+
10+
factory:
11+
class: Symfony\Component\DependencyInjection\Tests\Fixtures\FactoryDummy
12+
13+
foo:
14+
class: Symfony\Component\DependencyInjection\Tests\Fixtures\Foo

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings2.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
services:
2+
_defaults:
3+
bind:
4+
$abc: abcSecondValue
5+
6+
foo:
7+
class: Symfony\Component\DependencyInjection\Tests\Fixtures\Foo
8+
9+
factory:
10+
class: Symfony\Component\DependencyInjection\Tests\Fixtures\FactoryDummy

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings3.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
services:
2+
_defaults:
3+
bind:
4+
$abc: abcThirdValue
5+
6+
foo:
7+
class: Symfony\Component\DependencyInjection\Tests\Fixtures\Foo
8+
bind:

‎src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Symfony\Component\Config\Resource\FileResource;
1818
use Symfony\Component\Config\Resource\GlobResource;
1919
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
20+
use Symfony\Component\DependencyInjection\Compiler\ResolveBindingsPass;
2021
use Symfony\Component\DependencyInjection\ContainerBuilder;
2122
use Symfony\Component\DependencyInjection\Loader\IniFileLoader;
2223
use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
@@ -732,4 +733,22 @@ public function testBindings()
732733
'$factory' => 'factory',
733734
], array_map(function ($v) { return $v->getValues()[0]; }, $definition->getBindings()));
734735
}
736+
737+
/**
738+
* The pass may throw an exception, which will cause the test to fail.
739+
*/
740+
public function testOverriddenDefaultsBindings()
741+
{
742+
$container = new ContainerBuilder();
743+
744+
$loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml'));
745+
$loader->load('defaults_bindings.yml');
746+
$loader->load('defaults_bindings2.yml');
747+
$loader->load('defaults_bindings3.yml');
748+
749+
$pass = new ResolveBindingsPass();
750+
$pass->process($container);
751+
752+
$this->assertTrue(true);
753+
}
735754
}

0 commit comments

Comments
 (0)