bug #22386 [DI] Fix inheriting defaults with instanceof conditionals …

…(nicolas-grekas) This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Fix inheriting defaults with instanceof conditionals | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Commits ------- 168765d [DI] Fix inheriting defaults with instanceof conditionals
symfony · Apr 13, 2017 · 64b715b · 64b715b
2 parents 8596b19 + 168765d
commit 64b715b
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 22 deletions.
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php
@@ -27,19 +27,12 @@ class ResolveInstanceofConditionalsPass implements CompilerPassInterface
      */
     public function process(ContainerBuilder $container)
     {
-        $didProcess = false;
         foreach ($container->getDefinitions() as $id => $definition) {
             if ($definition instanceof ChildDefinition) {
                 // don't apply "instanceof" to children: it will be applied to their parent
                 continue;
             }
-            if ($definition !== $processedDefinition = $this->processDefinition($container, $id, $definition)) {
-                $didProcess = true;
-                $container->setDefinition($id, $processedDefinition);
-            }
-        }
-        if ($didProcess) {
-            $container->register('abstract.'.__CLASS__, '')->setAbstract(true);
+            $container->setDefinition($id, $this->processDefinition($container, $id, $definition));
         }
     }
 
@@ -53,37 +46,56 @@ private function processDefinition(ContainerBuilder $container, $id, Definition
         }
 
         $definition->setInstanceofConditionals(array());
-        $instanceofParent = null;
-        $parent = 'abstract.'.__CLASS__;
-        $shared = null;
+        $parent = $shared = null;
+        $instanceofTags = array();
 
         foreach ($instanceofConditionals as $interface => $instanceofDef) {
             if ($interface !== $class && (!$container->getReflectionClass($interface) || !$container->getReflectionClass($class))) {
                 continue;
             }
             if ($interface === $class || is_subclass_of($class, $interface)) {
-                $instanceofParent = clone $instanceofDef;
-                $instanceofParent->setAbstract(true)->setInheritTags(true)->setParent($parent);
+                $instanceofDef = clone $instanceofDef;
+                $instanceofDef->setAbstract(true)->setInheritTags(false)->setParent($parent ?: 'abstract.instanceof.'.$id);
                 $parent = 'instanceof.'.$interface.'.'.$id;
-                $container->setDefinition($parent, $instanceofParent);
+                $container->setDefinition($parent, $instanceofDef);
+                $instanceofTags[] = $instanceofDef->getTags();
+                $instanceofDef->setTags(array());
 
-                if (isset($instanceofParent->getChanges()['shared'])) {
-                    $shared = $instanceofParent->isShared();
+                if (isset($instanceofDef->getChanges()['shared'])) {
+                    $shared = $instanceofDef->isShared();
                 }
             }
         }
 
-        if ($instanceofParent) {
+        if ($parent) {
+            $abstract = $container->setDefinition('abstract.instanceof.'.$id, $definition);
+
             // cast Definition to ChildDefinition
             $definition = serialize($definition);
             $definition = substr_replace($definition, '53', 2, 2);
             $definition = substr_replace($definition, 'Child', 44, 0);
             $definition = unserialize($definition);
-            $definition->setInheritTags(true)->setParent($parent);
+            $definition->setParent($parent);
 
             if (null !== $shared && !isset($definition->getChanges()['shared'])) {
                 $definition->setShared($shared);
             }
+
+            $i = count($instanceofTags);
+            while (0 <= --$i) {
+                foreach ($instanceofTags[$i] as $k => $v) {
+                    foreach ($v as $v) {
+                        $definition->addTag($k, $v);
+                    }
+                }
+            }
+
+            // reset fields with "merge" behavior
+            $abstract
+                ->setArguments(array())
+                ->setMethodCalls(array())
+                ->setTags(array())
+                ->setAbstract(true);
         }
 
         return $definition;

diff --git a/...ny/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php b/...ny/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php
@@ -22,9 +22,9 @@ class ResolveInstanceofConditionalsPassTest extends TestCase
     public function testProcess()
     {
         $container = new ContainerBuilder();
-        $def = $container->register('foo', self::class);
+        $def = $container->register('foo', self::class)->addTag('tag')->setAutowired(true)->setChanges(array());
         $def->setInstanceofConditionals(array(
-            parent::class => (new ChildDefinition(''))->setProperty('foo', 'bar'),
+            parent::class => (new ChildDefinition(''))->setProperty('foo', 'bar')->addTag('baz', array('attr' => 123)),
         ));
 
         (new ResolveInstanceofConditionalsPass())->process($container);
@@ -33,9 +33,14 @@ public function testProcess()
         $def = $container->getDefinition('foo');
         $this->assertEmpty($def->getInstanceofConditionals());
         $this->assertInstanceof(ChildDefinition::class, $def);
-        $this->assertTrue($def->getInheritTags());
+        $this->assertTrue($def->isAutowired());
+        $this->assertFalse($def->getInheritTags());
         $this->assertSame($parent, $def->getParent());
-        $this->assertEquals(array('foo' => 'bar'), $container->getDefinition($parent)->getProperties());
+        $this->assertSame(array('tag' => array(array()), 'baz' => array(array('attr' => 123))), $def->getTags());
+
+        $parent = $container->getDefinition($parent);
+        $this->assertSame(array('foo' => 'bar'), $parent->getProperties());
+        $this->assertSame(array(), $parent->getTags());
     }
 
     public function testProcessInheritance()