Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit b0989aa

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
feature #29935 [DI] Fix bad error message for unused bind under _defaults (przemyslaw-bogusz)
This PR was merged into the 4.3-dev branch. Discussion ---------- [DI] Fix bad error message for unused bind under _defaults | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #27828 | License | MIT **Sidenote**: I originally included the fix in #29897, but I decided to close the previous PR and divide it into two separate PRs for clarity. **Description:** With this fix, the message regarding an unused bind will have a clear information about the type of the bind (defined under __defaults_, __instanceof_ or _per service_), as well as the name of the file, in which it was configurated. It's for, both, YAML and XML configurations. For the core team, please note, that the fix assumes a possibility of definings binds under __instanceof_, which was introduced in #27806. But since fixes are merged into other branches, I thought that it might be necessary to inlude this possibility. If this case requires making separate fixes for different branches, I will gladly do it. Commits ------- 35bf420 [DI] Fix bad error message for unused bind under _defaults
2 parents 3df05e0 + 35bf420 commit b0989aa
Copy full SHA for b0989aa

File tree

11 files changed

+104
-17
lines changed
Filter options

11 files changed

+104
-17
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Argument/BoundArgument.php
+11-3Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,35 +16,43 @@
1616
*/
1717
final class BoundArgument implements ArgumentInterface
1818
{
19+
const SERVICE_BINDING = 0;
20+
const DEFAULTS_BINDING = 1;
21+
const INSTANCEOF_BINDING = 2;
22+
1923
private static $sequence = 0;
2024

2125
private $value;
2226
private $identifier;
2327
private $used;
28+
private $type;
29+
private $file;
2430

25-
public function __construct($value, bool $trackUsage = true)
31+
public function __construct($value, bool $trackUsage = true, int $type = 0, string $file = null)
2632
{
2733
$this->value = $value;
2834
if ($trackUsage) {
2935
$this->identifier = ++self::$sequence;
3036
} else {
3137
$this->used = true;
3238
}
39+
$this->type = $type;
40+
$this->file = $file;
3341
}
3442

3543
/**
3644
* {@inheritdoc}
3745
*/
3846
public function getValues()
3947
{
40-
return [$this->value, $this->identifier, $this->used];
48+
return [$this->value, $this->identifier, $this->used, $this->type, $this->file];
4149
}
4250

4351
/**
4452
* {@inheritdoc}
4553
*/
4654
public function setValues(array $values)
4755
{
48-
list($this->value, $this->identifier, $this->used) = $values;
56+
list($this->value, $this->identifier, $this->used, $this->type, $this->file) = $values;
4957
}
5058
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php
+35-4Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,39 @@ public function process(ContainerBuilder $container)
3737
try {
3838
parent::process($container);
3939

40-
foreach ($this->unusedBindings as list($key, $serviceId)) {
41-
$message = sprintf('Unused binding "%s" in service "%s".', $key, $serviceId);
40+
foreach ($this->unusedBindings as list($key, $serviceId, $bindingType, $file)) {
41+
$argumentType = $argumentName = $message = null;
42+
43+
if (false !== strpos($key, ' ')) {
44+
list($argumentType, $argumentName) = explode(' ', $key, 2);
45+
} elseif ('$' === $key[0]) {
46+
$argumentName = $key;
47+
} else {
48+
$argumentType = $key;
49+
}
50+
51+
if ($argumentType) {
52+
$message .= sprintf('of type "%s" ', $argumentType);
53+
}
54+
55+
if ($argumentName) {
56+
$message .= sprintf('named "%s" ', $argumentName);
57+
}
58+
59+
if (BoundArgument::DEFAULTS_BINDING === $bindingType) {
60+
$message .= 'under "_defaults"';
61+
} elseif (BoundArgument::INSTANCEOF_BINDING === $bindingType) {
62+
$message .= 'under "_instanceof"';
63+
} else {
64+
$message .= sprintf('for service "%s"', $serviceId);
65+
}
66+
67+
if ($file) {
68+
$message .= sprintf(' in file "%s"', $file);
69+
}
70+
71+
$message = sprintf('A binding is configured for an argument %s, but no corresponding argument has been found. It may be unused and should be removed, or it may have a typo.', $message);
72+
4273
if ($this->errorMessages) {
4374
$message .= sprintf("\nCould be related to%s:", 1 < \count($this->errorMessages) ? ' one of' : '');
4475
}
@@ -75,12 +106,12 @@ protected function processValue($value, $isRoot = false)
75106
}
76107

77108
foreach ($bindings as $key => $binding) {
78-
list($bindingValue, $bindingId, $used) = $binding->getValues();
109+
list($bindingValue, $bindingId, $used, $bindingType, $file) = $binding->getValues();
79110
if ($used) {
80111
$this->usedBindings[$bindingId] = true;
81112
unset($this->unusedBindings[$bindingId]);
82113
} elseif (!isset($this->usedBindings[$bindingId])) {
83-
$this->unusedBindings[$bindingId] = [$key, $this->currentId];
114+
$this->unusedBindings[$bindingId] = [$key, $this->currentId, $bindingType, $file];
84115
}
85116

86117
if (preg_match('/^(?:(?:array|bool|float|int|string) )?\$/', $key)) {

‎src/Symfony/Component/DependencyInjection/Loader/Configurator/DefaultsConfigurator.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/Configurator/DefaultsConfigurator.php
+10Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\DependencyInjection\Loader\Configurator;
1313

14+
use Symfony\Component\DependencyInjection\Definition;
1415
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
1516

1617
/**
@@ -25,6 +26,15 @@ class DefaultsConfigurator extends AbstractServiceConfigurator
2526
use Traits\BindTrait;
2627
use Traits\PublicTrait;
2728

29+
private $path;
30+
31+
public function __construct(ServicesConfigurator $parent, Definition $definition, string $path = null)
32+
{
33+
parent::__construct($parent, $definition, null, []);
34+
35+
$this->path = $path;
36+
}
37+
2838
/**
2939
* Adds a tag for this definition.
3040
*

‎src/Symfony/Component/DependencyInjection/Loader/Configurator/InstanceofConfigurator.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/Configurator/InstanceofConfigurator.php
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\DependencyInjection\Loader\Configurator;
1313

14+
use Symfony\Component\DependencyInjection\Definition;
15+
1416
/**
1517
* @author Nicolas Grekas <p@tchwork.com>
1618
*/
@@ -28,6 +30,15 @@ class InstanceofConfigurator extends AbstractServiceConfigurator
2830
use Traits\TagTrait;
2931
use Traits\BindTrait;
3032

33+
private $path;
34+
35+
public function __construct(ServicesConfigurator $parent, Definition $definition, string $id, string $path = null)
36+
{
37+
parent::__construct($parent, $definition, $id, []);
38+
39+
$this->path = $path;
40+
}
41+
3142
/**
3243
* Defines an instanceof-conditional to be applied to following service definitions.
3344
*/

‎src/Symfony/Component/DependencyInjection/Loader/Configurator/ServiceConfigurator.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/Configurator/ServiceConfigurator.php
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,14 @@ class ServiceConfigurator extends AbstractServiceConfigurator
4545
private $container;
4646
private $instanceof;
4747
private $allowParent;
48+
private $path;
4849

49-
public function __construct(ContainerBuilder $container, array $instanceof, bool $allowParent, ServicesConfigurator $parent, Definition $definition, $id, array $defaultTags)
50+
public function __construct(ContainerBuilder $container, array $instanceof, bool $allowParent, ServicesConfigurator $parent, Definition $definition, $id, array $defaultTags, string $path = null)
5051
{
5152
$this->container = $container;
5253
$this->instanceof = $instanceof;
5354
$this->allowParent = $allowParent;
55+
$this->path = $path;
5456

5557
parent::__construct($parent, $definition, $id, $defaultTags);
5658
}

‎src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php
+5-3Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ class ServicesConfigurator extends AbstractConfigurator
2929
private $container;
3030
private $loader;
3131
private $instanceof;
32+
private $path;
3233
private $anonymousHash;
3334
private $anonymousCount;
3435

@@ -38,6 +39,7 @@ public function __construct(ContainerBuilder $container, PhpFileLoader $loader,
3839
$this->container = $container;
3940
$this->loader = $loader;
4041
$this->instanceof = &$instanceof;
42+
$this->path = $path;
4143
$this->anonymousHash = ContainerBuilder::hash($path ?: mt_rand());
4244
$this->anonymousCount = &$anonymousCount;
4345
$instanceof = [];
@@ -48,7 +50,7 @@ public function __construct(ContainerBuilder $container, PhpFileLoader $loader,
4850
*/
4951
final public function defaults(): DefaultsConfigurator
5052
{
51-
return new DefaultsConfigurator($this, $this->defaults = new Definition());
53+
return new DefaultsConfigurator($this, $this->defaults = new Definition(), $this->path);
5254
}
5355

5456
/**
@@ -58,7 +60,7 @@ final public function instanceof(string $fqcn): InstanceofConfigurator
5860
{
5961
$this->instanceof[$fqcn] = $definition = new ChildDefinition('');
6062

61-
return new InstanceofConfigurator($this, $definition, $fqcn);
63+
return new InstanceofConfigurator($this, $definition, $fqcn, $this->path);
6264
}
6365

6466
/**
@@ -90,7 +92,7 @@ final public function set(?string $id, string $class = null): ServiceConfigurato
9092
$definition->setBindings($defaults->getBindings());
9193
$definition->setChanges([]);
9294

93-
$configurator = new ServiceConfigurator($this->container, $this->instanceof, $allowParent, $this, $definition, $id, $defaults->getTags());
95+
$configurator = new ServiceConfigurator($this->container, $this->instanceof, $allowParent, $this, $definition, $id, $defaults->getTags(), $this->path);
9496

9597
return null !== $class ? $configurator->class($class) : $configurator;
9698
}

‎src/Symfony/Component/DependencyInjection/Loader/Configurator/Traits/BindTrait.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/Configurator/Traits/BindTrait.php
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@
1111

1212
namespace Symfony\Component\DependencyInjection\Loader\Configurator\Traits;
1313

14+
use Symfony\Component\DependencyInjection\Argument\BoundArgument;
1415
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
16+
use Symfony\Component\DependencyInjection\Loader\Configurator\DefaultsConfigurator;
17+
use Symfony\Component\DependencyInjection\Loader\Configurator\InstanceofConfigurator;
1518
use Symfony\Component\DependencyInjection\Reference;
1619

1720
trait BindTrait
@@ -35,7 +38,8 @@ final public function bind($nameOrFqcn, $valueOrRef)
3538
throw new InvalidArgumentException(sprintf('Invalid binding for service "%s": named arguments must start with a "$", and FQCN must map to references. Neither applies to binding "%s".', $this->id, $nameOrFqcn));
3639
}
3740
$bindings = $this->definition->getBindings();
38-
$bindings[$nameOrFqcn] = $valueOrRef;
41+
$type = $this instanceof DefaultsConfigurator ? BoundArgument::DEFAULTS_BINDING : ($this instanceof InstanceofConfigurator ? BoundArgument::INSTANCEOF_BINDING : BoundArgument::SERVICE_BINDING);
42+
$bindings[$nameOrFqcn] = new BoundArgument($valueOrRef, true, $type, $this->path ?? null);
3943
$this->definition->setBindings($bindings);
4044

4145
return $this;

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
+12-1Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,15 @@ private function getServiceDefaults(\DOMDocument $xml, $file)
175175
if (null === $defaultsNode = $xpath->query('//container:services/container:defaults')->item(0)) {
176176
return [];
177177
}
178+
179+
$bindings = [];
180+
foreach ($this->getArgumentsAsPhp($defaultsNode, 'bind', $file) as $argument => $value) {
181+
$bindings[$argument] = new BoundArgument($value, true, BoundArgument::DEFAULTS_BINDING, $file);
182+
}
183+
178184
$defaults = [
179185
'tags' => $this->getChildren($defaultsNode, 'tag'),
180-
'bind' => array_map(function ($v) { return new BoundArgument($v); }, $this->getArgumentsAsPhp($defaultsNode, 'bind', $file)),
186+
'bind' => $bindings,
181187
];
182188

183189
foreach ($defaults['tags'] as $tag) {
@@ -368,6 +374,11 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults)
368374
}
369375

370376
$bindings = $this->getArgumentsAsPhp($service, 'bind', $file);
377+
$bindingType = $this->isLoadingInstanceof ? BoundArgument::INSTANCEOF_BINDING : BoundArgument::SERVICE_BINDING;
378+
foreach ($bindings as $argument => $value) {
379+
$bindings[$argument] = new BoundArgument($value, true, $bindingType, $file);
380+
}
381+
371382
if (isset($defaults['bind'])) {
372383
// deep clone, to avoid multiple process of the same instance in the passes
373384
$bindings = array_merge(unserialize(serialize($defaults['bind'])), $bindings);

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
+9-1Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,9 @@ private function parseDefaults(array &$content, string $file): array
284284
throw new InvalidArgumentException(sprintf('Parameter "bind" in "_defaults" must be an array in %s. Check your YAML syntax.', $file));
285285
}
286286

287-
$defaults['bind'] = array_map(function ($v) { return new BoundArgument($v); }, $this->resolveServices($defaults['bind'], $file));
287+
foreach ($this->resolveServices($defaults['bind'], $file) as $argument => $value) {
288+
$defaults['bind'][$argument] = new BoundArgument($value, true, BoundArgument::DEFAULTS_BINDING, $file);
289+
}
288290
}
289291

290292
return $defaults;
@@ -534,6 +536,12 @@ private function parseDefinition($id, $service, $file, array $defaults)
534536
}
535537

536538
$bindings = array_merge($bindings, $this->resolveServices($service['bind'], $file));
539+
$bindingType = $this->isLoadingInstanceof ? BoundArgument::INSTANCEOF_BINDING : BoundArgument::SERVICE_BINDING;
540+
foreach ($bindings as $argument => $value) {
541+
if (!$value instanceof BoundArgument) {
542+
$bindings[$argument] = new BoundArgument($value, true, $bindingType, $file);
543+
}
544+
}
537545
}
538546

539547
$definition->setBindings($bindings);

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public function testProcess()
5050

5151
/**
5252
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
53-
* @expectedExceptionMessage Unused binding "$quz" in service "Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy".
53+
* @expectedExceptionMessage A binding is configured for an argument named "$quz" for service "Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy", but no corresponding argument has been found. It may be unused and should be removed, or it may have a typo.
5454
*/
5555
public function testUnusedBinding()
5656
{

‎src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,8 @@ public function process(ContainerBuilder $container)
139139
} elseif (isset($bindings[$bindingName = $type.' $'.$p->name]) || isset($bindings[$bindingName = '$'.$p->name]) || isset($bindings[$bindingName = $type])) {
140140
$binding = $bindings[$bindingName];
141141

142-
list($bindingValue, $bindingId) = $binding->getValues();
143-
$binding->setValues([$bindingValue, $bindingId, true]);
142+
list($bindingValue, $bindingId, , $bindingType, $bindingFile) = $binding->getValues();
143+
$binding->setValues([$bindingValue, $bindingId, true, $bindingType, $bindingFile]);
144144

145145
if (!$bindingValue instanceof Reference) {
146146
$args[$p->name] = new Reference('.value.'.$container->hash($bindingValue));

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.