8000 feature #36390 [DI] remove restriction and allow mixing "parent" and … · symfony/symfony@1ee1c81 · GitHub
[go: up one dir, main page]
Skip to content

Commit 1ee1c81

Browse files
fabpotfabpot
committed
feature #36390 [DI] remove restriction and allow mixing "parent" and instanceof-conditionals/defaults/bindings (nicolas-grekas)
This PR was merged into the 5.1-dev branch. Discussion ---------- [DI] remove restriction and allow mixing "parent" and instanceof-conditionals/defaults/bindings | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - We added these limitations because we didn't know how to merge all those dimensions. Yet, the code is perfectly able to handle all of them together. This PR makes `parent` definitions first-class citizens again - no need to split them in a dedicated file anymore. Commits ------- 6229253 [DI] remove restriction and allow mixing "parent" and instanceof-conditionals/defaults/bindings
2 parents 6ff7c2e + 6229253 commit 1ee1c81

16 files changed

+69
-150
lines changed

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

Lines 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

Lines 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

Lines 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

Lines 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

Lines 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

Lines 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

Lines 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

Lines 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

Lines 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

Lines 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

Lines 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

Lines 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

Lines 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

Lines 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