Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Support Symfony #[AutowireLocator] attribute #420

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

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

Conversation

Copy link

@RafaelKr RafaelKr commented Dec 30, 2024 •
edited
Loading

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

Copy link
Author

RafaelKr commented Dec 30, 2024 •
edited
Loading

(削除) 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 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 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
Copy link
Author

RafaelKr commented Dec 31, 2024 •
edited
Loading

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
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 🚀

Copy link
Author

RafaelKr commented Jan 3, 2025 •
edited
Loading

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?

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
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /