From 037a782b9114ba8b049ef214f6af3f21596785ad Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Tue, 25 Apr 2017 22:22:00 -0400 Subject: [PATCH] Making tags under _defaults always apply and removing inherit_tags entirely Now that inherit_tags has been removed, 3.3 has the same functionality as 3.2: tags are *never* cascaded from parent to child (but you tags do inherit from defaults to a service and instanceof to a service). --- .../DependencyInjection/CHANGELOG.md | 3 +- .../DependencyInjection/ChildDefinition.php | 25 ------- .../Compiler/PassConfig.php | 1 - .../ResolveInstanceofConditionalsPass.php | 2 +- .../Compiler/ResolveTagsInheritancePass.php | 74 ------------------- .../Loader/XmlFileLoader.php | 18 +---- .../Loader/YamlFileLoader.php | 17 +---- .../schema/dic/services/services-1.0.xsd | 3 - .../ResolveInstanceofConditionalsPassTest.php | 3 - .../ResolveTagsInheritancePassTest.php | 34 --------- .../Tests/Fixtures/xml/services29.xml | 2 +- .../expected.yml | 3 +- .../Tests/Fixtures/yaml/services28.yml | 1 - .../Fixtures/yaml/services31_invalid_tags.yml | 1 - .../Tests/Loader/XmlFileLoaderTest.php | 2 +- .../Tests/Loader/YamlFileLoaderTest.php | 5 +- 16 files changed, 10 insertions(+), 184 deletions(-) delete mode 100644 src/Symfony/Component/DependencyInjection/Compiler/ResolveTagsInheritancePass.php delete mode 100644 src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveTagsInheritancePassTest.php diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md index e57d856fe693c..7e03a508561de 100644 --- a/src/Symfony/Component/DependencyInjection/CHANGELOG.md +++ b/src/Symfony/Component/DependencyInjection/CHANGELOG.md @@ -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 diff --git a/src/Symfony/Component/DependencyInjection/ChildDefinition.php b/src/Symfony/Component/DependencyInjection/ChildDefinition.php index d202170e373b2..dd02f860677f8 100644 --- a/src/Symfony/Component/DependencyInjection/ChildDefinition.php +++ b/src/Symfony/Component/DependencyInjection/ChildDefinition.php @@ -23,7 +23,6 @@ class ChildDefinition extends Definition { private $parent; - private $inheritTags = false; /** * @param string $parent The id of Definition instance to decorate @@ -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. * diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php index 3936df509a0e9..036e91233c01a 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php @@ -43,7 +43,6 @@ public function __construct() 100 => array( $resolveClassPass = new ResolveClassPass(), new ResolveInstanceofConditionalsPass(), - new ResolveTagsInheritancePass(), ), ); diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php index 6ec07672567d6..a2e62f4e536a5 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php @@ -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(); diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveTagsInheritancePass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveTagsInheritancePass.php deleted file mode 100644 index f95e6bbd533ae..0000000000000 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveTagsInheritancePass.php +++ /dev/null @@ -1,74 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\DependencyInjection\Compiler; - -use Symfony\Component\DependencyInjection\ChildDefinition; -use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\DependencyInjection\Exception\RuntimeException; - -/** - * Applies tags inheritance to definitions. - * - * @author Nicolas Grekas - */ -class ResolveTagsInheritancePass extends AbstractRecursivePass -{ - private $abstractInheritedParents = array(); - - /** - * {@inheritdoc} - */ - public function process(ContainerBuilder $container) - { - try { - parent::process($container); - - foreach ($this->abstractInheritedParents as $id) { - $container->findDefinition($id)->setTags(array()); - } - } finally { - $this->abstractInheritedParents = array(); - } - } - - /** - * {@inheritdoc} - */ - protected function processValue($value, $isRoot = false) - { - if (!$value instanceof ChildDefinition || !$value->getInheritTags()) { - return parent::processValue($value, $isRoot); - } - $value->setInheritTags(false); - - if (!$this->container->has($parent = $value->getParent())) { - throw new RuntimeException(sprintf('Parent definition "%s" does not exist.', $parent)); - } - - $parentDef = $this->container->findDefinition($parent); - if ($parentDef->isAbstract()) { - $this->abstractInheritedParents[$parent] = $parent; - } - - if ($parentDef instanceof ChildDefinition) { - $this->processValue($parentDef); - } - - foreach ($parentDef->getTags() as $k => $v) { - foreach ($v as $v) { - $value->addTag($k, $v); - } - } - - return parent::processValue($value, $isRoot); - } -} diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index 5948201f02012..79a0b1213d4ce 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -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')); } @@ -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(); @@ -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']); } diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index ecac79367c279..e4b2a1ba6fe39 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -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', @@ -74,7 +73,6 @@ class YamlFileLoader extends FileLoader 'configurator' => 'configurator', 'calls' => 'calls', 'tags' => 'tags', - 'inherit_tags' => 'inherit_tags', 'autowire' => 'autowire', 'autoconfigure' => 'autoconfigure', ); @@ -93,7 +91,6 @@ class YamlFileLoader extends FileLoader private static $defaultsKeywords = array( 'public' => 'public', 'tags' => 'tags', - 'inherit_tags' => 'inherit_tags', 'autowire' => 'autowire', 'autoconfigure' => 'autoconfigure', ); @@ -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(); @@ -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']); } diff --git a/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd b/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd index c659694ffaa39..43ea65a2d8577 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd +++ b/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd @@ -103,7 +103,6 @@ - @@ -132,7 +131,6 @@ - @@ -169,7 +167,6 @@ - diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php index 6467ac7ed4d71..c6c3681025606 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php @@ -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 @@ -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()); @@ -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'); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveTagsInheritancePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveTagsInheritancePassTest.php deleted file mode 100644 index 3ed8e852afa82..0000000000000 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveTagsInheritancePassTest.php +++ /dev/null @@ -1,34 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\DependencyInjection\Tests\Compiler; - -use PHPUnit\Framework\TestCase; -use Symfony\Component\DependencyInjection\ChildDefinition; -use Symfony\Component\DependencyInjection\Compiler\ResolveTagsInheritancePass; -use Symfony\Component\DependencyInjection\ContainerBuilder; - -class ResolveTagsInheritancePassTest extends TestCase -{ - public function testProcess() - { - $container = new ContainerBuilder(); - $container->register('grandpa', self::class)->addTag('g'); - $container->setDefinition('parent', new ChildDefinition('grandpa'))->addTag('p')->setInheritTags(true)->setAbstract(true); - $container->setDefinition('child', new ChildDefinition('parent'))->setInheritTags(true); - - (new ResolveTagsInheritancePass())->process($container); - - $expected = array('p' => array(array()), 'g' => array(array())); - $this->assertSame($expected, $container->getDefinition('child')->getTags()); - $this->assertSame(array(), $container->getDefinition('parent')->getTags()); - } -} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services29.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services29.xml index 81f1c1a4409a1..1ce2d961eb93b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services29.xml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services29.xml @@ -6,7 +6,7 @@ - + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/defaults_instanceof_importance/expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/defaults_instanceof_importance/expected.yml index 4edeffa7c3053..e9161dccfc079 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/defaults_instanceof_importance/expected.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/defaults_instanceof_importance/expected.yml @@ -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 } # calls from instanceof are kept, but this comes later calls: # first call is from instanceof diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services28.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services28.yml index cacebf469cf2e..bc4b6b098f21a 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services28.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services28.yml @@ -14,7 +14,6 @@ services: class: Foo public: true autowire: ~ - inherit_tags: false no_defaults: class: Foo diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services31_invalid_tags.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services31_invalid_tags.yml index 9d4ac3515adc4..a6061a90e8751 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services31_invalid_tags.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services31_invalid_tags.yml @@ -4,4 +4,3 @@ services: Foo\Bar: tags: invalid - inherit_tags: true diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php index 6f85d2ee16be4..db48db99c6032 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php @@ -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()); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index 907fcb631210f..6407112b407d6 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php @@ -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());