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] AddCheckAliasValidityPass to check interface compatibility#50745
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
carsonbot commentedJun 22, 2023
Hey! To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done? Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review. Cheers! Carsonbot |
carsonbot commentedJun 22, 2023
Hey! I see that this is your first PR. That is great! Welcome! Symfony has acontribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
41b5f63 to810f7e9Comparen-valverde commentedJun 24, 2023
Few more thoughts for people coming here:
Friendly ping@nicolas-grekas since you've worked on the initial |
nicolas-grekas commentedJun 24, 2023
For sure this shouldn't be part of the always-on passes but if adding this to |
fabpot commentedFeb 3, 2024
@n-valverde Can you work on the change suggested by@nicolas-grekas |
ee18b9c to4aebb69Comparen-valverde commentedFeb 4, 2024
Hello, sorry for the delay, I rebased on 7.1 and made the change so the pass is only applied to
|
fabpot commentedFeb 4, 2024
That makes sense to me as it allows developers not using the full stack framework to still use the pass. |
d598f13 toc5dff26Comparefabpot commentedFeb 6, 2024
Thank you@n-valverde. |
Uh oh!
There was an error while loading.Please reload this page.
Hello, I would like to propose adding a pass to the compiler, to make sure that when defining an alias which happens to be an interface, the resolved service actually implements that interface. See example below.
Currently the above situation is allowed, and will ultimately lead to a type error at runtime when trying to inject
BarInterface, becauseBaris not compatible. I think this situation should be raised as a configuration error, as I can't think of any valid use case for this.I might be missing some use cases, and this might have been already discussed in some way, so any input appreciated, even if you think this is a bad idea for some reason 🙃
Thanks!