8000 feature #24290 Adding Definition::addError() and a compiler pass to t… · Basster/symfony@8136fa5 · GitHub
[go: up one dir, main page]
Skip to content

Commit 8136fa5

Browse files
nicolas-grekasnicolas-grekas
committed
feature symfony#24290 Adding Definition::addError() and a compiler pass to throw errors as exceptions (weaverryan)
This PR was squashed before being merged into the 3.4 branch (closes symfony#24290). Discussion ---------- Adding Definition::addError() and a compiler pass to throw errors as exceptions | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes & no | New feature? | yes | BC breaks? | no | Deprecations? | yes (very minor) | Tests pass? | yes | Fixed tickets | symfony#23606 | License | MIT | Doc PR | Not needed Hi guys! Very simple: when there is an error with a Definition, we can now call `Definition::addError()` instead of throwing an exception. Then, a new compiler pass (after removal) actually throws an exception. The advantage is that we can avoid throwing exceptions for services that are ultimately removed from the container. That's important for auto-registration, where we commonly register all services in `src/`... but then many of them are removed later. A few interesting notes: - We can probably convert more things from exceptions to `Definition::addError()`. I've only converted autowiring errors and things in `CheckArgumentsValidityPass` (that was necessary because it was throwing exceptions in some cases due to autowiring failing... which was the true error) - `Definition` can hold multiple errors, but I'm only showing the first error in the exception message. The reason is clarity: I think usually the first error is the most (or only) important. But having `Definition::addError()` avoids the possibility of a later error overriding an earlier one Cheers! Commits ------- a85b37a Adding Definition::addError() and a compiler pass to throw errors as exceptions
2 parents ec11187 + a85b37a commit 8136fa5

14 files changed

+194
-9
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
*/
2323
abstract class AbstractRecursivePass implements CompilerPassInterface
2424
{
25+
/**
26+
* @var ContainerBuilder
27+
*/
2528
protected $container;
2629
protected $currentId;
2730

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,15 @@
1111

1212
namespace Symfony\Component\DependencyInjection\Compiler;
1313

14+
@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);
15+
1416
use Symfony\Component\DependencyInjection\ContainerBuilder;
1517

1618
/**
1719
* Throws autowire exceptions from AutowirePass for definitions that still exist.
1820
*
21+
* @deprecated since version 3.4, will be removed in 4.0.
22+
*
1923
* @author Ryan Weaver <ryan@knpuniversity.com>
2024
*/
2125
class AutowireExceptionPass implements CompilerPassInterface

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,22 @@ class AutowirePass extends AbstractRecursivePass
3636
private $autowiringExceptions = array();
3737

3838
/**
39-
* @param bool $throwOnAutowireException If false, retrieved errors via getAutowiringExceptions
39+
* @param bool $throwOnAutowireException Errors can be retrieved via Definition::getErrors()
4040
*/
4141
public function __construct($throwOnAutowireException = true)
4242
{
4343
$this->throwOnAutowiringException = $throwOnAutowireException;
4444
}
4545

4646
/**
47+
* @deprecated since version 3.4, to be removed in 4.0.
48+
*
4749
* @return AutowiringFailedException[]
4850
*/
4951
public function getAutowiringExceptions()
5052
{
53+
@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);
54+
5155
return $this->autowiringExceptions;
5256
}
5357

@@ -106,6 +110,7 @@ protected function processValue($value, $isRoot = false)
106110
}
107111

108112
$this->autowiringExceptions[] = $e;
113+
$this->container->getDefinition($this->currentId)->addError($e->getMessage());
109114

110115
return parent::processValue($value, $isRoot);
111116
}

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

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@
2222
*/
2323
class CheckArgumentsValidityPass extends AbstractRecursivePass
2424
{
25+
private $throwExceptions;
26+
27+
public function __construct($throwExceptions = true)
28+
{
29+
$this->throwExceptions = $throwExceptions;
30+
}
31+
2532
/**
2633
* {@inheritdoc}
2734
*/
@@ -35,10 +42,20 @@ protected function processValue($value, $isRoot = false)
3542
foreach ($value->getArguments() as $k => $v) {
3643
if ($k !== $i++) {
3744
if (!is_int($k)) {
38-
throw new RuntimeException(sprintf('Invalid constructor argument for service "%s": integer expected but found string "%s". Check your service definition.', $this->currentId, $k));
45+
$msg = sprintf('Invalid constructor argument for service "%s": integer expected but found string "%s". Check your service definition.', $this->currentId, $k);
46+
$value->addError($msg);
47+
if ($this->throwExceptions) {
48+
throw new RuntimeException($msg);
49+
}
50+
51+
break;
3952
}
4053

41-
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));
54+
$msg = sprintf('Invalid constructor argument %d for service "%s": argument %d must be defined before. Check your service definition.', 1 + $k, $this->currentId, $i);
55+
$value->addError($msg);
56+
if ($this->throwExceptions) {
57+
throw new RuntimeException($msg);
58+
}
4259
}
4360
}
4461

@@ -47,10 +64,20 @@ protected function processValue($value, $isRoot = false)
4764
foreach ($methodCall[1] as $k => $v) {
4865
if ($k !== $i++) {
4966
if (!is_int($k)) {
50-
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));
67+
$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);
68+
$value->addError($msg);
69+
if ($this->throwExceptions) {
70+
throw new RuntimeException($msg);
71+
}
72+
73+
break;
5174
}
5275

53-
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));
76+
$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);
77+
$value->addError($msg);
78+
if ($this->throwExceptions) {
79+
throw new RuntimeException($msg);
80+
}
5481
}
5582
}
5683
}

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\DependencyInjection\Compiler;
13+
14+
use Symfony\Component\DependencyInjection\Definition;
15+
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
16+
17+
/**
18+
* Throws an exception for any Definitions that have errors and still exist.
19+
*
20+
* @author Ryan Weaver <ryan@knpuniversity.com>
21+
*/
22+
class DefinitionErrorExceptionPass extends AbstractRecursivePass
23+
{
24+
/**
25+
* {@inheritdoc}
26+
*/
27+
protected function processValue($value, $isRoot = false)
28+
{
29+
if (!$value instanceof Definition || empty($value->getErrors())) {
30+
return parent::processValue($value, $isRoot);
31+
}
32+
33+
// only show the first error so they user can focus on it
34+
$errors = $value->getErrors();
35+
$message = reset($errors);
36+
37+
throw new RuntimeException($message);
38+
}
39+
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,14 @@ public function setRepeatedPass(RepeatedPass $repeatedPass)
3838
*
3939
* The key is the inlined service id and its value is the list of services it was inlined into.
4040
*
41+
* @deprecated since version 3.4, to be removed in 4.0.
42+
*
4143
* @return array
4244
*/
4345
public function getInlinedServiceIds()
4446
{
47+
@trigger_error('Calling InlineServiceDefinitionsPass::getInlinedServiceIds() is deprecated since Symfony 3.4 and will be removed in 4.0.', E_USER_DEPRECATED);
48+
4549
return $this->inlinedServiceIds;
4650
}
4751

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,14 @@ public function __construct()
6060
new ResolveNamedArgumentsPass(),
6161
new AutowireRequiredMethodsPass(),
6262
new ResolveBindingsPass(),
63-
$autowirePass = new AutowirePass(false),
63+
new AutowirePass(false),
6464
new ResolveServiceSubscribersPass(),
6565
new ResolveReferencesToAliasesPass(),
6666
new ResolveInvalidReferencesPass(),
6767
new AnalyzeServiceReferencesPass(true),
6868
new CheckCircularReferencesPass(),
6969
new CheckReferenceValidityPass(),
70-
new CheckArgumentsValidityPass(),
70+
new CheckArgumentsValidityPass(false),
7171
));
7272

7373
$this->removingPasses = array(array(
@@ -76,11 +76,11 @@ public function __construct()
7676
new RemoveAbstractDefinitionsPass(),
7777
new RepeatedPass(array(
7878
new AnalyzeServiceReferencesPass(),
79-
$inlinedServicePass = new InlineServiceDefinitionsPass(),
79+
new InlineServiceDefinitionsPass(),
8080
new AnalyzeServiceReferencesPass(),
8181
new RemoveUnusedDefinitionsPass(),
8282
)),
83-
new AutowireExceptionPass($autowirePass, $inlinedServicePass),
83+
new DefinitionErrorExceptionPass(),
8484
new CheckExceptionOnInvalidReferenceBehaviorPass(),
8585
));
8686
}

‎src/Symfony/Component/DependencyInjection/Definition.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class Definition
4444
private $autowiringTypes = array();
4545
private $changes = array();
4646
private $bindings = array();
47+
private $errors = array();
4748

4849
protected $arguments = array();
4950

@@ -959,4 +960,24 @@ public function setBindings(array $bindings)
959960

960961
return $this;
961962
}
963+
964+
/**
965+
* Add an error that occurred when building this Definition.
966+
*
967+
* @param string $error
968+
*/
969+
public function addError($error)
970+
{
971+
$this->errors[] = $error;
972+
}
973+
974+
/**
975+
* Returns any errors that occurred while building this Definition.
976+
*
977+
* @return array
978+
*/
979+
public function getErrors()
980+
{
981+
return $this->errors;
982+
}
962983
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
use Symfony\Component\DependencyInjection\ContainerBuilder;
1919
use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
2020

21+
/**
22+
* @group legacy
23+
*/
2124
class AutowireExceptionPassTest extends TestCase
2225
{
2326
public function testThrowsException()

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,9 @@ public function testCompleteExistingDefinitionWithNotDefinedArguments()
160160
$this->assertEquals(DInterface::class, (string) $container->getDefinition('h')->getArgument(1));
161161
}
162162

163+
/**
164+
* @group legacy
165+
*/
163166
public function testExceptionsAreStored()
164167
{
165168
$container = new ContainerBuilder();

0 commit comments

Comments
 (0)
0