Skip to content

PHP 8.4 | Tokenizer/PHP: fix union/intersection/DNF type tokenization with final properties #907

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

Merged
merged 1 commit into from
Apr 1, 2025

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Mar 28, 2025

Description

Turns out there are tokenizer changes needed to support final properties as the retokenization of | to T_TYPE_UNION, & to T_TYPE_INTERSECTION and the parenthesis in DNF types to T_TYPE_*_PARENTHESIS was broken.

Fixed now.

Includes tests.

Suggested changelog entry

Fixed tokenization of union/intersection and DNF types in combination with PHP 8.4 final properties.

Related issues/external references

Related to #734 and #834

Verified

This commit was signed with the committer’s verified signature.
bagder Daniel Stenberg
… with `final` properties

Turns out there are tokenizer changes needed to support `final` properties as the retokenization of `|` to `T_TYPE_UNION`, `&` to T_TYPE_INTERSECTION` and the parenthesis in DNF types to `T_TYPE_*_PARENTHESIS` was broken.

Fixed now.

Includes tests.
@jrfnl
Copy link
Member Author

jrfnl commented Mar 28, 2025

@DanielEScherzer Would you like to review this PR ?

Copy link
Contributor

@DanielEScherzer DanielEScherzer left a comment

Choose a reason for hiding this comment

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

Code looks good to me, but the syntax highlighting in the PR description could use some improvement

@jrfnl
Copy link
Member Author

jrfnl commented Mar 29, 2025

Code looks good to me, but the syntax highlighting in the PR description could use some improvement

Fixed.

Reason I found this was cause I was looking at the asym visibility PR. A similar change will be needed in that PR to allow for the type tokens to be handled correctly in combination with asym visibility.

The change I made in PR #908 is also related and a similar change may be needed in the asym PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants