Fixing a bug where services that were eventually removed could cause autowire errors

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

@@ -0,0 +1,38 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * 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\ContainerBuilder;
+
+/**
+ * Throws autowire exceptions from AutowirePass for definitions that still exist.
+ *
+ * @author Ryan Weaver <ryan@knpuniversity.com>
+ */
+class AutowireExceptionPass implements CompilerPassInterface
+{
+    private $autowirePass;
+
+    public function __construct(AutowirePass $autowirePass)
+    {
+        $this->autowirePass = $autowirePass;
+    }
+
+    public function process(ContainerBuilder $container)
+    {
+        foreach ($this->autowirePass->getAutowiringExceptions() as $exception) {
+            if ($container->hasDefinition($exception->getServiceId())) {
+                throw $exception;
+            }
+        }
+    }
+}

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

@@ -14,6 +14,7 @@ namespace Symfony\Component\DependencyInjection\Compiler;
 use Symfony\Component\DependencyInjection\Config\AutowireServiceResource;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
 use Symfony\Component\DependencyInjection\Exception\RuntimeException;
 use Symfony\Component\DependencyInjection\LazyProxy\ProxyHelper;
 use Symfony\Component\DependencyInjection\TypedReference;
@@ -31,12 +32,33 @@ class AutowirePass extends AbstractRecursivePass
     private $ambiguousServiceTypes = array();
     private $autowired = array();
     private $lastFailure;
+    private $throwOnAutowiringException;
+    private $autowiringExceptions = array();
+
+    /**
+     * @param bool $throwOnAutowireException If false, retrieved errors via getAutowiringExceptions
+     */
+    public function __construct($throwOnAutowireException = true)
+    {
+        $this->throwOnAutowiringException = $throwOnAutowireException;
+    }
+
+    /**
+     * @return AutowiringFailedException[]
+     */
+    public function getAutowiringExceptions()
+    {
+        return $this->autowiringExceptions;
+    }
 
     /**
      * {@inheritdoc}
      */
     public function process(ContainerBuilder $container)
     {
+        // clear out any possibly stored exceptions from before
+        $this->autowiringExceptions = array();
+
         try {
             parent::process($container);
         } finally {
@@ -75,6 +97,21 @@ class AutowirePass extends AbstractRecursivePass
      * {@inheritdoc}
      */
     protected function processValue($value, $isRoot = false)
+    {
+        try {
+            return $this->doProcessValue($value, $isRoot);
+        } catch (AutowiringFailedException $e) {
+            if ($this->throwOnAutowiringException) {
+                throw $e;
+            }
+
+            $this->autowiringExceptions[] = $e;
+
+            return parent::processValue($value, $isRoot);
+        }
+    }
+
+    private function doProcessValue($value, $isRoot = false)
     {
         if ($value instanceof TypedReference) {
             if ($ref = $this->getAutowiredReference($value)) {
@@ -190,7 +227,7 @@ class AutowirePass extends AbstractRecursivePass
 
             if (!$reflectionMethod->isPublic()) {
                 $class = $reflectionClass->name;
-                throw new RuntimeException(sprintf('Cannot autowire service "%s": method "%s()" must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
+                throw new AutowiringFailedException($this->currentId, sprintf('Cannot autowire service "%s": method "%s()" must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
             }
             $methodCalls[] = array($method, $this->autowireMethod($reflectionMethod, array()));
         }
@@ -231,7 +268,7 @@ class AutowirePass extends AbstractRecursivePass
 
                 // no default value? Then fail
                 if (!$parameter->isDefaultValueAvailable()) {
-                    throw new RuntimeException(sprintf('Cannot autowire service "%s": argument "$%s" of method "%s()" must have a type-hint or be given a value explicitly.', $this->currentId, $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
+                    throw new AutowiringFailedException($this->currentId, sprintf('Cannot autowire service "%s": argument "$%s" of method "%s()" must have a type-hint or be given a value explicitly.', $this->currentId, $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
                 }
 
                 // specifically pass the default value
@@ -246,7 +283,7 @@ class AutowirePass extends AbstractRecursivePass
                 if ($parameter->isDefaultValueAvailable()) {
                     $value = $parameter->getDefaultValue();
                 } elseif (!$parameter->allowsNull()) {
-                    throw new RuntimeException($failureMessage);
+                    throw new AutowiringFailedException($this->currentId, $failureMessage);
                 }
                 $this->container->log($this, $failureMessage);
             }
@@ -407,15 +444,18 @@ class AutowirePass extends AbstractRecursivePass
         $argumentDefinition->setAutowired(true);
 
         try {
+            $originalThrowSetting = $this->throwOnAutowiringException;
+            $this->throwOnAutowiringException = true;
             $this->processValue($argumentDefinition, true);
             $this->container->setDefinition($argumentId, $argumentDefinition);
-        } catch (RuntimeException $e) {
+        } catch (AutowiringFailedException $e) {
             $this->autowired[$type] = false;
             $this->lastFailure = $e->getMessage();
             $this->container->log($this, $this->lastFailure);
 
             return;
         } finally {
+            $this->throwOnAutowiringException = $originalThrowSetting;
             $this->currentId = $currentId;
         }
 

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

@@ -57,7 +57,7 @@ class PassConfig
             new CheckDefinitionValidityPass(),
             new RegisterServiceSubscribersPass(),
             new ResolveNamedArgumentsPass(),
-            new AutowirePass(),
+            $autowirePass = new AutowirePass(false),
             new ResolveServiceSubscribersPass(),
             new ResolveReferencesToAliasesPass(),
             new ResolveInvalidReferencesPass(),
@@ -77,6 +77,7 @@ class PassConfig
                 new AnalyzeServiceReferencesPass(),
                 new RemoveUnusedDefinitionsPass(),
             )),
+            new AutowireExceptionPass($autowirePass),
             new CheckExceptionOnInvalidReferenceBehaviorPass(),
         ));
     }

src/Symfony/Component/DependencyInjection/Exception/AutowiringFailedException.php

@@ -0,0 +1,32 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\DependencyInjection\Exception;
+
+/**
+ * Thrown when a definition cannot be autowired.
+ */
+class AutowiringFailedException extends RuntimeException
+{
+    private $serviceId;
+
+    public function __construct($serviceId, $message = '', $code = 0, Exception $previous = null)
+    {
+        $this->serviceId = $serviceId;
+
+        parent::__construct($message, $code, $previous);
+    }
+
+    public function getServiceId()
+    {
+        return $this->serviceId;
+    }
+}

src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowireExceptionPassTest.php

@@ -0,0 +1,63 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * 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\Compiler\AutowireExceptionPass;
+use Symfony\Component\DependencyInjection\Compiler\AutowirePass;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
+
+class AutowireExceptionPassTest extends TestCase
+{
+    public function testThrowsException()
+    {
+        $autowirePass = $this->getMockBuilder(AutowirePass::class)
+            ->getMock();
+
+        $autowireException = new AutowiringFailedException('foo_service_id', 'An autowiring exception message');
+        $autowirePass->expects($this->any())
+            ->method('getAutowiringExceptions')
+            ->will($this->returnValue(array($autowireException)));
+
+        $container = new ContainerBuilder();
+        $container->register('foo_service_id');
+
+        $pass = new AutowireExceptionPass($autowirePass);
+
+        try {
+            $pass->process($container);
+            $this->fail('->process() should throw the exception if the service id exists');
+        } catch (\Exception $e) {
+            $this->assertSame($autowireException, $e);
+        }
+    }
+
+    public function testNoExceptionIfServiceRemoved()
+    {
+        $autowirePass = $this->getMockBuilder(AutowirePass::class)
+            ->getMock();
+
+        $autowireException = new AutowiringFailedException('non_existent_service');
+        $autowirePass->expects($this->any())
+            ->method('getAutowiringExceptions')
+            ->will($this->returnValue(array($autowireException)));
+
+        $container = new ContainerBuilder();
+
+        $pass = new AutowireExceptionPass($autowirePass);
+
+        $pass->process($container);
+        // mark the test as passed
+        $this->assertTrue(true);
+    }
+}

src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php

@@ -135,6 +135,21 @@ class AutowirePassTest extends TestCase
         $this->assertEquals(DInterface::class, (string) $container->getDefinition('h')->getArgument(1));
     }
 
+    public function testExceptionsAreStored()
+    {
+        $container = new ContainerBuilder();
+
+        $container->register('c1', __NAMESPACE__.'\CollisionA');
+        $container->register('c2', __NAMESPACE__.'\CollisionB');
+        $container->register('c3', __NAMESPACE__.'\CollisionB');
+        $aDefinition = $container->register('a', __NAMESPACE__.'\CannotBeAutowired');
+        $aDefinition->setAutowired(true);
+
+        $pass = new AutowirePass(false);
+        $pass->process($container);
+        $this->assertCount(1, $pass->getAutowiringExceptions());
+    }
+
     /**
      * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
      * @expectedExceptionMessage Cannot autowire service "a": argument "$collision" of method "Symfony\Component\DependencyInjection\Tests\Compiler\CannotBeAutowired::__construct()" references interface "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" but no such service exists. You should maybe alias this interface to one of these existing services: "c1", "c2", "c3".