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 6229253

Browse filesBrowse files
nicolas-grekasfabpot
nicolas-grekas
authored and
fabpot
committed
[DI] remove restriction and allow mixing "parent" and instanceof-conditionals/defaults/bindings
1 parent 6ff7c2e commit 6229253
Copy full SHA for 6229253

16 files changed

+69
-150
lines changed

‎src/Symfony/Component/DependencyInjection/CHANGELOG.md

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/CHANGELOG.md
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ CHANGELOG
88
* added support to autowire public typed properties in php 7.4
99
* added support for defining method calls, a configurator, and property setters in `InlineServiceConfigurator`
1010
* added possibility to define abstract service arguments
11+
* allowed mixing "parent" and instanceof-conditionals/defaults/bindings
1112
* updated the signature of method `Definition::setDeprecated()` to `Definition::setDeprecation(string $package, string $version, string $message)`
1213
* updated the signature of method `Alias::setDeprecated()` to `Alias::setDeprecation(string $package, string $version, string $message)`
1314
* updated the signature of method `DeprecateTrait::deprecate()` to `DeprecateTrait::deprecation(string $package, string $version, string $message)`

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

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

1212
namespace Symfony\Component\DependencyInjection;
1313

14-
use Symfony\Component\DependencyInjection\Exception\BadMethodCallException;
1514
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
1615
use Symfony\Component\DependencyInjection\Exception\OutOfBoundsException;
1716

@@ -105,20 +104,4 @@ public function replaceArgument($index, $value)
105104

106105
return $this;
107106
}
108-
109-
/**
110-
* @internal
111-
*/
112-
public function setAutoconfigured(bool $autoconfigured): self
113-
{
114-
throw new BadMethodCallException('A ChildDefinition cannot be autoconfigured.');
115-
}
116-
117-
/**
118-
* @internal
119-
*/
120-
public function setInstanceofConditionals(array $instanceof): self
121-
{
122-
throw new BadMethodCallException('A ChildDefinition cannot have instanceof conditionals set on it.');
123-
}
124107
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php
+8-9Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,6 @@ public function process(ContainerBuilder $container)
3636
}
3737

3838
foreach ($container->getDefinitions() as $id => $definition) {
39-
if ($definition instanceof ChildDefinition) {
40-
// don't apply "instanceof" to children: it will be applied to their parent
41-
continue;
42-
}
4339
$container->setDefinition($id, $this->processDefinition($container, $id, $definition));
4440
}
4541
}
@@ -59,11 +55,12 @@ private function processDefinition(ContainerBuilder $container, string $id, Defi
5955
$conditionals = $this->mergeConditionals($autoconfiguredInstanceof, $instanceofConditionals, $container);
6056

6157
$definition->setInstanceofConditionals([]);
62-
$parent = $shared = null;
58+
$shared = null;
6359
$instanceofTags = [];
6460
$instanceofCalls = [];
6561
$instanceofBindings = [];
6662
$reflectionClass = null;
63+
$parent = $definition instanceof ChildDefinition ? $definition->getParent() : null;
6764

6865
foreach ($conditionals as $interface => $instanceofDefs) {
6966
if ($interface !== $class && !(null === $reflectionClass ? $reflectionClass = ($container->getReflectionClass($class, false) ?: false) : $reflectionClass)) {
@@ -100,12 +97,14 @@ private function processDefinition(ContainerBuilder $container, string $id, Defi
10097
if ($parent) {
10198
$bindings = $definition->getBindings();
10299
$abstract = $container->setDefinition('.abstract.instanceof.'.$id, $definition);
103-
104-
// cast Definition to ChildDefinition
105100
$definition->setBindings([]);
106101
$definition = serialize($definition);
107-
$definition = substr_replace($definition, '53', 2, 2);
108-
$definition = substr_replace($definition, 'Child', 44, 0);
102+
103+
if (Definition::class === \get_class($abstract)) {
104+
// cast Definition to ChildDefinition
105+
$definition = substr_replace($definition, '53', 2, 2);
106+
$definition = substr_replace($definition, 'Child', 44, 0);
107+
}
109108
/** @var ChildDefinition $definition */
110109
$definition = unserialize($definition);
111110
$definition->setParent($parent);

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

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

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

14-
use Symfony\Component\DependencyInjection\ChildDefinition;
1514
use Symfony\Component\DependencyInjection\ContainerBuilder;
1615
use Symfony\Component\DependencyInjection\Definition;
1716

@@ -62,11 +61,6 @@ public function __destruct()
6261
parent::__destruct();
6362

6463
$this->container->removeBindings($this->id);
65-
66-
if (!$this->definition instanceof ChildDefinition) {
67-
$this->container->setDefinition($this->id, $this->definition->setInstanceofConditionals($this->instanceof));
68-
} else {
69-
$this->container->setDefinition($this->id, $this->definition);
70-
}
64+
$this->container->setDefinition($this->id, $this->definition->setInstanceofConditionals($this->instanceof));
7165
}
7266
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php
+3-8Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,6 @@ final public function instanceof(string $fqcn): InstanceofConfigurator
7272
final public function set(?string $id, string $class = null): ServiceConfigurator
7373
{
7474
$defaults = $this->defaults;
75-
$allowParent = !$defaults->getChanges() && empty($this->instanceof);
76-
7775
$definition = new Definition();
7876

7977
if (null === $id) {
@@ -92,7 +90,7 @@ final public function set(?string $id, string $class = null): ServiceConfigurato
9290
$definition->setBindings($defaults->getBindings());
9391
$definition->setChanges([]);
9492

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

9795
return null !== $class ? $configurator->class($class) : $configurator;
9896
}
@@ -114,9 +112,7 @@ final public function alias(string $id, string $referencedId): AliasConfigurator
114112
*/
115113
final public function load(string $namespace, string $resource): PrototypeConfigurator
116114
{
117-
$allowParent = !$this->defaults->getChanges() && empty($this->instanceof);
118-
119-
return new PrototypeConfigurator($this, $this->loader, $this->defaults, $namespace, $resource, $allowParent);
115+
return new PrototypeConfigurator($this, $this->loader, $this->defaults, $namespace, $resource, true);
120116
}
121117

122118
/**
@@ -126,10 +122,9 @@ final public function load(string $namespace, string $resource): PrototypeConfig
126122
*/
127123
final public function get(string $id): ServiceConfigurator
128124
{
129-
$allowParent = !$this->defaults->getChanges() && empty($this->instanceof);
130125
$definition = $this->container->getDefinition($id);
131126

132-
return new ServiceConfigurator($this->container, $definition->getInstanceofConditionals(), $allowParent, $this, $definition, $id, []);
127+
return new ServiceConfigurator($this->container, $definition->getInstanceofConditionals(), true, $this, $definition, $id, []);
133128
}
134129

135130
/**

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

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

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

14-
use Symfony\Component\DependencyInjection\ChildDefinition;
1514
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
1615

1716
trait AutoconfigureTrait
@@ -25,9 +24,6 @@ trait AutoconfigureTrait
2524
*/
2625
final public function autoconfigure(bool $autoconfigured = true): self
2726
{
28-
if ($autoconfigured && $this->definition instanceof ChildDefinition) {
29-
throw new InvalidArgumentException(sprintf('The service "%s" cannot have a "parent" and also have "autoconfigure". Try disabling autoconfiguration for the service.', $this->id));
30-
}
3127
$this->definition->setAutoconfigured($autoconfigured);
3228

3329
return $this;

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/Configurator/Traits/ParentTrait.php
-4Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,6 @@ final public function parent(string $parent): self
3131

3232
if ($this->definition instanceof ChildDefinition) {
3333
$this->definition->setParent($parent);
34-
} elseif ($this->definition->isAutoconfigured()) {
35-
throw new InvalidArgumentException(sprintf('The service "%s" cannot have a "parent" and also have "autoconfigure". Try disabling autoconfiguration for the service.', $this->id));
36-
} elseif ($this->definition->getBindings()) {
37-
throw new InvalidArgumentException(sprintf('The service "%s" cannot have a "parent" and also "bind" arguments.', $this->id));
3834
} else {
3935
// cast Definition to ChildDefinition
4036
$definition = serialize($this->definition);

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/FileLoader.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ protected function setDefinition($id, Definition $definition)
147147
}
148148
$this->instanceof[$id] = $definition;
149149
} else {
150-
$this->container->setDefinition($id, $definition instanceof ChildDefinition ? $definition : $definition->setInstanceofConditionals($this->instanceof));
150+
$this->container->setDefinition($id, $definition->setInstanceofConditionals($this->instanceof));
151151
}
152152
}
153153

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
+12-38Lines changed: 12 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -226,45 +226,23 @@ private function parseDefinition(\DOMElement $service, string $file, array $defa
226226
if ($this->isLoadingInstanceof) {
227227
$definition = new ChildDefinition('');
228228
} elseif ($parent = $service->getAttribute('parent')) {
229-
if (!empty($this->instanceof)) {
230-
throw new InvalidArgumentException(sprintf('The service "%s" cannot use the "parent" option in the same file where "instanceof" configuration is defined as using both is not supported. Move your child definitions to a separate file.', $service->getAttribute('id')));
231-
}
232-
233-
foreach ($defaults as $k => $v) {
234-
if ('tags' === $k) {
235-
// since tags are never inherited from parents, there is no confusion
236-
// thus we can safely add them as defaults to ChildDefinition
237-
continue;
238-
}
239-
if ('bind' === $k) {
240-
if ($defaults['bind']) {
241-
throw new InvalidArgumentException(sprintf('Bound values on service "%s" cannot be inherited from "defaults" when a "parent" is set. Move your child definitions to a separate file.', $service->getAttribute('id')));
242-
}
243-
244-
continue;
245-
}
246-
if (!$service->hasAttribute($k)) {
247-
throw new InvalidArgumentException(sprintf('Attribute "%s" on service "%s" cannot be inherited from "defaults" when a "parent" is set. Move your child definitions to a separate file or define this attribute explicitly.', $k, $service->getAttribute('id')));
248-
}
249-
}
250-
251229
$definition = new ChildDefinition($parent);
252230
} else {
253231
$definition = new Definition();
232+
}
254233

255-
if (isset($defaults['public'])) {
256-
$definition->setPublic($defaults['public']);
257-
}
258-
if (isset($defaults['autowire'])) {
259-
$definition->setAutowired($defaults['autowire']);
260-
}
261-
if (isset($defaults['autoconfigure'])) {
262-
$definition->setAutoconfigured($defaults['autoconfigure']);
263-
}
264-
265-
$definition->setChanges([]);
234+
if (isset($defaults['public'])) {
235+
$definition->setPublic($defaults['public']);
236+
}
237+
if (isset($defaults['autowire'])) {
238+
$definition->setAutowired($defaults['autowire']);
239+
}
240+
if (isset($defaults['autoconfigure'])) {
241+
$definition->setAutoconfigured($defaults['autoconfigure']);
266242
}
267243

244+
$definition->setChanges([]);
245+
268246
foreach (['class', 'public', 'shared', 'synthetic', 'abstract'] as $key) {
269247
if ($value = $service->getAttribute($key)) {
270248
$method = 'set'.$key;
@@ -284,11 +262,7 @@ private function parseDefinition(\DOMElement $service, string $file, array $defa
284262
}
285263

286264
if ($value = $service->getAttribute('autoconfigure')) {
287-
if (!$definition instanceof ChildDefinition) {
288-
$definition->setAutoconfigured(XmlUtils::phpize($value));
289-
} elseif ($value = XmlUtils::phpize($value)) {
290-
throw new InvalidArgumentException(sprintf('The service "%s" cannot have a "parent" and also have "autoconfigure". Try setting autoconfigure="false" for the service.', $service->getAttribute('id')));
291-
}
265+
$definition->setAutoconfigured(XmlUtils::phpize($value));
292266
}
293267

294268
if ($files = $this->getChildren($service, 'file')) {

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
+12-34Lines changed: 12 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -378,45 +378,27 @@ private function parseDefinition(string $id, $service, string $file, array $defa
378378
if ($this->isLoadingInstanceof) {
379379
$definition = new ChildDefinition('');
380380
} elseif (isset($service['parent'])) {
381-
if (!empty($this->instanceof)) {
382-
throw new InvalidArgumentException(sprintf('The service "%s" cannot use the "parent" option in the same file where "_instanceof" configuration is defined as using both is not supported. Move your child definitions to a separate file.', $id));
383-
}
384-
385-
foreach ($defaults as $k => $v) {
386-
if ('tags' === $k) {
387-
// since tags are never inherited from parents, there is no confusion
388-
// thus we can safely add them as defaults to ChildDefinition
389-
continue;
390-
}
391-
if ('bind' === $k) {
392-
throw new InvalidArgumentException(sprintf('Attribute "bind" on service "%s" cannot be inherited from "_defaults" when a "parent" is set. Move your child definitions to a separate file.', $id));
393-
}
394-
if (!isset($service[$k])) {
395-
throw new InvalidArgumentException(sprintf('Attribute "%s" on service "%s" cannot be inherited from "_defaults" when a "parent" is set. Move your child definitions to a separate file or define this attribute explicitly.', $k, $id));
396-
}
397-
}
398-
399381
if ('' !== $service['parent'] && '@' === $service['parent'][0]) {
400382
throw new InvalidArgumentException(sprintf('The value of the "parent" option for the "%s" service must be the id of the service without the "@" prefix (replace "%s" with "%s").', $id, $service['parent'], substr($service['parent'], 1)));
401383
}
402384

403385
$definition = new ChildDefinition($service['parent']);
404386
} else {
405387
$definition = new Definition();
388+
}
406389

407-
if (isset($defaults['public'])) {
408-
$definition->setPublic($defaults['public']);
409-
}
410-
if (isset($defaults['autowire'])) {
411-
$definition->setAutowired($defaults['autowire']);
412-
}
413-
if (isset($defaults['autoconfigure'])) {
414-
$definition->setAutoconfigured($defaults['autoconfigure']);
415-
}
416-
417-
$definition->setChanges([]);
390+
if (isset($defaults['public'])) {
391+
$definition->setPublic($defaults['public']);
392+
}
393+
if (isset($defaults['autowire'])) {
394+
$definition->setAutowired($defaults['autowire']);
395+
}
396+
if (isset($defaults['autoconfigure'])) {
397+
$definition->setAutoconfigured($defaults['autoconfigure']);
418398
}
419399

400+
$definition->setChanges([]);
401+
420402
if (isset($service['class'])) {
421403
$definition->setClass($service['class']);
422404
}
@@ -612,11 +594,7 @@ private function parseDefinition(string $id, $service, string $file, array $defa
612594
}
613595

614596
if (isset($service['autoconfigure'])) {
615-
if (!$definition instanceof ChildDefinition) {
616-
$definition->setAutoconfigured($service['autoconfigure']);
617-
} elseif ($service['autoconfigure']) {
618-
throw new InvalidArgumentException(sprintf('The service "%s" cannot have a "parent" and also have "autoconfigure". Try setting "autoconfigure: false" for the service.', $id));
619-
}
597+
$definition->setAutoconfigured($service['autoconfigure']);
620598
}
621599

622600
if (\array_key_exists('namespace', $service) && !\array_key_exists('resource', $service)) {

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/ChildDefinitionTest.php
+8-5Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,17 +127,20 @@ public function testGetArgumentShouldCheckBounds()
127127
$def->getArgument(1);
128128
}
129129

130-
public function testCannotCallSetAutoconfigured()
130+
public function testAutoconfigured()
131131
{
132-
$this->expectException('Symfony\Component\DependencyInjection\Exception\BadMethodCallException');
133132
$def = new ChildDefinition('foo');
134133
$def->setAutoconfigured(true);
134+
135+
$this->assertTrue($def->isAutoconfigured());
135136
}
136137

137-
public function testCannotCallSetInstanceofConditionals()
138+
public function testInstanceofConditionals()
138139
{
139-
$this->expectException('Symfony\Component\DependencyInjection\Exception\BadMethodCallException');
140+
$conditionals = ['Foo' => new ChildDefinition('')];
140141
$def = new ChildDefinition('foo');
141-
$def->setInstanceofConditionals(['Foo' => new ChildDefinition('')]);
142+
$def->setInstanceofConditionals($conditionals);
143+
144+
$this->assertSame($conditionals, $def->getInstanceofConditionals());
142145
}
143146
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_instanceof_with_parent.xml

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_instanceof_with_parent.xml
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
33
<services>
4-
<instanceof id="FooInterface" autowire="true" />
4+
<instanceof id="stdClass" autowire="true" />
55

66
<service id="parent_service" class="stdClass" public="true"/>
77
<service id="child_service" class="stdClass" parent="parent_service" />

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_instanceof_with_parent.yml
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
services:
22
_instanceof:
3-
FooInterface:
3+
stdClass:
44
autowire: true
55

66
parent_service:

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Loader/PhpFileLoaderTest.php
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,15 +79,15 @@ public function provideConfig()
7979
yield ['lazy_fqcn'];
8080
}
8181

82-
public function testAutoConfigureAndChildDefinitionNotAllowed()
82+
public function testAutoConfigureAndChildDefinition()
8383
{
84-
$this->expectException('Symfony\Component\DependencyInjection\Exception\InvalidArgumentException');
85-
$this->expectExceptionMessage('The service "child_service" cannot have a "parent" and also have "autoconfigure". Try disabling autoconfiguration for the service.');
8684
$fixtures = realpath(__DIR__.'/../Fixtures');
8785
$container = new ContainerBuilder();
8886
$loader = new PhpFileLoader($container, new FileLocator());
8987
$loader->load($fixtures.'/config/services_autoconfigure_with_parent.php');
9088
$container->compile();
89+
90+
$this->assertTrue($container->getDefinition('child_service')->isAutoconfigured());
9191
}
9292

9393
public function testFactoryShortNotationNotAllowed()

0 commit comments

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