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

[DI] remove restriction and allow mixing "parent" and instanceof-conditionals/defaults/bindings #36390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 12, 2020
Merged

[DI] remove restriction and allow mixing "parent" and instanceof-conditionals/defaults/bindings #36390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
[DI] remove restriction and allow mixing "parent" and instanceof-cond… 
…itionals/defaults/bindings
  • Loading branch information
@nicolas-grekas @fabpot
nicolas-grekas authored and fabpot committed Apr 12, 2020
commit 622925300ff29e9867dfd95a8eec8a07aa5a9e91
1 change: 1 addition & 0 deletions 1 src/Symfony/Component/DependencyInjection/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ CHANGELOG
* added support to autowire public typed properties in php 7.4
* added support for defining method calls, a configurator, and property setters in `InlineServiceConfigurator`
* added possibility to define abstract service arguments
* allowed mixing "parent" and instanceof-conditionals/defaults/bindings
* updated the signature of method `Definition::setDeprecated()` to `Definition::setDeprecation(string $package, string $version, string $message)`
* updated the signature of method `Alias::setDeprecated()` to `Alias::setDeprecation(string $package, string $version, string $message)`
* updated the signature of method `DeprecateTrait::deprecate()` to `DeprecateTrait::deprecation(string $package, string $version, string $message)`
Expand Down
17 changes: 0 additions & 17 deletions 17 src/Symfony/Component/DependencyInjection/ChildDefinition.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

namespace Symfony\Component\DependencyInjection;

use Symfony\Component\DependencyInjection\Exception\BadMethodCallException;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Exception\OutOfBoundsException;

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

return $this;
}

/**
* @internal
*/
public function setAutoconfigured(bool $autoconfigured): self
{
throw new BadMethodCallException('A ChildDefinition cannot be autoconfigured.');
}

/**
* @internal
*/
public function setInstanceofConditionals(array $instanceof): self
{
throw new BadMethodCallException('A ChildDefinition cannot have instanceof conditionals set on it.');
}
}
17 changes: 8 additions & 9 deletions 17 src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ public function process(ContainerBuilder $container)
}

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

$definition->setInstanceofConditionals([]);
$parent = $shared = null;
$shared = null;
$instanceofTags = [];
$instanceofCalls = [];
$instanceofBindings = [];
$reflectionClass = null;
$parent = $definition instanceof ChildDefinition ? $definition->getParent() : null;

foreach ($conditionals as $interface => $instanceofDefs) {
if ($interface !== $class && !(null === $reflectionClass ? $reflectionClass = ($container->getReflectionClass($class, false) ?: false) : $reflectionClass)) {
Expand Down Expand Up @@ -100,12 +97,14 @@ private function processDefinition(ContainerBuilder $container, string $id, Defi
if ($parent) {
$bindings = $definition->getBindings();
$abstract = $container->setDefinition('.abstract.instanceof.'.$id, $definition);

// cast Definition to ChildDefinition
$definition->setBindings([]);
$definition = serialize($definition);
$definition = substr_replace($definition, '53', 2, 2);
$definition = substr_replace($definition, 'Child', 44, 0);

if (Definition::class === \get_class($abstract)) {
// cast Definition to ChildDefinition
$definition = substr_replace($definition, '53', 2, 2);
$definition = substr_replace($definition, 'Child', 44, 0);
}
/** @var ChildDefinition $definition */
$definition = unserialize($definition);
$definition->setParent($parent);
Expand Down
8 changes: 1 addition & 7 deletions 8 src/Symfony/Component/DependencyInjection/Loader/Configurator/ServiceConfigurator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

namespace Symfony\Component\DependencyInjection\Loader\Configurator;

use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;

Expand Down Expand Up @@ -62,11 +61,6 @@ public function __destruct()
parent::__destruct();

$this->container->removeBindings($this->id);

if (!$this->definition instanceof ChildDefinition) {
$this->container->setDefinition($this->id, $this->definition->setInstanceofConditionals($this->instanceof));
} else {
$this->container->setDefinition($this->id, $this->definition);
}
$this->container->setDefinition($this->id, $this->definition->setInstanceofConditionals($this->instanceof));
}
}
11 changes: 3 additions & 8 deletions 11 src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ final public function instanceof(string $fqcn): InstanceofConfigurator
final public function set(?string $id, string $class = null): ServiceConfigurator
{
$defaults = $this->defaults;
$allowParent = !$defaults->getChanges() && empty($this->instanceof);

$definition = new Definition();

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

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

return null !== $class ? $configurator->class($class) : $configurator;
}
Expand All @@ -114,9 +112,7 @@ final public function alias(string $id, string $referencedId): AliasConfigurator
*/
final public function load(string $namespace, string $resource): PrototypeConfigurator
{
$allowParent = !$this->defaults->getChanges() && empty($this->instanceof);

return new PrototypeConfigurator($this, $this->loader, $this->defaults, $namespace, $resource, $allowParent);
return new PrototypeConfigurator($this, $this->loader, $this->defaults, $namespace, $resource, true);
}

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

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

/**
Expand Down
4 changes: 0 additions & 4 deletions 4 src/Symfony/Component/DependencyInjection/Loader/Configurator/Traits/AutoconfigureTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

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

use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;

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

return $this;
Expand Down
4 changes: 0 additions & 4 deletions 4 src/Symfony/Component/DependencyInjection/Loader/Configurator/Traits/ParentTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ final public function parent(string $parent): self

if ($this->definition instanceof ChildDefinition) {
$this->definition->setParent($parent);
} elseif ($this->definition->isAutoconfigured()) {
throw new InvalidArgumentException(sprintf('The service "%s" cannot have a "parent" and also have "autoconfigure". Try disabling autoconfiguration for the service.', $this->id));
} elseif ($this->definition->getBindings()) {
throw new InvalidArgumentException(sprintf('The service "%s" cannot have a "parent" and also "bind" arguments.', $this->id));
} else {
// cast Definition to ChildDefinition
$definition = serialize($this->definition);
Expand Down
2 changes: 1 addition & 1 deletion 2 src/Symfony/Component/DependencyInjection/Loader/FileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ protected function setDefinition($id, Definition $definition)
}
$this->instanceof[$id] = $definition;
} else {
$this->container->setDefinition($id, $definition instanceof ChildDefinition ? $definition : $definition->setInstanceofConditionals($this->instanceof));
$this->container->setDefinition($id, $definition->setInstanceofConditionals($this->instanceof));
}
}

Expand Down
50 changes: 12 additions & 38 deletions 50 src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,45 +226,23 @@ private function parseDefinition(\DOMElement $service, string $file, array $defa
if ($this->isLoadingInstanceof) {
$definition = new ChildDefinition('');
} elseif ($parent = $service->getAttribute('parent')) {
if (!empty($this->instanceof)) {
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')));
}

foreach ($defaults as $k => $v) {
if ('tags' === $k) {
// since tags are never inherited from parents, there is no confusion
// thus we can safely add them as defaults to ChildDefinition
continue;
}
if ('bind' === $k) {
if ($defaults['bind']) {
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')));
}

continue;
}
if (!$service->hasAttribute($k)) {
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')));
}
}

$definition = new ChildDefinition($parent);
} else {
$definition = new Definition();
}

if (isset($defaults['public'])) {
$definition->setPublic($defaults['public']);
}
if (isset($defaults['autowire'])) {
$definition->setAutowired($defaults['autowire']);
}
if (isset($defaults['autoconfigure'])) {
$definition->setAutoconfigured($defaults['autoconfigure']);
}

$definition->setChanges([]);
if (isset($defaults['public'])) {
$definition->setPublic($defaults['public']);
}
if (isset($defaults['autowire'])) {
$definition->setAutowired($defaults['autowire']);
}
if (isset($defaults['autoconfigure'])) {
$definition->setAutoconfigured($defaults['autoconfigure']);
}

$definition->setChanges([]);

foreach (['class', 'public', 'shared', 'synthetic', 'abstract'] as $key) {
if ($value = $service->getAttribute($key)) {
$method = 'set'.$key;
Expand All @@ -284,11 +262,7 @@ private function parseDefinition(\DOMElement $service, string $file, array $defa
}

if ($value = $service->getAttribute('autoconfigure')) {
if (!$definition instanceof ChildDefinition) {
$definition->setAutoconfigured(XmlUtils::phpize($value));
} elseif ($value = XmlUtils::phpize($value)) {
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')));
}
$definition->setAutoconfigured(XmlUtils::phpize($value));
}

if ($files = $this->getChildren($service, 'file')) {
Expand Down
46 changes: 12 additions & 34 deletions 46 src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,45 +378,27 @@ private function parseDefinition(string $id, $service, string $file, array $defa
if ($this->isLoadingInstanceof) {
$definition = new ChildDefinition('');
} elseif (isset($service['parent'])) {
if (!empty($this->instanceof)) {
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));
}

foreach ($defaults as $k => $v) {
if ('tags' === $k) {
// since tags are never inherited from parents, there is no confusion
// thus we can safely add them as defaults to ChildDefinition
continue;
}
if ('bind' === $k) {
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));
}
if (!isset($service[$k])) {
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));
}
}

if ('' !== $service['parent'] && '@' === $service['parent'][0]) {
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)));
}

$definition = new ChildDefinition($service['parent']);
} else {
$definition = new Definition();
}

if (isset($defaults['public'])) {
$definition->setPublic($defaults['public']);
}
if (isset($defaults['autowire'])) {
$definition->setAutowired($defaults['autowire']);
}
if (isset($defaults['autoconfigure'])) {
$definition->setAutoconfigured($defaults['autoconfigure']);
}

$definition->setChanges([]);
if (isset($defaults['public'])) {
$definition->setPublic($defaults['public']);
}
if (isset($defaults['autowire'])) {
$definition->setAutowired($defaults['autowire']);
}
if (isset($defaults['autoconfigure'])) {
$definition->setAutoconfigured($defaults['autoconfigure']);
}

$definition->setChanges([]);

if (isset($service['class'])) {
$definition->setClass($service['class']);
}
Expand Down Expand Up @@ -612,11 +594,7 @@ private function parseDefinition(string $id, $service, string $file, array $defa
}

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

if (\array_key_exists('namespace', $service) && !\array_key_exists('resource', $service)) {
Expand Down
13 changes: 8 additions & 5 deletions 13 src/Symfony/Component/DependencyInjection/Tests/ChildDefinitionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,17 +127,20 @@ public function testGetArgumentShouldCheckBounds()
$def->getArgument(1);
}

public function testCannotCallSetAutoconfigured()
public function testAutoconfigured()
{
$this->expectException('Symfony\Component\DependencyInjection\Exception\BadMethodCallException');
$def = new ChildDefinition('foo');
$def->setAutoconfigured(true);

$this->assertTrue($def->isAutoconfigured());
}

public function testCannotCallSetInstanceofConditionals()
public function testInstanceofConditionals()
{
$this->expectException('Symfony\Component\DependencyInjection\Exception\BadMethodCallException');
$conditionals = ['Foo' => new ChildDefinition('')];
$def = new ChildDefinition('foo');
$def->setInstanceofConditionals(['Foo' => new ChildDefinition('')]);
$def->setInstanceofConditionals($conditionals);

$this->assertSame($conditionals, $def->getInstanceofConditionals());
}
}
2 changes: 1 addition & 1 deletion 2 ...fony/Component/DependencyInjection/Tests/Fixtures/xml/services_instanceof_with_parent.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<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">
<services>
<instanceof id="FooInterface" autowire="true" />
<instanceof id="stdClass" autowire="true" />

<service id="parent_service" class="stdClass" public="true"/>
<service id="child_service" class="stdClass" parent="parent_service" />
Expand Down
2 changes: 1 addition & 1 deletion 2 ...ony/Component/DependencyInjection/Tests/Fixtures/yaml/services_instanceof_with_parent.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
services:
_instanceof:
FooInterface:
stdClass:
autowire: true

parent_service:
Expand Down
6 changes: 3 additions & 3 deletions 6 src/Symfony/Component/DependencyInjection/Tests/Loader/PhpFileLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,15 @@ public function provideConfig()
yield ['lazy_fqcn'];
}

public function testAutoConfigureAndChildDefinitionNotAllowed()
public function testAutoConfigureAndChildDefinition()
{
$this->expectException('Symfony\Component\DependencyInjection\Exception\InvalidArgumentException');
$this->expectExceptionMessage('The service "child_service" cannot have a "parent" and also have "autoconfigure". Try disabling autoconfiguration for the service.');
$fixtures = realpath(__DIR__.'/../Fixtures');
$container = new ContainerBuilder();
$loader = new PhpFileLoader($container, new FileLocator());
$loader->load($fixtures.'/config/services_autoconfigure_with_parent.php');
$container->compile();

$this->assertTrue($container->getDefinition('child_service')->isAutoconfigured());
}

public function testFactoryShortNotationNotAllowed()
Expand Down
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.