[DI] Fixes: symfony#28326 - Overriding services autowired by name under _defaults bind not working

przemyslaw-bogusz · nicolas-grekas · commit 35a40ace6f5a · 2019-04-12T16:15:59.000+02:00
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php
@@ -34,6 +34,8 @@ class ResolveBindingsPass extends AbstractRecursivePass
      */
     public function process(ContainerBuilder $container)
     {
+        $this->usedBindings = $container->getRemovedBindingIds();
+
         try {
             parent::process($container);
 
diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php
@@ -123,6 +123,8 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
 
     private $removedIds = [];
 
+    private $removedBindingIds = [];
+
     private static $internalTypes = [
         'int' => true,
         'float' => true,
@@ -1504,6 +1506,35 @@ public function normalizeId($id)
         return isset($this->definitions[$id]) || isset($this->aliasDefinitions[$id]) || isset($this->removedIds[$id]) ? $id : parent::normalizeId($id);
     }
 
+    /**
+     * Gets removed binding ids.
+     *
+     * @return array
+     *
+     * @internal
+     */
+    public function getRemovedBindingIds()
+    {
+        return $this->removedBindingIds;
+    }
+
+    /**
+     * Adds a removed binding id.
+     *
+     * @param int $id
+     *
+     * @internal
+     */
+    public function addRemovedBindingIds($id)
+    {
+        if ($this->hasDefinition($id)) {
+            foreach ($this->getDefinition($id)->getBindings() as $key => $binding) {
+                list(, $bindingId) = $binding->getValues();
+                $this->removedBindingIds[(int) $bindingId] = true;
+            }
+        }
+    }
+
     /**
      * Returns the Service Conditionals.
      *
diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServiceConfigurator.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServiceConfigurator.php
@@ -59,6 +59,8 @@ public function __destruct()
     {
         parent::__destruct();
 
+        $this->container->addRemovedBindingIds($this->id);
+
         if (!$this->definition instanceof ChildDefinition) {
             $this->container->setDefinition($this->id, $this->definition->setInstanceofConditionals($this->instanceof));
         } else {
diff --git a/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php
@@ -91,6 +91,8 @@ public function registerClasses(Definition $prototype, $namespace, $resource, $e
      */
     protected function setDefinition($id, Definition $definition)
     {
+        $this->container->addRemovedBindingIds($id);
+
         if ($this->isLoadingInstanceof) {
             if (!$definition instanceof ChildDefinition) {
                 throw new InvalidArgumentException(sprintf('Invalid type definition "%s": ChildDefinition expected, "%s" given.', $id, \get_class($definition)));
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings.yml
@@ -0,0 +1,11 @@
+services:
+    _defaults:
+        bind:
+            $quz: value
+            $foo: [value]
+
+    bar:
+        class: Symfony\Component\DependencyInjection\Tests\Fixtures\Bar
+
+    foo:
+        class: Symfony\Component\DependencyInjection\Tests\Fixtures\FactoryDummy
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings2.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings2.yml
@@ -0,0 +1,7 @@
+services:
+    _defaults:
+        bind:
+            $quz: ~
+
+    bar:
+        class: Symfony\Component\DependencyInjection\Tests\Fixtures\Bar
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php
@@ -17,6 +17,7 @@
 use Symfony\Component\Config\Resource\FileResource;
 use Symfony\Component\Config\Resource\GlobResource;
 use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
+use Symfony\Component\DependencyInjection\Compiler\ResolveBindingsPass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Loader\IniFileLoader;
 use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
@@ -732,4 +733,20 @@ public function testBindings()
             '$factory' => 'factory',
         ], array_map(function ($v) { return $v->getValues()[0]; }, $definition->getBindings()));
     }
+
+    /**
+     * The pass may throw an exception, which will cause the test to fail.
+     */
+    public function testOverriddenDefaultsBindings()
+    {
+        $container = new ContainerBuilder();
+
+        $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml'));
+        $loader->load('defaults_bindings.yml');
+        $loader->load('defaults_bindings2.yml');
+
+        (new ResolveBindingsPass())->process($container);
+
+        $this->assertTrue(true);
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -34,6 +34,8 @@ class ResolveBindingsPass extends AbstractRecursivePass`
`34`	`34`	`*/`
`35`	`35`	`public function process(ContainerBuilder $container)`
`36`	`36`	`{`
	`37`	`+ $this->usedBindings = $container->getRemovedBindingIds();`
	`38`	`+`
`37`	`39`	`try {`
`38`	`40`	`parent::process($container);`
`39`	`41`
Original file line number	Diff line number	Diff line change
`@@ -59,6 +59,8 @@ public function __destruct()`
`59`	`59`	`{`
`60`	`60`	`parent::__destruct();`
`61`	`61`
	`62`	`+ $this->container->addRemovedBindingIds($this->id);`
	`63`	`+`
`62`	`64`	`if (!$this->definition instanceof ChildDefinition) {`
`63`	`65`	`$this->container->setDefinition($this->id, $this->definition->setInstanceofConditionals($this->instanceof));`
`64`	`66`	`} else {`
Original file line number	Diff line number	Diff line change
`@@ -91,6 +91,8 @@ public function registerClasses(Definition $prototype, $namespace, $resource, $e`
`91`	`91`	`*/`
`92`	`92`	`protected function setDefinition($id, Definition $definition)`
`93`	`93`	`{`
	`94`	`+ $this->container->addRemovedBindingIds($id);`
	`95`	`+`
`94`	`96`	`if ($this->isLoadingInstanceof) {`
`95`	`97`	`if (!$definition instanceof ChildDefinition) {`
`96`	`98`	`throw new InvalidArgumentException(sprintf('Invalid type definition "%s": ChildDefinition expected, "%s" given.', $id, \get_class($definition)));`