8000 Included reviewer comments · symfony/symfony@627c437 · GitHub
[go: up one dir, main page]
Skip to content

Commit 627c437

Browse files
przemyslaw-boguszprzemyslaw-bogusz
committed
Included reviewer comments
1 parent 500b349 commit 627c437

File tree

11 files changed

+46
-36
lines changed

11 files changed

+46
-36
lines changed

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

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,9 @@
1616
*/
1717
final class BoundArgument implements ArgumentInterface
1818
{
19-
const SERVICE_BIND = 0;
20-
const DEFAULTS_BIND = 1;
21-
const INSTANCEOF_BIND = 2;
22-
23-
public static $messages = [
24-
1 => 'under "_defaults"',
25-
2 => 'under "_instanceof"',
26-
];
19+
const SERVICE_BINDING = 0;
20+
const DEFAULTS_BINDING = 1;
21+
const INSTANCEOF_BINDING = 2;
2722

2823
private static $sequence = 0;
2924

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,7 @@ public function process(ContainerBuilder $container)
3838
parent::process($container);
3939

4040
foreach ($this->unusedBindings as list($key, $serviceId, $bindingType, $file)) {
41-
$message = 'You have a "bind" configured for an argument ';
42-
43-
$argumentType = $argumentName = null;
41+
$argumentType = $argumentName = $message = null;
4442

4543
if (false !== strpos($key, ' ')) {
4644
list($argumentType, $argumentName) = explode(' ', $key, 2);
@@ -58,8 +56,10 @@ public function process(ContainerBuilder $container)
5856
$message .= sprintf('named "%s" ', $argumentName);
5957
}
6058

61-
if ($bindingType) {
62-
$message .= BoundArgument::$messages[$bindingType];
59+
if (BoundArgument::DEFAULTS_BINDING === $bindingType) {
60+
$message .= 'under "_defaults"';
61+
} elseif (BoundArgument::INSTANCEOF_BINDING === $bindingType) {
62+
$message .= 'under "_instanceof"';
6363
} else {
6464
$message .= sprintf('for service "%s"', $serviceId);
6565
}
@@ -68,7 +68,7 @@ public function process(ContainerBuilder $container)
6868
$message .= sprintf(' in file "%s"', $file);
6969
}
7070

71-
$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.';
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);
7272

7373
if ($this->errorMessages) {
7474
$message .= sprintf("\nCould be related to%s:", 1 < \count($this->errorMessages) ? ' one of' : '');

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

Lines 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‎

Lines 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‎

Lines 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‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public function __construct(ContainerBuilder $container, PhpFileLoader $loader,
5050
*/
5151
final public function defaults(): DefaultsConfigurator
5252
{
53-
return new DefaultsConfigurator($this, $this->defaults = new Definition());
53+
return new DefaultsConfigurator($this, $this->defaults = new Definition(), $this->path);
5454
}
5555

5656
/**
@@ -60,7 +60,7 @@ final public function instanceof(string $fqcn): InstanceofConfigurator
6060
{
6161
$this->instanceof[$fqcn] = $definition = new ChildDefinition('');
6262

63-
return new InstanceofConfigurator($this, $definition, $fqcn);
63+
return new InstanceofConfigurator($this, $definition, $fqcn, $this->path);
6464
}
6565

6666
/**
@@ -92,7 +92,7 @@ final public function set(?string $id, string $class = null): ServiceConfigurato
9292
$definition->setBindings($defaults->getBindings());
9393
$definition->setChanges([]);
9494

95-
$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);
9696

9797
return null !== $class ? $configurator->class($class) : $configurator;
9898
}
@@ -139,9 +139,4 @@ final public function __invoke(string $id, string $class = null): ServiceConfigu
139139
{
140140
return $this->set($id, $class);
141141
}
142-
143-
final public function getPath()
144-
{
145-
return $this->path;
146-
}
147142
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,8 @@ final public function bind($nameOrFqcn, $valueOrRef)
3939
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));
4040
}
4141
$bindings = $this->definition->getBindings();
42-
$type = $this instanceof DefaultsConfigurator ? BoundArgument::DEFAULTS_BIND : ($this instanceof InstanceofConfigurator ? BoundArgument::INSTANCEOF_BIND : BoundArgument::SERVICE_BIND);
43-
$file = $this->parent instanceof ServicesConfigurator ? $this->parent->getPath() : null;
44-
$bindings[$nameOrFqcn] = new BoundArgument($valueOrRef, true, $type, $file);
42+
$type = $this instanceof DefaultsConfigurator ? BoundArgument::DEFAULTS_BINDING : ($this instanceof InstanceofConfigurator ? BoundArgument::INSTANCEOF_BINDING : BoundArgument::SERVICE_BINDING);
43+
$bindings[$nameOrFqcn] = new BoundArgument($valueOrRef, true, $type, $this->path ?? null);
4544
$this->definition->setBindings($bindings);
4645

4746
return $this;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ private function getServiceDefaults(\DOMDocument $xml, $file)
178178

179179
$bindings = [];
180180
foreach ($this->getArgumentsAsPhp($defaultsNode, 'bind', $file) as $argument => $value) {
181-
$bindings[$argument] = new BoundArgument($value, true, BoundArgument::DEFAULTS_BIND, $file);
181+
$bindings[$argument] = new BoundArgument($value, true, BoundArgument::DEFAULTS_BINDING, $file);
182182
}
183183

184184
$defaults = [
@@ -374,7 +374,7 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults)
374374
}
375375

376376
$bindings = $this->getArgumentsAsPhp($service, 'bind', $file);
377-
$bindingType = $this->isLoadingInstanceof ? BoundArgument::INSTANCEOF_BIND : BoundArgument::SERVICE_BIND;
377+
$bindingType = $this->isLoadingInstanceof ? BoundArgument::INSTANCEOF_BINDING : BoundArgument::SERVICE_BINDING;
378378
foreach ($bindings as $argument => $value) {
379379
$bindings[$argument] = new BoundArgument($value, true, $bindingType, $file);
380380
}

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ private function parseDefaults(array &$content, string $file): array
285285
}
286286

287287
foreach ($this->resolveServices($defaults['bind'], $file) as $argument => $value) {
288-
$defaults['bind'][$argument] = new BoundArgument($value, true, BoundArgument::DEFAULTS_BIND, $file);
288+
$defaults['bind'][$argument] = new BoundArgument($value, true, BoundArgument::DEFAULTS_BINDING, $file);
289289
}
290290
}
291291

@@ -534,12 +534,11 @@ private function parseDefinition($id, $service, $file, array $defaults)
534534
}
535535

536536
$bindings = array_merge($bindings, $this->resolveServices($service['bind'], $file));
537-
$bindingType = $this->isLoadingInstanceof ? BoundArgument::INSTANCEOF_BIND : BoundArgument::SERVICE_BIND;
537+
$bindingType = $this->isLoadingInstanceof ? BoundArgument::INSTANCEOF_BINDING : BoundArgument::SERVICE_BINDING;
538538
foreach ($bindings as $argument => $value) {
539-
if ($value instanceof BoundArgument) {
540-
continue;
539+
if (!$value instanceof BoundArgument) {
540+
$bindings[$argument] = new BoundArgument($value, true, $bindingType, $file);
541541
}
542-
$bindings[$argument] = new BoundArgument($value, true, $bindingType, $file);
543542
}
544543
}
545544

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function testProcess()
4949

5050
/**
5151
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
52-
* @expectedExceptionMessage You have a "bind" configured for an argument named "$quz" for service "Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy". 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.
52+
* @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.
5353
*/
5454
public function testUnusedBinding()
5555
{

0 commit comments

Comments
 (0)
0