[DI] Fixed index args bug with ResolveNamedArgumentsPass #22677
Conversation
|
@weaverryan didn't we decide to deprecate the use of keys not starting with a |
There was a problem hiding this comment.
👍 ... but @dunglas comment is important: did you agree to deprecate this feature?
|
Thank you @weaverryan. |
…weaverryan)
This PR was merged into the 3.3-dev branch.
Discussion
----------
[DI] Fixed index args bug with ResolveNamedArgumentsPass
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | none
| License | MIT
| Doc PR | n/a
While upgrading a project, this code suddenly broke:
```yml
services:
ice_cream_service:
class: AppBundle\Service\IceCreamService
autowire: true
arguments:
1: 'http://api.example.com'
```
```php
class IceCreamService
{
public function __construct(EntityManager $em, $apiUrl)
{
}
}
```
Suddenly, the index `1` was not being mapped to `$apiUrl`. This was valid in 3.2, but broke when `ResolveNamedArgumentsPass` accidentally re-set the index. Simple fix :). Ping @dunglas
Cheers!
Commits
-------
7cc7c85 Fixing bug where indexed args were set wrong in pass in some situations
|
@GuilhemN I remember we looked at that together also if I'm not wrong? Any comment here? |
|
We talked about it in #21700 and on slack but I can't recover our discussion... If I remember well your argument was that it has never been a supported use case and it only worked by chance for autowiring. For example, the following didn't behave the way described in this pr: services:
ice_cream_service:
class: AppBundle\Service\IceCreamService
autowire: true
arguments:
2: 'bar'
1: 'http://api.example.com'I'm not against this change but it probably requires a patch in older versions. |
|
About deprecating specific indexed args, I don't know what we might have agreed on, but I certainly think it makes sense. Let's make a PR for 3.4. But, for 3.2, using indexed args WAS a valid way of doing things... I actually would teach it that way (I even thought it was doc'ed, but it's not). So, I think this correctly merged (to keep BC) and we can definitely deprecate for 3.4 - named args are WAY better :). |
While upgrading a project, this code suddenly broke:
Suddenly, the index
1was not being mapped to$apiUrl. This was valid in 3.2, but broke whenResolveNamedArgumentsPassaccidentally re-set the index. Simple fix :). Ping @dunglasCheers!