8000 bug #22565 If an instanceof does not exist, throw an exception (weave… · symfony/symfony@f8133cb · GitHub
[go: up one dir, main page]
Skip to content

Commit f8133cb

Browse files
fabpotfabpot
committed
bug #22565 If an instanceof does not exist, throw an exception (weaverryan)
This PR was merged into the 3.3-dev branch. Discussion ---------- If an instanceof does not exist, throw an exception | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes-ish | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | none | License | MIT | Doc PR | n/a Before, it was possible to add an `instanceof` with a non-existent class/interface. Now it throws an exception, but ONLY for conditionals set directly on a Definition: we still allow for global "automatic" instanceof definitions to be set on the `ContainerBuilder`. I'm not sure if that's correct... it makes life easier for bundler maintainers. If we also strictly validated the existence of "automatic" instanceof's, then (for example) in `FrameworkExtension`, we would need to do an `if interface_exists()` around most of the `registerForAutoconfiguration()` calls. Commits ------- ab0fd6e If a (non-global) instanceof does not exist, throw an exception
2 parents 143b5ff + ab0fd6e commit f8133cb

File tree

2 files changed

+38
-5
lines changed

2 files changed

+38
-5
lines changed

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Symfony\Component\DependencyInjection\ChildDefinition;
1515
use Symfony\Component\DependencyInjection\ContainerBuilder;
1616
use Symfony\Component\DependencyInjection\Definition;
17+
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
1718

1819
/**
1920
* Applies instanceof conditionals to definitions.
@@ -40,7 +41,6 @@ private function processDefinition(ContainerBuilder $container, $id, Definition
4041
{
4142
$instanceofConditionals = $definition->getInstanceofConditionals();
4243
$automaticInstanceofConditionals = $definition->isAutoconfigured() ? $container->getAutomaticInstanceofDefinitions() : array();
43-
4444
if (!$instanceofConditionals && !$automaticInstanceofConditionals) {
4545
return $definition;
4646
}
@@ -49,14 +49,14 @@ private function processDefinition(ContainerBuilder $container, $id, Definition
4949
return $definition;
5050
}
5151

52-
$conditionals = $this->mergeConditionals($automaticInstanceofConditionals, $instanceofConditionals);
52+
$conditionals = $this->mergeConditionals($automaticInstanceofConditionals, $instanceofConditionals, $container);
5353

5454
$definition->setInstanceofConditionals(array());
5555
$parent = $shared = null;
5656
$instanceofTags = array();
5757

5858
foreach ($conditionals as $interface => $instanceofDefs) {
59-
if ($interface !== $class && (!$container->getReflectionClass($interface) || !$container->getReflectionClass($class))) {
59+
if ($interface !== $class && (!$container->getReflectionClass($class))) {
6060
continue;
6161
}
6262

@@ -113,12 +113,17 @@ private function processDefinition(ContainerBuilder $container, $id, Definition
113113
return $definition;
114114
}
115115

116-
private function mergeConditionals(array $automaticInstanceofConditionals, array $instanceofConditionals)
116+
private function mergeConditionals(array $automaticInstanceofConditionals, array $instanceofConditionals, ContainerBuilder $container)
117117
{
118118
// make each value an array of ChildDefinition
119-
$conditionals = array_map(function($childDef) { return array($childDef); }, $automaticInstanceofConditionals);
119+
$conditionals = array_map(function ($childDef) { return array($childDef); }, $automaticInstanceofConditionals);
120120

121121
foreach ($instanceofConditionals as $interface => $instanceofDef) {
122+
// make sure the interface/class exists (but don't validate automaticInstanceofConditionals)
123+
if (!$container->getReflectionClass($interface)) {
124+
throw new RuntimeException(sprintf('"%s" is set as an "instanceof" conditional, but it does not exist.', $interface));
125+
}
126+
122127
if (!isset($automaticInstanceofConditionals[$interface])) {
123128
$conditionals[$interface] = array();
124129
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,4 +154,32 @@ public function testProcessDoesNotUseAutomaticInstanceofDefinitionsIfNotEnabled(
154154
// no automatic_tag, it was not enabled on the Definition
155155
$this->assertFalse($def->isAutowired());
156156
}
157+
158+
/**
159+
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
160+
* @expectedExceptionMessage "App\FakeInterface" is set as an "instanceof" conditional, but it does not exist.
161+
*/
162+
public function testBadInterfaceThrowsException()
163+
{
164+
$container = new ContainerBuilder();
165+
$def = $container->register('normal_service', self::class);
166+
$def->setInstanceofConditionals(array(
167+
'App\\FakeInterface' => (new ChildDefinition(''))
168+
->addTag('foo_tag'),
169+
));
170+
171+
(new ResolveInstanceofConditionalsPass())->process($container);
172+
}
173+
174+
public function testBadInterfaceForAutomaticInstanceofIsOkException()
175+
{
176+
$container = new ContainerBuilder();
177+
$container->register('normal_service', self::class)
178+
->setAutoconfigured(true);
179+
$container->registerForAutoconfiguration('App\\FakeInterface')
180+
->setAutowired(true);
181+
182+
(new ResolveInstanceofConditionalsPass())->process($container);
183+
$this->assertTrue($container->hasDefinition('normal_service'));
184+
}
157185
}

0 commit comments

Comments
 (0)
0