Skip to content

Commit 9ecacb4

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '6.4' into 7.3
* 6.4: [DependencyInjection] Fix dealing with errored service definitions
2 parents fd4ed7f + daa4da5 commit 9ecacb4

File tree

6 files changed

+56
-7
lines changed

6 files changed

+56
-7
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ class CheckDefinitionValidityPass implements CompilerPassInterface
3838
public function process(ContainerBuilder $container): void
3939
{
4040
foreach ($container->getDefinitions() as $id => $definition) {
41+
if ($definition->hasErrors()) {
42+
continue;
43+
}
44+
4145
// synthetic service is public
4246
if ($definition->isSynthetic() && !$definition->isPublic()) {
4347
throw new RuntimeException(\sprintf('A synthetic service ("%s") must be public.', $id));

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,17 @@ class ResolveClassPass implements CompilerPassInterface
2323
public function process(ContainerBuilder $container): void
2424
{
2525
foreach ($container->getDefinitions() as $id => $definition) {
26-
if ($definition->isSynthetic() || null !== $definition->getClass()) {
26+
if ($definition->isSynthetic()
27+
|| $definition->hasErrors()
28+
|| null !== $definition->getClass()
29+
|| !preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+)++$/', $id)
30+
) {
2731
continue;
2832
}
29-
if (preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+)++$/', $id)) {
30-
if ($definition instanceof ChildDefinition && !class_exists($id)) {
31-
throw new InvalidArgumentException(\sprintf('Service definition "%s" has a parent but no class, and its name looks like an FQCN. Either the class is missing or you want to inherit it from the parent service. To resolve this ambiguity, please rename this service to a non-FQCN (e.g. using dots), or create the missing class.', $id));
32-
}
33-
$definition->setClass($id);
33+
if ($definition instanceof ChildDefinition && !class_exists($id)) {
34+
throw new InvalidArgumentException(\sprintf('Service definition "%s" has a parent but no class, and its name looks like an FQCN. Either the class is missing or you want to inherit it from the parent service. To resolve this ambiguity, please rename this service to a non-FQCN (e.g. using dots), or create the missing class.', $id));
3435
}
36+
$definition->setClass($id);
3537
}
3638
}
3739
}

‎src/Symfony/Component/DependencyInjection/Loader/FileLoader.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,12 +198,12 @@ public function registerClasses(Definition $prototype, string $namespace, string
198198
}
199199

200200
$this->setDefinition($class, $definition = $getPrototype());
201+
$definition->setClass($class);
201202
if (null !== $errorMessage) {
202203
$definition->addError($errorMessage);
203204

204205
continue;
205206
}
206-
$definition->setClass($class);
207207

208208
$interfaces = [];
209209
foreach (class_implements($class, false) as $interface) {

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,18 @@ public function testDynamicPrivateName()
157157
$this->addToAssertionCount(1);
158158
}
159159

160+
public function testProcessSkipsDefinitionsWithErrors()
161+
{
162+
$container = new ContainerBuilder();
163+
$definition = $container->register('a')->setSynthetic(true)->setPublic(false);
164+
$definition->addError('Some error message');
165+
166+
// This should not throw an exception because the definition has errors
167+
$this->process($container);
168+
169+
$this->addToAssertionCount(1);
170+
}
171+
160172
protected function process(ContainerBuilder $container)
161173
{
162174
$pass = new CheckDefinitionValidityPass();

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,4 +93,16 @@ public function testAmbiguousChildDefinition()
9393

9494
(new ResolveClassPass())->process($container);
9595
}
96+
97+
public function testSkipsDefinitionsWithErrors()
98+
{
99+
$container = new ContainerBuilder();
100+
$def = $container->register('App\SomeClass');
101+
$def->addError('Some error message');
102+
103+
(new ResolveClassPass())->process($container);
104+
105+
// The class should not be set because the definition has errors
106+
$this->assertNull($def->getClass());
107+
}
96108
}

‎src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,25 @@ public function testMissingParentClass()
233233
);
234234
}
235235

236+
public function testClassIsSetEvenWhenDefinitionHasErrors()
237+
{
238+
$container = new ContainerBuilder();
239+
$container->setParameter('bad_classes_dir', 'BadClasses');
240+
$loader = new TestFileLoader($container, new FileLocator(self::$fixturesPath.'/Fixtures'), 'test');
241+
242+
$loader->registerClasses(
243+
(new Definition())->setAutoconfigured(true),
244+
'Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\BadClasses\\',
245+
'Prototype/%bad_classes_dir%/*'
246+
);
247+
248+
$definition = $container->getDefinition(MissingParent::class);
249+
250+
// The class should be set even when the definition has errors
251+
$this->assertSame(MissingParent::class, $definition->getClass());
252+
$this->assertNotEmpty($definition->getErrors());
253+
}
254+
236255
public function testRegisterClassesWithBadPrefix()
237256
{
238257
$this->expectException(InvalidArgumentException::class);

0 commit comments

Comments
 (0)