-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DependencyInjection] #[Autowire]
attribute should have precedence over bindings
#51559
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DependencyInjection] #[Autowire]
attribute should have precedence over bindings
#51559
Conversation
Thanks for fixing this 🙏 . Could you rebase this? |
b9bacf2
to
57f4b25
Compare
@nicolas-grekas is it right to have to make ResolveBindingPass aware of the Autowire attribute ? Shouldn't this be handled by the AutowirePass ? |
I agree, the bindings pass has no reasons to know about autowiring stuff. |
@nicolas-grekas @stof My initial approach was to fix it in I can try to change the order of the passes, but I'm worried that might break other stuff. I'll try it sometime today or tomorrow. |
It's also not obvious to me what we want here. What this PR achieves is this precedence, isn't it?
Dunno if we should account for that but bindings can also be defined by explicit configuration and/or by autoconfiguration. Dunno how we can make AutowirePass achieve this ordering (or another) given that once bindings are resolved, we don't know if some argument's values came through bindings or via another path (explicit/auto config). Maybe this approach is the most pragmatic. WDYT? |
src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php
Outdated
Show resolved
Hide resolved
57f4b25
to
c429fe5
Compare
Any other opinion here? @stof? |
Thank you @HypeMC. |
It seems logical (I think) that the
#[Autowire]
attribute has precedence over any bindings.