10000 Making tags under _defaults always apply by weaverryan · Pull Request #22530 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

Making tags under _defaults always apply #22530

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.

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
May 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions src/Symfony/Component/DependencyInjection/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ CHANGELOG
* added "iterator" argument type for lazy iteration over a set of values and services
* added "closure-proxy" argument type for turning services' methods into lazy callables
* added file-wide configurable defaults for service attributes "public", "tags",
"autowire" and "inherit-tags"
* added "inherit-tags" service attribute to control tags' inheritance from parent context
"autowire" and "autoconfigure"
* made the "class" attribute optional, using the "id" as fallback
* using the `PhpDumper` with an uncompiled `ContainerBuilder` is deprecated and
will not be supported anymore in 4.0
Expand Down
25 changes: 0 additions & 25 deletions src/Symfony/Component/DependencyInjection/ChildDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
class ChildDefinition extends Definition
{
private $parent;
private $inheritTags = false;

/**
* @param string $parent The id of Definition instance to decorate
Expand Down Expand Up @@ -57,30 +56,6 @@ public function setParent($parent)
return $this;
}

/**
* Sets whether tags should be inherited from the parent or not.
*
* @param bool $boolean
*
* @return $this
*/
public function setInheritTags($boolean)
{
$this->inheritTags = (bool) $boolean;

return $this;
}

/**
* Returns whether tags should be inherited from the parent or not.
*
* @return bool
*/
public function getInheritTags()
{
return $this->inheritTags;
}

/**
* Gets an argument to pass to the service constructor/factory method.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ public function __construct()
100 => array(
$resolveClassPass = new ResolveClassPass(),
new ResolveInstanceofConditionalsPass(),
new ResolveTagsInheritancePass(),
),
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ private function processDefinition(ContainerBuilder $container, $id, Definition
foreach ($instanceofDefs as $key => $instanceofDef) {
/** @var ChildDefinition $instanceofDef */
$instanceofDef = clone $instanceofDef;
$instanceofDef->setAbstract(true)->setInheritTags(false)->setParent($parent ?: 'abstract.instanceof.'.$id);
$instanceofDef->setAbstract(true)->setParent($parent ?: 'abstract.instanceof.'.$id);
$parent = 'instanceof.'.$interface.'.'.$key.'.'.$id;
$container->setDefinition($parent, $instanceofDef);
$instanceofTags[] = $instanceofDef->getTags();
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,6 @@ private function getServiceDefaults(\DOMDocument $xml, $file)
if ($defaultsNode->hasAttribute('public')) {
$defaults['public'] = XmlUtils::phpize($defaultsNode->getAttribute('public'));
}
if ($defaultsNode->hasAttribute('inherit-tags')) {
$defaults['inherit-tags'] = XmlUtils::phpize($defaultsNode->getAttribute('inherit-tags'));
}
if ($defaultsNode->hasAttribute('autoconfigure')) {
$defaults['autoconfigure'] = XmlUtils::phpize($defaultsNode->getAttribute('autoconfigure'));
}
Expand Down Expand Up @@ -225,13 +222,6 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults =
}

$definition = new ChildDefinition($parent);

if ($value = $service->getAttribute('inherit-tags')) {
$definition->setInheritTags(XmlUtils::phpize($value));
} elseif (isset($defaults['inherit-tags'])) {
$definition->setInheritTags($defaults['inherit-tags']);
}
$defaults = array();
} else {
$definition = new Definition();

Expand Down Expand Up @@ -318,13 +308,7 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults =

$tags = $this->getChildren($service, 'tag');

if (empty($defaults['tags'])) {
// no-op
} elseif (!$value = $service->getAttribute('inherit-tags')) {
if (!$tags) {
$tags = $defaults['tags'];
}
} elseif (XmlUtils::phpize($value)) {
if (!empty($defaults['tags'])) {
$tags = array_merge($tags, $defaults['tags']);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ class YamlFileLoader extends FileLoader
'configurator' => 'configurator',
'calls' => 'calls',
'tags' => 'tags',
'inherit_tags' => 'inherit_tags',
'decorates' => 'decorates',
'decoration_inner_name' => 'decoration_inner_name',
'decoration_priority' => 'decoration_priority',
Expand All @@ -74,7 +73,6 @@ class YamlFileLoader extends FileLoader
'configurator' => 'configurator',
'calls' => 'calls',
'tags' => 'tags',
'inherit_tags' => 'inherit_tags',
'autowire' => 'autowire',
'autoconfigure' => 'autoconfigure',
);
Expand All @@ -93,7 +91,6 @@ class YamlFileLoader extends FileLoader
private static $defaultsKeywords = array(
'public' => 'public',
'tags' => 'tags',
'inherit_tags' => 'inherit_tags',
'autowire' => 'autowire',
'autoconfigure' => 'autoconfigure',
);
Expand Down Expand Up @@ -365,12 +362,6 @@ private function parseDefinition($id, $service, $file, array $defaults)
}

$definition = new ChildDefinition($service['parent']);

$inheritTag = isset($service['inherit_tags']) ? $service['inherit_tags'] : (isset($defaults['inherit_tags']) ? $defaults['inherit_tags'] : null);
if (null !== $inheritTag) {
$definition->setInheritTags($inheritTag);
}
$defaults = array();
} else {
$definition = new Definition();

Expand Down Expand Up @@ -458,13 +449,7 @@ private function parseDefinition($id, $service, $file, array $defaults)
throw new InvalidArgumentException(sprintf('Parameter "tags" must be an array for service "%s" in %s. Check your YAML syntax.', $id, $file));
}

if (!isset($defaults['tags'])) {
// no-op
} elseif (!isset($service['inherit_tags'])) {
if (!isset($service['tags'])) {
$tags = $defaults['tags'];
}
} elseif ($service['inherit_tags']) {
if (isset($defaults['tags'])) {
$tags = array_merge($tags, $defaults['tags']);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@
</xsd:choice>
<xsd:attribute name="public" type="boolean" />
<xsd:attribute name="autowire" type="boolean" />
<xsd:attribute name="inherit-tags" type="boolean" />
<xsd:attribute name="autoconfigure" type="boolean" />
</xsd:complexType>

Expand Down Expand Up @@ -132,7 +131,6 @@
<xsd:attribute name="decoration-inner-name" type="xsd:string" />
<xsd:attribute name="decoration-priority" type="xsd:integer" />
<xsd:attribute name="autowire" type="boolean" />
<xsd:attribute name="inherit-tags" type="boolean" />
<xsd:attribute name="autoconfigure" type="boolean" />
</xsd:complexType>

Expand Down Expand Up @@ -169,7 +167,6 @@
<xsd:attribute name="abstract" type="boolean" />
<xsd:attribute name="parent" type="xsd:string" />
<xsd:attribute name="autowire" type="boolean" />
<xsd:attribute name="inherit-tags" type="boolean" />
<xsd:attribute name="autoconfigure" type="boolean" />
</xsd:complexType>

Expand Down
10000
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\Compiler\ResolveInstanceofConditionalsPass;
use Symfony\Component\DependencyInjection\Compiler\ResolveDefinitionTemplatesPass;
use Symfony\Component\DependencyInjection\Compiler\ResolveTagsInheritancePass;
use Symfony\Component\DependencyInjection\ContainerBuilder;

class ResolveInstanceofConditionalsPassTest extends TestCase
Expand All @@ -35,7 +34,6 @@ public function testProcess()
$this->assertEmpty($def->getInstanceofConditionals());
$this->assertInstanceof(ChildDefinition::class, $def);
$this->assertTrue($def->isAutowired());
$this->assertFalse($def->getInheritTags());
$this->assertSame($parent, $def->getParent());
$this->assertSame(array('tag' => array(array()), 'baz' => array(array('attr' => 123))), $def->getTags());

Expand Down Expand Up @@ -121,7 +119,6 @@ public function testProcessUsesAutoconfiguredInstanceof()
->setFactory('autoconfigured_factory');

(new ResolveInstanceofConditionalsPass())->process($container);
(new ResolveTagsInheritancePass())->process($container);
(new ResolveDefinitionTemplatesPass())->process($container);

$def = $container->getDefinition('normal_service');
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
</defaults>

<service id="with_defaults" class="Foo" />
<service id="no_defaults" class="Foo" public="true" autowire="false" inherit-tags="false">
<service id="no_defaults" class="Foo" public="true" autowire="false">
</service>
</services>
</container>
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ services:
# these 2 are from instanceof
- { name: foo_tag, tag_option: from_instanceof }
- { name: bar_tag }
# the tag from defaults do NOT cascade (but see #22530)
# - { name: from_defaults }
- { name: from_defaults }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key fix for this PR: the from_defaults tag is now inherited:

# calls from instanceof are kept, but this comes later
calls:
# first call is from instanceof
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ services:
class: Foo
public: true
autowire: ~
inherit_tags: false

no_defaults:
class: Foo
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@ services:

Foo\Bar:
tags: invalid
inherit_tags: true
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,7 @@ public function testDefaults()

$this->assertTrue($container->getDefinition('no_defaults')->isPublic());

$this->assertSame(array(), $container->getDefinition('no_defaults')->getTags());
$this->assertSame(array('foo' => array(array())), $container->getDefinition('no_defaults')->getTags());

$this->assertFalse($container->getDefinition('no_defaults')->isAutowired());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -412,8 +412,9 @@ public function testDefaults()
$this->assertTrue($container->getDefinition('with_null')->isPublic());
$this->assertTrue($container->getDefinition('no_defaults')->isPublic());

$this->assertSame(array(), $container->getDefinition('with_null')->getTags());
$this->assertSame(array(), $container->getDefinition('no_defaults')->getTags());
// foo tag is inherited from defaults
$this->assertSame(array('foo' => array(array())), $container->getDefinition('with_null')->getTags());
$this->assertSame(array('foo' => array(array())), $container->getDefinition('no_defaults')->getTags());

$this->assertTrue($container->getDefinition('with_null')->isAutowired());
$this->assertFalse($container->getDefinition('no_defaults')->isAutowired());
Expand Down
0