Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DependencyInjection] Fix "Cannot replace arguments" errors caused by ResolveAutowireInlineAttributesPass#54908
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedMay 13, 2024
| Q | A |
|---|---|
| Branch? | 7.1 |
| Bug fix? | yes |
| New feature? | no |
| Deprecations? | no |
| Issues | Fix#54823 |
| License | MIT |
… ResolveAutowireInlineAttributesPass
| } | ||
| $dummy =$value; | ||
| while (null ===$dummy->getClass() &&$dummyinstanceof ChildDefinition) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this is dead code: definitions with no class are already skipped before (and that's legit for child definitions too)
| foreach ($parametersas$index =>$parameter) { | ||
| if ($isChildDefinition) { | ||
| $index ='index_'.$index; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Renaming of indexes for child definitions is what caused the issue.
The patch fixes this using named arguments instead, which means using Definition::setArgument() instead of replaceArgument() when resolving child definitions.
GromNaN left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This fixes the issue and the test is perfect.
derrabus left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This patch fixes the issue for me. Thank you!
derrabus commentedMay 13, 2024
Thank you Nicolas for taking care of this regression. |