-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
PHP 8.0 | Support union types in PHP Tokenizer and File class utility methods #3032
PHP 8.0 | Support union types in PHP Tokenizer and File class utility methods #3032
Conversation
I've marked this for 3.5.6 as I didn't see anything that would be a BC break, but please let me know if I've missed something. |
@gsherwood There's no BC-break, but it does introduce a new token, which depending on interpretation of semver could be considered a feature which should go into a minor rather than a patch version. |
Oops.. just noticed some copy/pasta in the test file docblock, let me quickly fix that. |
6586584
to
95afd97
Compare
Fixed now. |
Indeed it does. It should be in 3.6.0. Thanks. |
Looks like one of the builds stalled and needs to be retriggered. |
... which will indicate the `|` character in PHP 8.0 union types.
…NION This adds a new block of logic to the `PHP::processAdditional()` method which changes the token code and type of `T_BITWISE_OR` `|` tokens in type declarations to `T_TYPE_UNION`. As the `PHP::processAdditional()` method walks backwards through the token stack, the arrow function backfill will not have been done yet, so for those some special conditions have been put in place. I've tried to limit the token walking within the new block as much as possible while still maintaining accuracy. This includes changing all union type operators in a single type declaration in one go, instead of on each individual `T_BITWISE_OR` token, which prevents the same logic having to be executed multiple times for multi-union types like `int|float|null`. Includes dedicated unit tests. Ref: https://wiki.php.net/rfc/union_types_v2
…8 union types `array` keywords used as return types in PHP 8 union types would only be correctly changed to `T_STRING` if they were the first type in the union. Fixed now. Includes adding `T_STATIC` to the array of allowed tokens. While previously it wasn't an issue that the token was not included in the array, it is necessary for the token to be there to support union types. This change will be tested via the union type related tests for the `File::getMethodProperties()` method.
As the `PHP::processAdditional()` method walks backwards through the file, by the time the `fn` keyword backfill logic is hit, any union type `|` tokens will have already been converted to `T_TYPE_UNION`. So, to make the arrow function backfill compatible with PHP 8 union types, the `T_TYPE_UNION` token needs to be added to the "allowed tokens" (`$ignore`) array. Includes unit tests.
…ter types This adds support for union types to the `File::getMethodParameters()` method. Includes extensive unit tests.
… types This adds support for union types to the `File::getMethodProperties()` method. Includes extensive unit tests.
This adds support for union types to the `File::getMemberProperties()` method. Includes extensive unit tests.
56a65b2
to
53aa409
Compare
Rebased for merge conflicts with merged PR #3066 |
... to allow for the new `T_TYPE_UNION` token.
Merged 🎉 Thanks so much! This has gone and closed #2968 as well, which I think is okay now as everything mentioned on there is done. The sniffs themselves will need changes, but those feel like different issues for me to look at later. Can you check my logic on that please. |
@gsherwood #2968 shouldn't be closed yet, but then again, the two remaining PRs I had ready locally for that issue have now been pulled:
Based on my earlier examination, the only other PHPCS native sniff which will need adjusting is the I might, of course, have overlooked a sniff, but if that's the case, no doubt someone will report it soon enough 😄 |
Implements support for PHP 8.0 union types in the PHP tokenizer and the
File::getMethodParameters()
,File::getMethodProperties()
and theFile::getMemberProperties()
methods.Note: Individual sniffs which examine the parameter/return/property type returned by those methods will have to implement support for union types by using
explode('|', ltrim($type, '?'))
on the type returned by these methods to get to the individual types or walk the tokens from the start of the type to the end of the type.Ref: https://wiki.php.net/rfc/union_types_v2
Implementation as discussed in #2968. Partially fixes #2968.
Commit details
Tokens: add new
T_TYPE_UNION
token... which will indicate the
|
character in PHP 8.0 union types.Tokenizer/PHP: tokenize the "|" for union types as T_TYPE_UNION
This adds a new block of logic to the
PHP::processAdditional()
method which changes the token code and type ofT_BITWISE_OR
|
tokens in type declarations toT_TYPE_UNION
.As the
PHP::processAdditional()
method walks backwards through the token stack, the arrow function backfill will not have been done yet, so for those some special conditions have been put in place.I've tried to limit the token walking within the new block as much as possible while still maintaining accuracy.
This includes changing all union type operators in a single type declaration in one go, instead of on each individual
T_BITWISE_OR
token, which prevents the same logic having to be executed multiple times for multi-union types likeint|float|null
.Includes dedicated unit tests.
Tokenizer/PHP: array return type keyword to T_STRING vs PHP 8 union types
array
keywords used as return types in PHP 8 union types would only be correctly changed toT_STRING
if they were the first type in the union.Fixed now.
Includes adding
T_STATIC
to the array of allowed tokens. While previously it wasn't an issue that the token was not included in the array, it is necessary for the token to be there to support union types.This change will be tested via the union type related tests for the
File::getMethodProperties()
method.Tokenizer/PHP: arrow function backfill vs PHP8 union types
As the
PHP::processAdditional()
method walks backwards through the file, by the time thefn
keyword backfill logic is hit, any union type|
tokens will have already been converted toT_TYPE_UNION
.So, to make the arrow function backfill compatible with PHP 8 union types, the
T_TYPE_UNION
token needs to be added to the "allowed tokens" ($ignore
) array.Includes unit tests.
File::getMethodParameters(): add support for "union" parameter types
This adds support for union types to the
File::getMethodParameters()
method.Includes extensive unit tests.
File::getMethodProperties(): add support for "union" return types
This adds support for union types to the
File::getMethodProperties()
method.Includes extensive unit tests.
File::getMemberProperties(): add support for "union" types
This adds support for union types to the
File::getMemberProperties()
method.Includes extensive unit tests.
Docs: update "nullable_type" comments to clarify meaning
[EDIT] Added one more commit
Tokenizer/PHP: add new token to the $knownLengths property
[EDIT] And another one
Tests: update the Tokenizer/UndoNamespacedNameSingleToken test
... to allow for the new
T_TYPE_UNION
token. (Ref: #3063)