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.

Sign up for GitHub

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

Jump to bottom

Add nikic/php-parser v5 support #11035

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

Add nikic/php-parser v5 support #11035

wants to merge 11 commits into from

Conversation

VincentLanglet
Copy link
Contributor

No description provided.

@VincentLanglet VincentLanglet deleted the nikic5 branch July 3, 2024 09:54
@VincentLanglet VincentLanglet mentioned this pull request Jul 25, 2024
Closed
@VincentLanglet VincentLanglet changed the title Try to add nikic/php-parser v5 support Add nikic/php-parser v5 support Jul 25, 2024
@VincentLanglet VincentLanglet mentioned this pull request Jul 25, 2024
Merged
SCIF
SCIF reviewed Aug 9, 2024
View reviewed changes
composer.json Outdated Show resolved Hide resolved
@VincentLanglet VincentLanglet marked this pull request as ready for review August 9, 2024 18:11
edsrzf
edsrzf reviewed Aug 10, 2024
View reviewed changes
Copy link
Contributor

@edsrzf edsrzf left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran this over a code base that's still using php-parser 4. There's a problem with usePHPParserV4, which I've commented on, but with that fixed it seems to work.

Do we need to look at running tests against php-parser 4 as well?

src/Psalm/Internal/BCHelper.php Outdated Show resolved Hide resolved
@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Aug 10, 2024 •
edited
Loading

I ran this over a code base that's still using php-parser 4. There's a problem with usePHPParserV4, which I've commented on, but with that fixed it seems to work.

Do we need to look at running tests against php-parser 4 as well?

Thanks. I added the CI "lowest" with php parser 4 and fixed the build.

@VincentLanglet
Copy link
Contributor Author

I think this i ready to be review @orklah @weirdan

SCIF
SCIF reviewed Aug 13, 2024
View reviewed changes
src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php
Comment on lines +96 to +99
/**
* @var SplObjectStorage<PhpParser\Node\FunctionLike, null>
*/
private SplObjectStorage $closure_statements;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@chriskapp
Copy link

It would be really nice if we could merge this, currently it is not possible to install Psalm with PHPUnit 11 since it requires nikic/php-parser: ^5.0

@VincentLanglet
Copy link
Contributor Author

Friendly ping @weirdan if you have time to take a look (or can ping another maintainer ?).
This PR would be really helpful, by allowing to use both Psalm and PHPunit 11.

@weirdan
Copy link
Collaborator

weirdan commented Sep 8, 2024

Yeah, it's on my todo list. The thing is big, so it'll take me some time.

@gazben gazben mentioned this pull request Sep 14, 2024
Open
@Lukasss93 Lukasss93 mentioned this pull request Sep 17, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SCIF SCIF SCIF left review comments

@edsrzf edsrzf Awaiting requested review from edsrzf

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@VincentLanglet @chriskapp @weirdan @edsrzf @SCIF