Skip to content

Conversation

@mreiden
Copy link
Contributor

@mreiden mreiden commented Aug 13, 2024

Depends on php-parser dependency support.

Kenneth-Sills/php-parser#1

@czosel
Copy link
Collaborator

czosel commented Nov 6, 2024

@mreiden The PR in the parser has landed in v3.2.0 🎉 Do you want to update your PR?

src/parser.mjs Outdated
const parser = new engine({
parser: {
extractDoc: true,
version: opts.phpVersion,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This won't be needed once glayzzle/php-parser#1150 has landed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should use the php version a user sets (https://github.com/prettier/plugin-php#configuration) instead of using the php-parser default though? If I remember correctly, the user setting has no effect on the parser without this line.

The php-parser default now being 8.3 makes it unnecessary for most people, but if someone set it to 8.0 it would use unsupported syntax. Then again, only 8.2+ are currently supported by the php team, so maybe it's time to drop all the 5.x and 7.x versions and the default changed from "7.0" to "8.3"?

Copy link
Collaborator

@czosel czosel Dec 31, 2024

Choose a reason for hiding this comment

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

I think the idea is that we always try to parse with the latest PHP version, and only use the prettier version setting to control our printing (i.e. don't print features that are unsupported in the configured version). Since glayzzle/php-parser#1150 is now landed, I don't really see a reason to change this. I'd be very open to update the default for our version setting to something more reasonable though! 👍

@genintho
Copy link
Contributor

@czosel this will need a new release of php-parser to include the fix to the original implementation.

@czosel
Copy link
Collaborator

czosel commented Dec 31, 2024

@czosel this will need a new release of php-parser to include the fix to the original implementation.

Done! Sorry about the delay.

@tom-on-the-internet
Copy link

Looking forward to this. 🤞

@yusufkandemir
Copy link

It's been 7 months since the upstream support was released: https://github.com/glayzzle/php-parser/releases/tag/v3.2.2

mreiden added 2 commits July 15, 2025 22:12
…d 7.x.

This matches the default php version used in the php-parser project and
removes options for very old eol versions.  PHP 5.6 eol in January 2019
and 7.4 in November 2022.
@cseufert cseufert force-pushed the php83-typed-constants branch 2 times, most recently from 81957ec to eea57cc Compare July 15, 2025 12:20
@cseufert
Copy link
Collaborator

I am closing this as as of v0.24.0 this patch is redundant.

@cseufert cseufert closed this Jul 16, 2025
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.

6 participants