-
Notifications
You must be signed in to change notification settings - Fork 510
Try olvlvl/composer-attribute-collector #4064
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
Conversation
Won't work on PHP 7.4. But the idea is to rebase my fork on top of it and only have minimal changes there. |
yeah, the problem is that it currently doesn't even work on PHP 8.x |
I cloned
I removed all the "patches" and used my package. After the autoload dump I see the attribute file with collected target parameters. I guess the collector is doing its job. targetParameters: array (
'PHPStan\\DependencyInjection\\AutowiredParameter' =>
array (
0 =>
array (
0 =>
array (
'ref' => '%currentWorkingDirectory%',
),
1 => 'PHPStan\\File\\ParentDirectoryRelativePathHelper',
2 => '__construct',
3 => 'parentDirectory',
),
1 =>
array (
0 =>
array (
'ref' => '%currentWorkingDirectory%',
),
1 => 'PHPStan\\File\\FileHelper',
2 => '__construct',
3 => 'workingDirectory',
), Running
I see After removing the interface filter I see the class in the dump:
which matches the definition:
After that, I get tons of these errors, but I guess that's because I'm running PHP 8.4.
Can you try on your side if removing the interface filter works? I don't mind collecting attributes on interfaces, it's just I never thought of this usage. In private static function buildFileFilter(): Filter
{
return new Filter\Chain([
new ContentFilter(),
//new InterfaceFilter()
]);
} I created a Dockerfile with PHP 8.2, restored the "patches", and ran
Then, running
It would be great to have a Dockerfile for reproducible runs. |
@olvlvl If you're on macOS the fix for patches is described in the README. Other than that the project works without issues even on PHP 8.4. |
Yeah, that's one of the commits I did ondrejmirtes/composer-attribute-collector@d285f54 |
The PR is ready over here: olvlvl/composer-attribute-collector#38 |
The changes are ready: olvlvl/composer-attribute-collector#37. I've tested phpstan-src, the tests are passing. Let me know how it goes for you 🤞 |
I think it looks great, thank you. PHP7.4 failling is expected as this is only supported on ondrejs fork. |
Great! I'm gonna merge the changes to |
Gentlemen, please use the development branch |
I released v2.1.0 and killed the 2.1 dev branch. |
thanks. next step is porting back the changes into ondrejs fork. it will take a few days until I have time todo it |
@staabm Ideally you'd start over with the origin repo. I added these 19 commits in my repo so you should be able to replay them (I mean manually) on top of origin/HEAD again: olvlvl/composer-attribute-collector@main...ondrejmirtes:composer-attribute-collector:main But I acknowledge it'd be a very tedious task with very little benefit. I'm fine with leaving the things as they are currently, and just switch to |
Let me know if I can help. Good luck! @ondrejmirtes Thanks for PHPStan, I love it <3 |
No description provided.