Fixing autowiring collision failure #18039
Conversation
… autowire (instead of an error)
| // keep an array of all services matching this type | ||
| if (!isset($this->ambiguousServiceTypes[$type])) { | ||
| $this->ambiguousServiceTypes[$type] = array( | ||
| $this->types[$type], |
There was a problem hiding this comment.
Are we sure that there always is an entry in $this->types for $type? In set(), we later on explicitly do check for isset($this->types[$type]).
There was a problem hiding this comment.
It looks ok. The existence of the key is checked here: https://github.com/weaverryan/symfony/blob/autowire-colission-fix/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php#L192
There was a problem hiding this comment.
Yeah, but addServiceToAmbiguousType() is also called before that check: https://github.com/weaverryan/symfony/blob/autowire-colission-fix/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php#L186
There was a problem hiding this comment.
In that case, the call to $this->types[$type] is always skipped because of if (!isset($this->ambiguousServiceTypes[$type])) { (the line you pointed out is in a isset($this->ambiguousServiceTypes[$type]) block).
|
👍 |
|
Thank you @weaverryan. |
This PR was merged into the 3.1-dev branch. Discussion ---------- Fixing autowiring collision failure | Q | A | ------------- | --- | Branch | master | Bug fix? | yes (bug introduced in master) | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | none | License | MIT | Doc PR | n/a In #17877, I introduced a bug: https://github.com/symfony/symfony/pull/17877/files#diff-62df969ae028c559d33ffd256de1ac49L200. Namely, if some class cannot be autowired because there is an *odd* number of matching services, then it *would* autowire the last service found, instead of throwing an exception. The tests only tested even numbers, which is how it was missed. This fixes that. Thanks! Commits ------- 2aea337 Fixing a bug where an odd number of type collisions would incorrectly autowire (instead of an error)
In #17877, I introduced a bug: https://github.com/symfony/symfony/pull/17877/files#diff-62df969ae028c559d33ffd256de1ac49L200.
Namely, if some class cannot be autowired because there is an odd number of matching services, then it would autowire the last service found, instead of throwing an exception. The tests only tested even numbers, which is how it was missed. This fixes that.
Thanks!