From a85b37a0e68e4a5b23113f41f79d6a88b53a61b8 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 22 Sep 2017 13:54:05 -0400 Subject: [PATCH] Adding Definition::addError() and a compiler pass to throw errors as exceptions --- .../Compiler/AbstractRecursivePass.php | 3 ++ .../Compiler/AutowireExceptionPass.php | 4 ++ .../Compiler/AutowirePass.php | 7 ++- .../Compiler/CheckArgumentsValidityPass.php | 35 ++++++++++-- .../Compiler/DefinitionErrorExceptionPass.php | 39 ++++++++++++++ .../Compiler/InlineServiceDefinitionsPass.php | 4 ++ .../Compiler/PassConfig.php | 8 +-- .../DependencyInjection/Definition.php | 21 ++++++++ .../Compiler/AutowireExceptionPassTest.php | 3 ++ .../Tests/Compiler/AutowirePassTest.php | 3 ++ .../CheckArgumentsValidityPassTest.php | 11 ++++ .../DefinitionErrorExceptionPassTest.php | 53 +++++++++++++++++++ .../InlineServiceDefinitionsPassTest.php | 3 ++ .../Tests/DefinitionTest.php | 9 ++++ 14 files changed, 194 insertions(+), 9 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Compiler/DefinitionErrorExceptionPass.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Compiler/DefinitionErrorExceptionPassTest.php diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php index bbe869b935..0851d33e7e 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php @@ -22,6 +22,9 @@ use Symfony\Component\DependencyInjection\Reference; */ abstract class AbstractRecursivePass implements CompilerPassInterface { + /** + * @var ContainerBuilder + */ protected $container; protected $currentId; diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowireExceptionPass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowireExceptionPass.php index 12be3d915f..f01617dd63 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowireExceptionPass.php +++ b/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 */ class AutowireExceptionPass implements CompilerPassInterface diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php index 8cb1978ef6..607b70c85f 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php @@ -36,7 +36,7 @@ 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) { @@ -44,10 +44,14 @@ class AutowirePass extends AbstractRecursivePass } /** + * @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 @@ class AutowirePass extends AbstractRecursivePass } $this->autowiringExceptions[] = $e; + $this->container->getDefinition($this->currentId)->addError($e->getMessage()); return parent::processValue($value, $isRoot); } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckArgumentsValidityPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckArgumentsValidityPass.php index 6b48a15691..7f032058ab 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/CheckArgumentsValidityPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckArgumentsValidityPass.php @@ -22,6 +22,13 @@ use Symfony\Component\DependencyInjection\Exception\RuntimeException; */ class CheckArgumentsValidityPass extends AbstractRecursivePass { + private $throwExceptions; + + public function __construct($throwExceptions = true) + { + $this->throwExceptions = $throwExceptions; + } + /** * {@inheritdoc} */ @@ -35,10 +42,20 @@ class CheckArgumentsValidityPass extends AbstractRecursivePass 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 @@ class CheckArgumentsValidityPass extends AbstractRecursivePass 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); + } } } } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/DefinitionErrorExceptionPass.php b/src/Symfony/Component/DependencyInjection/Compiler/DefinitionErrorExceptionPass.php new file mode 100644 index 0000000000..69cd899f2a --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Compiler/DefinitionErrorExceptionPass.php @@ -0,0 +1,39 @@ + + * + * 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 + */ +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); + } +} diff --git a/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php index 73f75b30a9..05e7786798 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php @@ -38,10 +38,14 @@ class InlineServiceDefinitionsPass extends AbstractRecursivePass implements Repe * * 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; } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php index 79792bee4a..1edc4c21b1 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php @@ -59,14 +59,14 @@ class PassConfig 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 @@ class PassConfig new RemoveAbstractDefinitionsPass(), new RepeatedPass(array( new AnalyzeServiceReferencesPass(), - $inlinedServicePass = new InlineServiceDefinitionsPass(), + new InlineServiceDefinitionsPass(), new AnalyzeServiceReferencesPass(), new RemoveUnusedDefinitionsPass(), )), - new AutowireExceptionPass($autowirePass, $inlinedServicePass), + new DefinitionErrorExceptionPass(), new CheckExceptionOnInvalidReferenceBehaviorPass(), )); } diff --git a/src/Symfony/Component/DependencyInjection/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php index 34d6f46cac..464932fc0a 100644 --- a/src/Symfony/Component/DependencyInjection/Definition.php +++ b/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 @@ class Definition 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; + } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowireExceptionPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowireExceptionPassTest.php index 4f0b3d6c25..a9c3445cef 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowireExceptionPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowireExceptionPassTest.php @@ -18,6 +18,9 @@ use Symfony\Component\DependencyInjection\Compiler\InlineServiceDefinitionsPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException; +/** + * @group legacy + */ class AutowireExceptionPassTest extends TestCase { public function testThrowsException() diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php index 94666df608..3a48cfae8d 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php @@ -157,6 +157,9 @@ class AutowirePassTest extends TestCase $this->assertEquals(DInterface::class, (string) $container->getDefinition('h')->getArgument(1)); } + /** + * @group legacy + */ public function testExceptionsAreStored() { $container = new ContainerBuilder(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckArgumentsValidityPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckArgumentsValidityPassTest.php index 891acbeee0..d121689ff9 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckArgumentsValidityPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckArgumentsValidityPassTest.php @@ -64,4 +64,15 @@ class CheckArgumentsValidityPassTest extends TestCase 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()); + } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/DefinitionErrorExceptionPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/DefinitionErrorExceptionPassTest.php new file mode 100644 index 0000000000..e0585e2132 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/DefinitionErrorExceptionPassTest.php @@ -0,0 +1,53 @@ + + * + * 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)); + } +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/InlineServiceDefinitionsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/InlineServiceDefinitionsPassTest.php index 26b24fa713..6154080c8b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/InlineServiceDefinitionsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/InlineServiceDefinitionsPassTest.php @@ -252,6 +252,9 @@ class InlineServiceDefinitionsPassTest extends TestCase $this->assertSame('inline', (string) $values[0]); } + /** + * @group legacy + */ public function testGetInlinedServiceIdData() { $container = new ContainerBuilder(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php b/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php index 2821dc17c0..5fe4236ad9 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php @@ -387,4 +387,13 @@ class DefinitionTest extends TestCase $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()); + } }