feature symfony#22356 [DI] Rework config hierarchy: defaults > instanceof > service config (weaverryan, nicolas-grekas)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Rework config hierarchy: defaults > instanceof > service config

| 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        | -

Replaces symfony#22294.
The main change is that ChildDefinition does not have "instanceof" applied anymore. All the complexity of the pass came from the merging logic required to deal with them. But in fact, we'd better not have any such logic and just not apply "instanceof" to ChildDefinition (but have them inherit from their parents with the usual logic).

Commits
-------

6d6116b Adding an integration test for the hirarchy of defaults, instanceof, child, parent definitions
ab86457 [DI] Rework config hierarchy: defaults > instanceof > service config
cbaee55 [DI] Track changes at the "Definition" level

Author: fabpot

src/Symfony/Component/DependencyInjection/ChildDefinition.php

@@ -23,15 +23,12 @@ class ChildDefinition extends Definition
 {
     private $parent;
     private $inheritTags = false;
-    private $changes = array();
 
     /**
      * @param string $parent The id of Definition instance to decorate
      */
     public function __construct($parent)
     {
-        parent::__construct();
-
         $this->parent = $parent;
     }
 
@@ -46,13 +43,17 @@ public function getParent()
     }
 
     /**
-     * Returns all changes tracked for the Definition object.
+     * Sets the Definition being decorated.
+     *
+     * @param string $parent
      *
-     * @return array An array of changes for this Definition
+     * @return $this
      */
-    public function getChanges()
+    public function setParent($parent)
     {
-        return $this->changes;
+        $this->parent = $parent;
+
+        return $this;
     }
 
     /**
@@ -79,116 +80,6 @@ public function getInheritTags()
         return $this->inheritTags;
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function setClass($class)
-    {
-        $this->changes['class'] = true;
-
-        return parent::setClass($class);
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function setFactory($callable)
-    {
-        $this->changes['factory'] = true;
-
-        return parent::setFactory($callable);
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function setConfigurator($callable)
-    {
-        $this->changes['configurator'] = true;
-
-        return parent::setConfigurator($callable);
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function setFile($file)
-    {
-        $this->changes['file'] = true;
-
-        return parent::setFile($file);
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function setShared($boolean)
-    {
-        $this->changes['shared'] = true;
-
-        return parent::setShared($boolean);
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function setPublic($boolean)
-    {
-        $this->changes['public'] = true;
-
-        return parent::setPublic($boolean);
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function setLazy($boolean)
-    {
-        $this->changes['lazy'] = true;
-
-        return parent::setLazy($boolean);
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function setAbstract($boolean)
-    {
-        $this->changes['abstract'] = true;
-
-        return parent::setAbstract($boolean);
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function setDecoratedService($id, $renamedId = null, $priority = 0)
-    {
-        $this->changes['decorated_service'] = true;
-
-        return parent::setDecoratedService($id, $renamedId, $priority);
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function setDeprecated($boolean = true, $template = null)
-    {
-        $this->changes['deprecated'] = true;
-
-        return parent::setDeprecated($boolean, $template);
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function setAutowired($autowired)
-    {
-        $this->changes['autowired'] = true;
-
-        return parent::setAutowired($autowired);
-    }
-
     /**
      * Gets an argument to pass to the service constructor/factory method.
      *

src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php

@@ -65,11 +65,12 @@ protected function processValue($value, $isRoot = false)
             $value->setProperties($this->processValue($value->getProperties()));
             $value->setMethodCalls($this->processValue($value->getMethodCalls()));
 
-            if ($v = $value->getFactory()) {
-                $value->setFactory($this->processValue($v));
+            $changes = $value->getChanges();
+            if (isset($changes['factory'])) {
+                $value->setFactory($this->processValue($value->getFactory()));
             }
-            if ($v = $value->getConfigurator()) {
-                $value->setConfigurator($this->processValue($v));
+            if (isset($changes['configurator'])) {
+                $value->setConfigurator($this->processValue($value->getConfigurator()));
             }
         }
 

src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php

@@ -42,7 +42,8 @@ public function __construct()
         $this->beforeOptimizationPasses = array(
             100 => array(
                 $resolveClassPass = new ResolveClassPass(),
-                new ResolveDefinitionInheritancePass(),
+                new ResolveInstanceofConditionalsPass(),
+                new ResolveTagsInheritancePass(),
             ),
         );
 

src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionInheritancePass.php

@@ -84,7 +84,7 @@ private function doResolveDefinition(ChildDefinition $definition)
         $def = new Definition();
 
         // merge in parent definition
-        // purposely ignored attributes: abstract, tags
+        // purposely ignored attributes: abstract, shared, tags
         $def->setClass($parentDef->getClass());
         $def->setArguments($parentDef->getArguments());
         $def->setMethodCalls($parentDef->getMethodCalls());
@@ -101,27 +101,8 @@ private function doResolveDefinition(ChildDefinition $definition)
         $def->setPublic($parentDef->isPublic());
         $def->setLazy($parentDef->isLazy());
         $def->setAutowired($parentDef->isAutowired());
+        $def->setChanges($parentDef->getChanges());
 
-        self::mergeDefinition($def, $definition);
-
-        // merge autowiring types
-        foreach ($definition->getAutowiringTypes(false) as $autowiringType) {
-            $def->addAutowiringType($autowiringType);
-        }
-
-        // these attributes are always taken from the child
-        $def->setAbstract($definition->isAbstract());
-        $def->setShared($definition->isShared());
-        $def->setTags($definition->getTags());
-
-        return $def;
-    }
-
-    /**
-     * @internal
-     */
-    public static function mergeDefinition(Definition $def, ChildDefinition $definition)
-    {
         // overwrite with values specified in the decorator
         $changes = $definition->getChanges();
         if (isset($changes['class'])) {
@@ -148,6 +129,9 @@ public static function mergeDefinition(Definition $def, ChildDefinition $definit
         if (isset($changes['autowired'])) {
             $def->setAutowired($definition->isAutowired());
         }
+        if (isset($changes['shared'])) {
+            $def->setShared($definition->isShared());
+        }
         if (isset($changes['decorated_service'])) {
             $decoratedService = $definition->getDecoratedService();
             if (null === $decoratedService) {
@@ -177,5 +161,16 @@ public static function mergeDefinition(Definition $def, ChildDefinition $definit
         if ($calls = $definition->getMethodCalls()) {
             $def->setMethodCalls(array_merge($def->getMethodCalls(), $calls));
         }
+
+        // merge autowiring types
+        foreach ($definition->getAutowiringTypes(false) as $autowiringType) {
+            $def->addAutowiringType($autowiringType);
+        }
+
+        // these attributes are always taken from the child
+        $def->setAbstract($definition->isAbstract());
+        $def->setTags($definition->getTags());
+
+        return $def;
     }
 }