-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add support to PHP-Parser 5.0 #133
Conversation
Related to Static Analysis failure (https://github.com/composer-unused/symbol-parser/actions/runs/7989925501/job/21817531464#step:5:10)
Answer is on PHP-Parser Upgrade Guide
PS: fixed with commit cf9f01c |
@icanhazstring Due to my last comment on nikic/PHP-Parser#981 (comment) we should think to :
WDYT about this ? |
Yes. I do have a open PR on composer-unused to drop 7.4 support . But have problems with the phar build. But dropping it here would also make sense then 👍 |
@icanhazstring Very strange behavior between PHPUnit on local and PHAR version Running with PHPUnit 9.6 (Composer) results
While running with PHPUnit 9.6 (PHAR) results
|
@icanhazstring Could you re-trigger a CI pipeline build on this PR to compare with PHP-Parser 5.0.1 Not needed : see answer at nikic/PHP-Parser#981 (comment) |
@icanhazstring I'll update workflow in next PR iteration to use PHPUnit dev dependency installed by Composer PS: see commit 7efe697 |
@icanhazstring Finally we did it ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for the update 🎉👍
Not sure if this is a good idea, as it could cause bugs like this in compatible php-parser versions: Instead the composer-unused should be scoped, so it does not enforce dependency on php-parser at all. |
@TomasVotruba I've tested my contribs (see RFC composer-unused/composer-unused#630) in real condition and found no issues with Parser 4.18 or 5.0 But as nobody is perfect ;-) I've called community to help to test it too ! |
All Submissions:
New Feature Submissions:
Hello,
With this new PR, I want to add support to newest major version 5 of PHP-Parser, but of course keep compatibility with previous v4.
This is possible when you read official documentation
So
composer.json
constraint is upgraded from min 4.17 to 4.18To test locally compatibility, I've a repo where I've put a little CLI application to test use cases.
TL;DR; spatie/fork#54 to see the changes I've applied on
spatie/fork
project.Locally I update manually
nikic/php-parser
dependency to get :4.18.0
version5.0.0
versionHere are results I get from my sandbox project
Resource scripts are available at :
Use Case 1 : `play:fork:symbol-parser:uc1` output
Use Case 2 : `play:fork:symbol-parser:uc2` output