Skip to content
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

Support Symfony #[AutowireLocator] attribute #420

Open
wants to merge 5 commits into
base: 1.4.x
Choose a base branch
from

Conversation

RafaelKr
Copy link

@RafaelKr RafaelKr commented Dec 30, 2024

I implemented initial support for the #[AutowireLocator] attribute. See https://symfony.com/blog/new-in-symfony-6-4-autowirelocator-and-autowireiterator-attributes

The simple syntax is supported now:

#[AutowireLocator([FooHandler::class, BarHandler::class])]

It also supports when classes are specified via string:

#[AutowireLocator(['App\FooHandler'])]

The advanced syntax described in the blog post is not also supported yet now:

#[AutowireLocator([
    'foo' => FooHandler::class,
    'bar' => new SubscribedService(type: 'string', attributes: new Autowire('%some.parameter%')),
    'optionalBaz' => '?'.BazHandler::class,
])]
private ContainerInterface $handlers,

Maybe someone else wants to figure this out. Unfortunately I have no more time left to do it in the coming days.

Edit 2: I just found out how I can both simplify this a lot and implement the advanced syntax by just creating an instance of the AutowireLocator attribute. And I'll move this to AutowireLoaderServiceMapFactory so it will be also available for PHPStan\Rules\Symfony\ContainerInterfaceUnknownServiceRule.

closes #411

@RafaelKr RafaelKr force-pushed the 1.4.x branch 9 times, most recently from 83ef507 to 5984d62 Compare December 30, 2024 17:59
@RafaelKr
Copy link
Author

RafaelKr commented Dec 30, 2024

I tried to fix all errors but I'm not sure how to fix those 2 errors left.

Edit: Got it.

@RafaelKr RafaelKr force-pushed the 1.4.x branch 2 times, most recently from 9334602 to 13edbbc Compare December 30, 2024 18:05
…PrivateServiceRule

We now create an AutowireLocator instance and check if the service exists there. This also removes a lot of nested foreach loops and thus decreases the complexity of the isAutowireLocatorService method a lot.

https://symfony.com/blog/new-in-symfony-6-4-autowirelocator-and-autowireiterator-attributes
@RafaelKr RafaelKr deleted the branch phpstan:1.4.x December 31, 2024 16:33
@RafaelKr RafaelKr closed this Dec 31, 2024
@RafaelKr RafaelKr deleted the 1.4.x branch December 31, 2024 16:33
@RafaelKr RafaelKr restored the 1.4.x branch December 31, 2024 16:35
@RafaelKr RafaelKr deleted the 1.4.x branch December 31, 2024 16:37
@RafaelKr RafaelKr restored the 1.4.x branch December 31, 2024 16:38
@RafaelKr RafaelKr reopened this Dec 31, 2024
@RafaelKr RafaelKr force-pushed the 1.4.x branch 4 times, most recently from fe2a56d to 4ef06e1 Compare December 31, 2024 16:57
@RafaelKr RafaelKr marked this pull request as ready for review December 31, 2024 16:58
@RafaelKr
Copy link
Author

RafaelKr commented Dec 31, 2024

I'll add some tests later, besides from that it's ready for review.

Btw: I tried to rename the branch of my Repo to feat/autowirelocator, but it wasn't possible without losing the connection to this PR, the PR showed the branch was deleted in that case.

This allows us to share the logic for other rules
@RafaelKr RafaelKr changed the title Basic support for simple Symfony #[AutowireLocator] attribute Support Symfony #[AutowireLocator] attribute Jan 3, 2025
@RafaelKr RafaelKr force-pushed the 1.4.x branch 2 times, most recently from ec48673 to b2c2416 Compare January 3, 2025 20:30
@RafaelKr
Copy link
Author

RafaelKr commented Jan 3, 2025

I also added tests now, but I have no idea how to resolve the failing lint and test checks. If someone can resolve those we're ready to merge from my side 🚀

@RafaelKr
Copy link
Author

RafaelKr commented Jan 3, 2025

I also noticed it only works when we're using Constructor property promotion, so when specifying private ContainerInterface $locator in the constructor.

It won't work when $locator is specified as a class member and then assigned in the constructor method (as here e.g. https://github.com/phpstan/phpstan-symfony/blob/c7b7e7f520893621558bfbfdb2694d4364565c1d/tests/Rules/Symfony/ExampleServiceSubscriber.php).

Is this a problem in practice?

@ondrejmirtes
Copy link
Member

Hi, I can't review this when the build isn't green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants