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
-------

ab0fd6e663 If a (non-global) instanceof does not exist, throw an exception
This commit is contained in:
Fabien Potencier 2017-04-28 13:30:07 -07:00
commit f8133cbd4e
2 changed files with 38 additions and 5 deletions

View File

@ -14,6 +14,7 @@ namespace Symfony\Component\DependencyInjection\Compiler;
use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
/**
* Applies instanceof conditionals to definitions.
@ -40,7 +41,6 @@ class ResolveInstanceofConditionalsPass implements CompilerPassInterface
{
$instanceofConditionals = $definition->getInstanceofConditionals();
$automaticInstanceofConditionals = $definition->isAutoconfigured() ? $container->getAutomaticInstanceofDefinitions() : array();
if (!$instanceofConditionals && !$automaticInstanceofConditionals) {
return $definition;
}
@ -49,14 +49,14 @@ class ResolveInstanceofConditionalsPass implements CompilerPassInterface
return $definition;
}
$conditionals = $this->mergeConditionals($automaticInstanceofConditionals, $instanceofConditionals);
$conditionals = $this->mergeConditionals($automaticInstanceofConditionals, $instanceofConditionals, $container);
$definition->setInstanceofConditionals(array());
$parent = $shared = null;
$instanceofTags = array();
foreach ($conditionals as $interface => $instanceofDefs) {
if ($interface !== $class && (!$container->getReflectionClass($interface) || !$container->getReflectionClass($class))) {
if ($interface !== $class && (!$container->getReflectionClass($class))) {
continue;
}
@ -113,12 +113,17 @@ class ResolveInstanceofConditionalsPass implements CompilerPassInterface
return $definition;
}
private function mergeConditionals(array $automaticInstanceofConditionals, array $instanceofConditionals)
private function mergeConditionals(array $automaticInstanceofConditionals, array $instanceofConditionals, ContainerBuilder $container)
{
// make each value an array of ChildDefinition
$conditionals = array_map(function($childDef) { return array($childDef); }, $automaticInstanceofConditionals);
$conditionals = array_map(function ($childDef) { return array($childDef); }, $automaticInstanceofConditionals);
foreach ($instanceofConditionals as $interface => $instanceofDef) {
// make sure the interface/class exists (but don't validate automaticInstanceofConditionals)
if (!$container->getReflectionClass($interface)) {
throw new RuntimeException(sprintf('"%s" is set as an "instanceof" conditional, but it does not exist.', $interface));
}
if (!isset($automaticInstanceofConditionals[$interface])) {
$conditionals[$interface] = array();
}

View File

@ -154,4 +154,32 @@ class ResolveInstanceofConditionalsPassTest extends TestCase
// no automatic_tag, it was not enabled on the Definition
$this->assertFalse($def->isAutowired());
}
/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @expectedExceptionMessage "App\FakeInterface" is set as an "instanceof" conditional, but it does not exist.
*/
public function testBadInterfaceThrowsException()
{
$container = new ContainerBuilder();
$def = $container->register('normal_service', self::class);
$def->setInstanceofConditionals(array(
'App\\FakeInterface' => (new ChildDefinition(''))
->addTag('foo_tag'),
));
(new ResolveInstanceofConditionalsPass())->process($container);
}
public function testBadInterfaceForAutomaticInstanceofIsOkException()
{
$container = new ContainerBuilder();
$container->register('normal_service', self::class)
->setAutoconfigured(true);
$container->registerForAutoconfiguration('App\\FakeInterface')
->setAutowired(true);
(new ResolveInstanceofConditionalsPass())->process($container);
$this->assertTrue($container->hasDefinition('normal_service'));
}
}