From 168765da0f273c8ea9c08c197976c0533cd36848 Mon Sep 17 00:00:00 2001
From: Nicolas Grekas <nicolas.grekas@gmail.com>
Date: Wed, 12 Apr 2017 00:06:53 +0200
Subject: [PATCH] [DI] Fix inheriting defaults with instanceof conditionals

---
 .../ResolveInstanceofConditionalsPass.php     | 48 ++++++++++++-------
 .../ResolveInstanceofConditionalsPassTest.php | 13 +++--
 2 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php
index 940d113c0b..479fe12559 100644
--- 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 @@ class ResolveInstanceofConditionalsPass implements CompilerPassInterface
         }
 
         $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/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php
index af27ab28c1..0829aa8b3e 100644
--- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php
+++ b/src/Symfony/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 @@ class ResolveInstanceofConditionalsPassTest extends TestCase
         $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()