Adding Definition::addError() and a compiler pass to throw errors as exceptions

Author: weaverryan

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

@@ -22,6 +22,9 @@
  */
 abstract class AbstractRecursivePass implements CompilerPassInterface
 {
+    /**
+     * @var ContainerBuilder
+     */
     protected $container;
     protected $currentId;
 

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

@@ -11,11 +11,15 @@
 
 namespace Symfony\Component\DependencyInjection\Compiler;
 
+@trigger_error('The '.__NAMESPACE__.'\AutowireExceptionPass class is deprecated since version 3.4 and will be removed in 4.0. Use the DefinitionErrorExceptionPass class instead.', E_USER_DEPRECATED);
+
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 
 /**
  * Throws autowire exceptions from AutowirePass for definitions that still exist.
  *
+ * @deprecated since version 3.4, will be removed in 4.0.
+ *
  * @author Ryan Weaver <[email protected]>
  */
 class AutowireExceptionPass implements CompilerPassInterface

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

@@ -36,18 +36,22 @@ class AutowirePass extends AbstractRecursivePass
     private $autowiringExceptions = array();
 
     /**
-     * @param bool $throwOnAutowireException If false, retrieved errors via getAutowiringExceptions
+     * @param bool $throwOnAutowireException Errors can be retrieved via Definition::getErrors()
      */
     public function __construct($throwOnAutowireException = true)
     {
         $this->throwOnAutowiringException = $throwOnAutowireException;
     }
 
     /**
+     * @deprecated since version 3.4, to be removed in 4.0.
+     *
      * @return AutowiringFailedException[]
      */
     public function getAutowiringExceptions()
     {
+        @trigger_error('Calling AutowirePass::getAutowiringExceptions() is deprecated since Symfony 3.4 and will be removed in 4.0. Use Definition::getErrors() instead.', E_USER_DEPRECATED);
+
         return $this->autowiringExceptions;
     }
 
@@ -106,6 +110,7 @@ protected function processValue($value, $isRoot = false)
             }
 
             $this->autowiringExceptions[] = $e;
+            $this->container->getDefinition($this->currentId)->addError($e->getMessage());
 
             return parent::processValue($value, $isRoot);
         }

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

@@ -22,6 +22,13 @@
  */
 class CheckArgumentsValidityPass extends AbstractRecursivePass
 {
+    private $throwExceptions;
+
+    public function __construct($throwExceptions = true)
+    {
+        $this->throwExceptions = $throwExceptions;
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -35,10 +42,20 @@ protected function processValue($value, $isRoot = false)
         foreach ($value->getArguments() as $k => $v) {
             if ($k !== $i++) {
                 if (!is_int($k)) {
-                    throw new RuntimeException(sprintf('Invalid constructor argument for service "%s": integer expected but found string "%s". Check your service definition.', $this->currentId, $k));
+                    $msg = sprintf('Invalid constructor argument for service "%s": integer expected but found string "%s". Check your service definition.', $this->currentId, $k);
+                    $value->addError($msg);
+                    if ($this->throwExceptions) {
+                        throw new RuntimeException($msg);
+                    }
+
+                    break;
                 }
 
-                throw new RuntimeException(sprintf('Invalid constructor argument %d for service "%s": argument %d must be defined before. Check your service definition.', 1 + $k, $this->currentId, $i));
+                $msg = sprintf('Invalid constructor argument %d for service "%s": argument %d must be defined before. Check your service definition.', 1 + $k, $this->currentId, $i);
+                $value->addError($msg);
+                if ($this->throwExceptions) {
+                    throw new RuntimeException($msg);
+                }
             }
         }
 
@@ -47,10 +64,20 @@ protected function processValue($value, $isRoot = false)
             foreach ($methodCall[1] as $k => $v) {
                 if ($k !== $i++) {
                     if (!is_int($k)) {
-                        throw new RuntimeException(sprintf('Invalid argument for method call "%s" of service "%s": integer expected but found string "%s". Check your service definition.', $methodCall[0], $this->currentId, $k));
+                        $msg = sprintf('Invalid argument for method call "%s" of service "%s": integer expected but found string "%s". Check your service definition.', $methodCall[0], $this->currentId, $k);
+                        $value->addError($msg);
+                        if ($this->throwExceptions) {
+                            throw new RuntimeException($msg);
+                        }
+
+                        break;
                     }
 
-                    throw new RuntimeException(sprintf('Invalid argument %d for method call "%s" of service "%s": argument %d must be defined before. Check your service definition.', 1 + $k, $methodCall[0], $this->currentId, $i));
+                    $msg = sprintf('Invalid argument %d for method call "%s" of service "%s": argument %d must be defined before. Check your service definition.', 1 + $k, $methodCall[0], $this->currentId, $i);
+                    $value->addError($msg);
+                    if ($this->throwExceptions) {
+                        throw new RuntimeException($msg);
+                    }
                 }
             }
         }

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

@@ -0,0 +1,39 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * 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\Definition;
+use Symfony\Component\DependencyInjection\Exception\RuntimeException;
+
+/**
+ * Throws an exception for any Definitions that have errors and still exist.
+ *
+ * @author Ryan Weaver <[email protected]>
+ */
+class DefinitionErrorExceptionPass extends AbstractRecursivePass
+{
+    /**
+     * {@inheritdoc}
+     */
+    protected function processValue($value, $isRoot = false)
+    {
+        if (!$value instanceof Definition || empty($value->getErrors())) {
+            return parent::processValue($value, $isRoot);
+        }
+
+        // only show the first error so they user can focus on it
+        $errors = $value->getErrors();
+        $message = reset($errors);
+
+        throw new RuntimeException($message);
+    }
+}

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

@@ -38,10 +38,14 @@ public function setRepeatedPass(RepeatedPass $repeatedPass)
      *
      * The key is the inlined service id and its value is the list of services it was inlined into.
      *
+     * @deprecated since version 3.4, to be removed in 4.0.
+     *
      * @return array
      */
     public function getInlinedServiceIds()
     {
+        @trigger_error('Calling InlineServiceDefinitionsPass::getInlinedServiceIds() is deprecated since Symfony 3.4 and will be removed in 4.0.', E_USER_DEPRECATED);
+
         return $this->inlinedServiceIds;
     }
 

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

@@ -59,14 +59,14 @@ public function __construct()
             new RegisterServiceSubscribersPass(),
             new ResolveNamedArgumentsPass(),
             new ResolveBindingsPass(),
-            $autowirePass = new AutowirePass(false),
+            new AutowirePass(false),
             new ResolveServiceSubscribersPass(),
             new ResolveReferencesToAliasesPass(),
             new ResolveInvalidReferencesPass(),
             new AnalyzeServiceReferencesPass(true),
             new CheckCircularReferencesPass(),
             new CheckReferenceValidityPass(),
-            new CheckArgumentsValidityPass(),
+            new CheckArgumentsValidityPass(false),
         ));
 
         $this->removingPasses = array(array(
@@ -75,11 +75,11 @@ public function __construct()
             new RemoveAbstractDefinitionsPass(),
             new RepeatedPass(array(
                 new AnalyzeServiceReferencesPass(),
-                $inlinedServicePass = new InlineServiceDefinitionsPass(),
+                new InlineServiceDefinitionsPass(),
                 new AnalyzeServiceReferencesPass(),
                 new RemoveUnusedDefinitionsPass(),
             )),
-            new AutowireExceptionPass($autowirePass, $inlinedServicePass),
+            new DefinitionErrorExceptionPass(),
             new CheckExceptionOnInvalidReferenceBehaviorPass(),
         ));
     }

src/Symfony/Component/DependencyInjection/Definition.php

@@ -44,6 +44,7 @@ class Definition
     private $autowiringTypes = array();
     private $changes = array();
     private $bindings = array();
+    private $errors = array();
 
     protected $arguments = array();
 
@@ -959,4 +960,24 @@ public function setBindings(array $bindings)
 
         return $this;
     }
+
+    /**
+     * Add an error that occurred when building this Definition.
+     *
+     * @param string $error
+     */
+    public function addError($error)
+    {
+        $this->errors[] = $error;
+    }
+
+    /**
+     * Returns any errors that occurred while building this Definition.
+     *
+     * @return array
+     */
+    public function getErrors()
+    {
+        return $this->errors;
+    }
 }

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

@@ -18,6 +18,9 @@
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
 
+/**
+ * @group legacy
+ */
 class AutowireExceptionPassTest extends TestCase
 {
     public function testThrowsException()

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

@@ -157,6 +157,9 @@ public function testCompleteExistingDefinitionWithNotDefinedArguments()
         $this->assertEquals(DInterface::class, (string) $container->getDefinition('h')->getArgument(1));
     }
 
+    /**
+     * @group legacy
+     */
     public function testExceptionsAreStored()
     {
         $container = new ContainerBuilder();

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

@@ -64,4 +64,15 @@ public function definitionProvider()
             array(array(), array(array('baz', array(1 => 1)))),
         );
     }
+
+    public function testNoException()
+    {
+        $container = new ContainerBuilder();
+        $definition = $container->register('foo');
+        $definition->setArguments(array(null, 'a' => 'a'));
+
+        $pass = new CheckArgumentsValidityPass(false);
+        $pass->process($container);
+        $this->assertCount(1, $definition->getErrors());
+    }
 }

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

@@ -0,0 +1,53 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * 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\DefinitionErrorExceptionPass;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+
+class DefinitionErrorExceptionPassTest extends TestCase
+{
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
+     * @expectedExceptionMessage Things went wrong!
+     */
+    public function testThrowsException()
+    {
+        $container = new ContainerBuilder();
+        $def = new Definition();
+        $def->addError('Things went wrong!');
+        $def->addError('Now something else!');
+        $container->register('foo_service_id')
+            ->setArguments(array(
+                $def,
+            ));
+
+        $pass = new DefinitionErrorExceptionPass();
+        $pass->process($container);
+    }
+
+    public function testNoExceptionThrown()
+    {
+        $container = new ContainerBuilder();
+        $def = new Definition();
+        $container->register('foo_service_id')
+            ->setArguments(array(
+                $def,
+            ));
+
+        $pass = new DefinitionErrorExceptionPass();
+        $pass->process($container);
+        $this->assertSame($def, $container->getDefinition('foo_service_id')->getArgument(0));
+    }
+}

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

@@ -252,6 +252,9 @@ public function testProcessDoesNotSetLazyArgumentValuesAfterInlining()
         $this->assertSame('inline', (string) $values[0]);
     }
 
+    /**
+     * @group legacy
+     */
     public function testGetInlinedServiceIdData()
     {
         $container = new ContainerBuilder();

src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php

@@ -387,4 +387,13 @@ public function testShouldAutoconfigure()
         $def->setAutoconfigured(true);
         $this->assertTrue($def->isAutoconfigured());
     }
+
+    public function testAddError()
+    {
+        $def = new Definition('stdClass');
+        $this->assertEmpty($def->getErrors());
+        $def->addError('First error');
+        $def->addError('Second error');
+        $this->assertSame(array('First error', 'Second error'), $def->getErrors());
+    }
 }