Skip to content

Commit ceeb4e6

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '6.3' into 6.4
* 6.3: [FrameworkBundle] Fixed parsing new JSON output of debug:config not possible [DependencyInjection] Skip errored definitions deep-referenced as runtime exceptions [AssetMapper] Allow DirectoryResource for cache [PhpUnitBridge] Require PHPUnit 9.6 by default [WebProfilerBundle] Fix the accessibility of some background color [HttpKernel] Nullable and default value arguments in RequestPayloadValueResolver [WebProfilerBundle] right blocks: fix display [Messenger] Preserve existing Doctrine schema [Serializer] Refactor tests to not extends ObjectNormalizer [HttpFoundation] Make Request::getPayload() return an empty InputBag if request body is empty [HttpClient] Explicitly exclude CURLOPT_POSTREDIR [FrameworkBundle] Fix setting decorated services during tests
2 parents 965d2e7 + 7abf242 commit ceeb4e6

File tree

6 files changed

+121
-28
lines changed

6 files changed

+121
-28
lines changed

‎Compiler/DecoratorServicePass.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public function process(ContainerBuilder $container)
4242
$definitions->insert([$id, $definition], [$decorated[2], --$order]);
4343
}
4444
$decoratingDefinitions = [];
45+
$decoratedIds = [];
4546

4647
$tagsToKeep = $container->hasParameter('container.behavior_describing_tags')
4748
? $container->getParameter('container.behavior_describing_tags')
@@ -58,6 +59,7 @@ public function process(ContainerBuilder $container)
5859
$renamedId = $id.'.inner';
5960
}
6061

62+
$decoratedIds[$inner] ??= $renamedId;
6163
$this->currentId = $renamedId;
6264
$this->processValue($definition);
6365

@@ -114,7 +116,7 @@ public function process(ContainerBuilder $container)
114116
}
115117

116118
foreach ($decoratingDefinitions as $inner => $definition) {
117-
$definition->addTag('container.decorator', ['id' => $inner]);
119+
$definition->addTag('container.decorator', ['id' => $inner, 'inner' => $decoratedIds[$inner]]);
118120
}
119121
}
120122

‎Compiler/DefinitionErrorExceptionPass.php

Lines changed: 87 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\DependencyInjection\Compiler;
1313

14+
use Symfony\Component\DependencyInjection\Argument\ArgumentInterface;
15+
use Symfony\Component\DependencyInjection\ContainerBuilder;
1416
use Symfony\Component\DependencyInjection\ContainerInterface;
1517
use Symfony\Component\DependencyInjection\Definition;
1618
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
@@ -23,31 +25,98 @@
2325
*/
2426
class DefinitionErrorExceptionPass extends AbstractRecursivePass
2527
{
26-
protected function processValue(mixed $value, bool $isRoot = false): mixed
28+
private array $erroredDefinitions = [];
29+
private array $targetReferences = [];
30+
private array $sourceReferences = [];
31+
32+
/**
33+
* @return void
34+
*/
35+
public function process(ContainerBuilder $container)
2736
{
28-
if (!$value instanceof Definition || !$value->hasErrors() || $value->hasTag('container.error')) {
29-
return parent::processValue($value, $isRoot);
30-
}
37+
try {
38+
parent::process($container);
39+
40+
if (!$this->erroredDefinitions) {
41+
return;
42+
}
43+
44+
$runtimeIds = [];
45+
46+
foreach ($this->sourceReferences as $id => $sourceIds) {
47+
foreach ($sourceIds as $sourceId => $isRuntime) {
48+
if (!$isRuntime) {
49+
continue 2;
50+
}
51+
}
52+
53+
unset($this->erroredDefinitions[$id]);
54+
$runtimeIds[$id] = $id;
55+
}
56+
57+
if (!$this->erroredDefinitions) {
58+
return;
59+
}
60+
61+
foreach ($this->targetReferences as $id => $targetIds) {
62+
if (!isset($this->sourceReferences[$id]) || isset($runtimeIds[$id]) || isset($this->erroredDefinitions[$id])) {
63+
continue;
64+
}
65+
foreach ($this->targetReferences[$id] as $targetId => $isRuntime) {
66+
foreach ($this->sourceReferences[$id] as $sourceId => $isRuntime) {
67+
if ($sourceId !== $targetId) {
68+
$this->sourceReferences[$targetId][$sourceId] = false;
69+
$this->targetReferences[$sourceId][$targetId] = false;
70+
}
71+
}
72+
}
3173

32-
if ($isRoot && !$value->isPublic()) {
33-
$graph = $this->container->getCompiler()->getServiceReferenceGraph();
34-
$runtimeException = false;
35-
foreach ($graph->getNode($this->currentId)->getInEdges() as $edge) {
36-
if (!$edge->getValue() instanceof Reference || ContainerInterface::RUNTIME_EXCEPTION_ON_INVALID_REFERENCE !== $edge->getValue()->getInvalidBehavior()) {
37-
$runtimeException = false;
38-
break;
74+
unset($this->sourceReferences[$id]);
75+
}
76+
77+
foreach ($this->erroredDefinitions as $id => $definition) {
78+
if (isset($this->sourceReferences[$id])) {
79+
continue;
3980
}
40-
$runtimeException = true;
81+
82+
// only show the first error so the user can focus on it
83+
$errors = $definition->getErrors();
84+
85+
throw new RuntimeException(reset($errors));
4186
}
42-
if ($runtimeException) {
43-
return parent::processValue($value, $isRoot);
87+
} finally {
88+
$this->erroredDefinitions = [];
89+
$this->targetReferences = [];
90+
$this->sourceReferences = [];
91+
}
92+
}
93+
94+
protected function processValue(mixed $value, bool $isRoot = false): mixed
95+
{
96+
if ($value instanceof ArgumentInterface) {
97+
parent::processValue($value->getValues());
98+
99+
return $value;
100+
}
101+
102+
if ($value instanceof Reference && $this->currentId !== $targetId = (string) $value) {
103+
if (ContainerInterface::RUNTIME_EXCEPTION_ON_INVALID_REFERENCE === $value->getInvalidBehavior()) {
104+
$this->sourceReferences[$targetId][$this->currentId] ??= true;
105+
$this->targetReferences[$this->currentId][$targetId] ??= true;
106+
} else {
107+
$this->sourceReferences[$targetId][$this->currentId] = false;
108+
$this->targetReferences[$this->currentId][$targetId] = false;
44109
}
110+
111+
return $value;
112+
}
113+
114+
if (!$value instanceof Definition || !$value->hasErrors() || $value->hasTag('container.error')) {
115+
return parent::processValue($value, $isRoot);
45116
}
46117

47-
// only show the first error so the user can focus on it
48-
$errors = $value->getErrors();
49-
$message = reset($errors);
118+
$this->erroredDefinitions[$this->currentId] = $value;
50119

51-
throw new RuntimeException($message);
120+
return parent::processValue($value);
52121
}
53122
}

‎Tests/Compiler/DecoratorServicePassTest.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ public function testProcessWithInvalidDecorated()
161161
public function testProcessNoInnerAliasWithInvalidDecorated()
162162
{
163163
$container = new ContainerBuilder();
164-
$decoratorDefinition = $container
164+
$container
165165
->register('decorator')
166166
->setDecoratedService('unknown_decorated', null, 0, ContainerInterface::NULL_ON_INVALID_REFERENCE)
167167
;
@@ -173,7 +173,7 @@ public function testProcessNoInnerAliasWithInvalidDecorated()
173173
public function testProcessWithInvalidDecoratedAndWrongBehavior()
174174
{
175175
$container = new ContainerBuilder();
176-
$decoratorDefinition = $container
176+
$container
177177
->register('decorator')
178178
->setDecoratedService('unknown_decorated', null, 0, 12)
179179
;
@@ -198,7 +198,7 @@ public function testProcessMovesTagsFromDecoratedDefinitionToDecoratingDefinitio
198198
$this->process($container);
199199

200200
$this->assertEmpty($container->getDefinition('baz.inner')->getTags());
201-
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar'], 'container.decorator' => [['id' => 'foo']]], $container->getDefinition('baz')->getTags());
201+
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar'], 'container.decorator' => [['id' => 'foo', 'inner' => 'baz.inner']]], $container->getDefinition('baz')->getTags());
202202
}
203203

204204
public function testProcessMovesTagsFromDecoratedDefinitionToDecoratingDefinitionMultipleTimes()
@@ -221,7 +221,7 @@ public function testProcessMovesTagsFromDecoratedDefinitionToDecoratingDefinitio
221221
$this->process($container);
222222

223223
$this->assertEmpty($container->getDefinition('deco1')->getTags());
224-
$this->assertEquals(['bar' => ['attr' => 'baz'], 'container.decorator' => [['id' => 'foo']]], $container->getDefinition('deco2')->getTags());
224+
$this->assertEquals(['bar' => ['attr' => 'baz'], 'container.decorator' => [['id' => 'foo', 'inner' => 'deco1.inner']]], $container->getDefinition('deco2')->getTags());
225225
}
226226

227227
public function testProcessLeavesServiceLocatorTagOnOriginalDefinition()
@@ -240,7 +240,7 @@ public function testProcessLeavesServiceLocatorTagOnOriginalDefinition()
240240
$this->process($container);
241241

242242
$this->assertEquals(['container.service_locator' => [0 => []]], $container->getDefinition('baz.inner')->getTags());
243-
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar'], 'container.decorator' => [['id' => 'foo']]], $container->getDefinition('baz')->getTags());
243+
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar'], 'container.decorator' => [['id' => 'foo', 'inner' => 'baz.inner']]], $container->getDefinition('baz')->getTags());
244244
}
245245

246246
public function testProcessLeavesServiceSubscriberTagOnOriginalDefinition()
@@ -259,7 +259,7 @@ public function testProcessLeavesServiceSubscriberTagOnOriginalDefinition()
259259
$this->process($container);
260260

261261
$this->assertEquals(['container.service_subscriber' => [], 'container.service_subscriber.locator' => []], $container->getDefinition('baz.inner')->getTags());
262-
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar'], 'container.decorator' => [['id' => 'foo']]], $container->getDefinition('baz')->getTags());
262+
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar'], 'container.decorator' => [['id' => 'foo', 'inner' => 'baz.inner']]], $container->getDefinition('baz')->getTags());
263263
}
264264

265265
public function testProcessLeavesProxyTagOnOriginalDefinition()
@@ -278,7 +278,7 @@ public function testProcessLeavesProxyTagOnOriginalDefinition()
278278
$this->process($container);
279279

280280
$this->assertEquals(['proxy' => 'foo'], $container->getDefinition('baz.inner')->getTags());
281-
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar'], 'container.decorator' => [['id' => 'foo']]], $container->getDefinition('baz')->getTags());
281+
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar'], 'container.decorator' => [['id' => 'foo', 'inner' => 'baz.inner']]], $container->getDefinition('baz')->getTags());
282282
}
283283

284284
public function testCannotDecorateSyntheticService()

‎Tests/Compiler/DefinitionErrorExceptionPassTest.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\DependencyInjection\ContainerBuilder;
1717
use Symfony\Component\DependencyInjection\Definition;
1818
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
19+
use Symfony\Component\DependencyInjection\Reference;
1920

2021
class DefinitionErrorExceptionPassTest extends TestCase
2122
{
@@ -50,6 +51,27 @@ public function testNoExceptionThrown()
5051
$this->assertSame($def, $container->getDefinition('foo_service_id')->getArgument(0));
5152
}
5253

54+
public function testSkipNestedErrors()
55+
{
56+
$container = new ContainerBuilder();
57+
58+
$container->register('nested_error', 'stdClass')
59+
->addError('Things went wrong!');
60+
61+
$container->register('bar', 'stdClass')
62+
->addArgument(new Reference('nested_error'));
63+
64+
$container->register('foo', 'stdClass')
65+
->addArgument(new Reference('bar', ContainerBuilder::RUNTIME_EXCEPTION_ON_INVALID_REFERENCE));
66+
67+
$pass = new DefinitionErrorExceptionPass();
68+
$pass->process($container);
69+
70+
$this->expectException(RuntimeException::class);
71+
$this->expectExceptionMessage('Things went wrong!');
72+
$container->get('foo');
73+
}
74+
5375
public function testSkipErrorFromTag()
5476
{
5577
$container = new ContainerBuilder();

‎Tests/Fixtures/config/anonymous.expected.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,5 @@ services:
1616
class: Symfony\Component\DependencyInjection\Tests\Fixtures\StdClassDecorator
1717
public: true
1818
tags:
19-
- container.decorator: { id: decorated }
19+
- container.decorator: { id: decorated, inner: decorator42 }
2020
arguments: [!service { class: stdClass }]

‎Tests/Fixtures/config/child.expected.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ services:
88
class: Class2
99
public: true
1010
tags:
11-
- container.decorator: { id: bar }
11+
- container.decorator: { id: bar, inner: b }
1212
file: file.php
1313
lazy: true
1414
arguments: [!service { class: Class1 }]

0 commit comments

Comments
 (0)