Skip to content

Commit

Permalink
bug #30889 [DependencyInjection] Fix a wrong error when using a facto…
Browse files Browse the repository at this point in the history
…ry (Simperfit)

This PR was merged into the 3.4 branch.

Discussion
----------

[DependencyInjection] Fix a wrong error when using a factory

…d a call

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #30885    <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        |

<!--
Write a short README entry for your feature/bugfix here (replace this comment block.)
This will help people understand your PR and can be used as a start of the Doc PR.
Additionally:
 - Bug fixes must be submitted against the lowest branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the master branch.
-->
This a work in progress, it needs tests, firstly I wanted to ask the author to test and tell if it's ok.

Commits
-------

5d4e3a2 [WIP] [DependencyInjection] Fix a wrong error when using a factory and a call
  • Loading branch information
fabpot committed Apr 6, 2019
2 parents fb1c154 + 5d4e3a2 commit 86210b3
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 2 deletions.
Expand Up @@ -181,7 +181,15 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC
if ($method instanceof \ReflectionFunctionAbstract) {
$reflectionMethod = $method;
} else {
$reflectionMethod = $this->getReflectionMethod(new Definition($reflectionClass->name), $method);
$definition = new Definition($reflectionClass->name);
try {
$reflectionMethod = $this->getReflectionMethod($definition, $method);
} catch (RuntimeException $e) {
if ($definition->getFactory()) {
continue;
}
throw $e;
}
}

$arguments = $this->autowireMethod($reflectionMethod, $arguments);
Expand Down
Expand Up @@ -115,7 +115,14 @@ protected function processValue($value, $isRoot = false)
if ($method instanceof \ReflectionFunctionAbstract) {
$reflectionMethod = $method;
} else {
$reflectionMethod = $this->getReflectionMethod($value, $method);
try {
$reflectionMethod = $this->getReflectionMethod($value, $method);
} catch (RuntimeException $e) {
if ($value->getFactory()) {
continue;
}
throw $e;
}
}

foreach ($reflectionMethod->getParameters() as $key => $parameter) {
Expand Down
Expand Up @@ -23,6 +23,7 @@
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\Tests\Fixtures\includes\FooVariadic;
use Symfony\Component\DependencyInjection\TypedReference;
use Symfony\Component\HttpKernel\HttpKernelInterface;

require_once __DIR__.'/../Fixtures/includes/autowiring_classes.php';

Expand Down Expand Up @@ -605,6 +606,18 @@ public function testSetterInjection()
);
}

public function testWithNonExistingSetterAndAutowiring()
{
$container = new ContainerBuilder();

$definition = $container->register(HttpKernelInterface::class, HttpKernelInterface::class)->setAutowired(true);
$definition->addMethodCall('setLogger');
$this->expectException(RuntimeException::class);
(new ResolveClassPass())->process($container);
(new AutowireRequiredMethodsPass())->process($container);
(new AutowirePass())->process($container);
}

public function testExplicitMethodInjection()
{
$container = new ContainerBuilder();
Expand Down
Expand Up @@ -16,11 +16,14 @@
use Symfony\Component\DependencyInjection\Compiler\AutowireRequiredMethodsPass;
use Symfony\Component\DependencyInjection\Compiler\ResolveBindingsPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass;
use Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy;
use Symfony\Component\DependencyInjection\Tests\Fixtures\ParentNotExists;
use Symfony\Component\DependencyInjection\TypedReference;
use Symfony\Component\HttpKernel\HttpKernelInterface;

require_once __DIR__.'/../Fixtures/includes/autowiring_classes.php';

Expand Down Expand Up @@ -111,4 +114,45 @@ public function testScalarSetter()

$this->assertEquals([['setDefaultLocale', ['fr']]], $definition->getMethodCalls());
}

public function testWithNonExistingSetterAndBinding()
{
$container = new ContainerBuilder();

$bindings = [
'$c' => (new Definition('logger'))->setFactory('logger'),
];

$definition = $container->register(HttpKernelInterface::class, HttpKernelInterface::class);
$definition->addMethodCall('setLogger');
$definition->setBindings($bindings);
$this->expectException(RuntimeException::class);

$pass = new ResolveBindingsPass();
$pass->process($container);
}

public function testTupleBinding()
{
$container = new ContainerBuilder();

$bindings = [
'$c' => new BoundArgument(new Reference('bar')),
CaseSensitiveClass::class.'$c' => new BoundArgument(new Reference('foo')),
];

$definition = $container->register(NamedArgumentsDummy::class, NamedArgumentsDummy::class);
$definition->addMethodCall('setSensitiveClass');
$definition->addMethodCall('setAnotherC');
$definition->setBindings($bindings);

$pass = new ResolveBindingsPass();
$pass->process($container);

$expected = [
['setSensitiveClass', [new Reference('foo')]],
['setAnotherC', [new Reference('bar')]],
];
$this->assertEquals($expected, $definition->getMethodCalls());
}
}

0 comments on commit 86210b3

Please sign in to comment.