Skip to content

Commit

Permalink
bug #23605 [DI][Bug] Autowiring thinks optional args on core classes …
Browse files Browse the repository at this point in the history
…are required (weaverryan)

This PR was merged into the 3.3 branch.

Discussion
----------

[DI][Bug] Autowiring thinks optional args on core classes are required

| Q             | A
| ------------- | ---
| Branch?       | 3,3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | none
| License       | MIT
| Doc PR        | n/a

Currently, the following fails:

```yml
services:
    PDO:
        class: PDO
        arguments:
            - 'sqlite:/foo.db'
```

The error:

> Cannot autowire service "PDO": argument "$username" of method "__construct()" must have a type-hint or be given a value explicitly

`$username` is the second argument to `PDO`, and it's optional. Here's the reason: it appears that `$parameter->isDefaultValueAvailable()` returns false for optional arguments of core classes. But, `$parameter->isOptional()` returns true.

This allows optional arguments to not throw an exception. I can't think of any edge cases this will cause - but it's possible I'm not thinking of something :).

Cheers!

Commits
-------

178a0f7 Fixing a bug where if a core class was autowired, autowiring tried to autowire optional args as if they were required
  • Loading branch information
fabpot committed Jul 21, 2017
2 parents 444a840 + 178a0f7 commit adeab15
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
Expand Up @@ -273,6 +273,12 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a

// no default value? Then fail
if (!$parameter->isDefaultValueAvailable()) {
// For core classes, isDefaultValueAvailable() can
// be false when isOptional() returns true. If the
// argument *is* optional, allow it to be missing
if ($parameter->isOptional()) {
continue;
}
throw new AutowiringFailedException($this->currentId, sprintf('Cannot autowire service "%s": argument "$%s" of method "%s()" must have a type-hint or be given a value explicitly.', $this->currentId, $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
}

Expand Down
Expand Up @@ -512,6 +512,23 @@ public function testOptionalScalarArgsNotPassedIfLast()
);
}

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

$container->register('pdo_service', \PDO::class)
->addArgument('sqlite:/foo.db')
->setAutowired(true);

(new AutowirePass())->process($container);

$definition = $container->getDefinition('pdo_service');
$this->assertEquals(
array('sqlite:/foo.db'),
$definition->getArguments()
);
}

public function testSetterInjection()
{
$container = new ContainerBuilder();
Expand Down

0 comments on commit adeab15

Please sign in to comment.