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.

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 | "undo" namespaced names as single token #3063

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Aug 30, 2020

As per the proposal in #3041.

This effectively "undoes" the new PHP 8.0 tokenization of identifier names for PHPCS 3.x.

Includes extensive unit tests to ensure the correct re-tokenization as well as that the rest of the tokenization is not adversely affected by this change.

Includes preventing function ... within a group use statement from breaking the retokenization.

Includes fixing the nullable tokenization when combined with any of the new PHP 8 identifier name tokens.

Fixes #3041 for PHPCS 3.x

@jrfnl jrfnl force-pushed the feature/3041-backport-old-identifier-tokenization-php-8 branch 2 times, most recently from 7c1f941 to d5c858a Compare August 30, 2020 18:15
@jrfnl
Copy link
Contributor Author

jrfnl commented Aug 30, 2020

Found a few more small gotcha's, not directly related to the PHP 8 change, but for things the PHPCS tokenizer just didn't take into account while it should, which came to light while working on this.

Working on adding support for those now too.

@jrfnl jrfnl force-pushed the feature/3041-backport-old-identifier-tokenization-php-8 branch from d5c858a to 31e3ba0 Compare August 30, 2020 23:05
@jrfnl jrfnl force-pushed the feature/3041-backport-old-identifier-tokenization-php-8 branch from 31e3ba0 to 9dd420e Compare August 30, 2020 23:27
@jrfnl
Copy link
Contributor Author

jrfnl commented Aug 30, 2020

Opened PR #3066 for the other gotcha's.

@jrfnl
Copy link
Contributor Author

jrfnl commented Aug 30, 2020

Note: the changes in this PR should NOT be cherry-picked to the PHPCS 4.x branch. I will submit a separate PR for PHPCS 4.x as per the proposal in #3041.

@jrfnl jrfnl changed the title PHP 8.0: "undo" namespaced names as single token PHP 8.0 | "undo" namespaced names as single token Aug 30, 2020
@kukulich
Copy link
Contributor

Just tested with Slevomat CS and it works perfect!

@jrfnl
Copy link
Contributor Author

jrfnl commented Aug 31, 2020

@kukulich Thanks for testing this!

@gsherwood gsherwood added this to Idea Bank in PHPCS v3 Development via automation Aug 31, 2020
@gsherwood gsherwood added this to the 3.5.7 milestone Aug 31, 2020
@jrfnl jrfnl force-pushed the feature/3041-backport-old-identifier-tokenization-php-8 branch from 9dd420e to 083573d Compare September 2, 2020 00:08
@jrfnl
Copy link
Contributor Author

jrfnl commented Sep 2, 2020

Rebased to fix merge conflict.

@jrfnl jrfnl force-pushed the feature/3041-backport-old-identifier-tokenization-php-8 branch from 083573d to ba63323 Compare September 2, 2020 11:00
As per the proposal in 3041.

This effectively "undoes" the new PHP 8.0 tokenization of identifier names for PHPCS 3.x.

Includes extensive unit tests to ensure the correct re-tokenization as well as that the rest of the tokenization is not adversely affected by this change.

Includes preventing `function ...` within a group use statement from breaking the retokenization.

Includes fixing the nullable tokenization when combined with any of the new PHP 8 identifier name tokens.
jrfnl added a commit to PHPCSStandards/PHPCSUtils that referenced this pull request Sep 13, 2020
In PHP 8.0 identifier name tokenization will change as outline in the [accepted RFC "Treat namespaced names as single token"(https://wiki.php.net/rfc/namespaced_names_as_token).

When the PHP 8.0 identifier name tokenization is used, the target token to find for some tests will need to be a different token - for instance: `T_STRING` vs `T_FULLY_QUALIFIED_NAME` -.
Along the same lines, the expected token positions in the return value of various functions will also be different when the PHP < 8.0 tokenization is used as certain tokens will be "squashed" into one token.

This adds a test helper method to allow tests to "know" whether or not to expect the PHP 8.0 identifier name tokenization, so the test setup/expectations can be adjusted based on the expected tokenization.

The method is based on the _current reality_.
At this time the PHP 8 tokenization should be expected on all PHPCS versions when run on PHP 8.

Refs:
* https://wiki.php.net/rfc/namespaced_names_as_token
* [Proposal for handling this PHP 8 change in PHPCS](squizlabs/PHP_CodeSniffer#3041)
* [Open PR for the PHPCS 3.x branch to "undo" the PHP 8 tokenization](squizlabs/PHP_CodeSniffer#3063)
jrfnl added a commit to PHPCSStandards/PHPCSUtils that referenced this pull request Sep 13, 2020
In PHP 8.0 identifier name tokenization will change as outline in the [accepted RFC "Treat namespaced names as single token"](https://wiki.php.net/rfc/namespaced_names_as_token).

When the PHP 8.0 identifier name tokenization is used, the target token to find for some tests will need to be a different token - for instance: `T_STRING` vs `T_FULLY_QUALIFIED_NAME` -.
Along the same lines, the expected token positions in the return value of various functions will also be different when the PHP < 8.0 tokenization is used as certain tokens will be "squashed" into one token.

This adds a test helper method to allow tests to "know" whether or not to expect the PHP 8.0 identifier name tokenization, so the test setup/expectations can be adjusted based on the expected tokenization.

The method is based on the _current reality_.
At this time the PHP 8 tokenization should be expected on all PHPCS versions when run on PHP 8.

Refs:
* https://wiki.php.net/rfc/namespaced_names_as_token
* [Proposal for handling this PHP 8 change in PHPCS](squizlabs/PHP_CodeSniffer#3041)
* [Open PR for the PHPCS 3.x branch to "undo" the PHP 8 tokenization](squizlabs/PHP_CodeSniffer#3063)
gsherwood added a commit that referenced this pull request Sep 20, 2020
@gsherwood gsherwood merged commit 6525028 into squizlabs:master Sep 20, 2020
PHPCS v3 Development automation moved this from Idea Bank to Ready for Release Sep 20, 2020
@gsherwood
Copy link
Member

Thanks so much for this change

@jrfnl jrfnl deleted the feature/3041-backport-old-identifier-tokenization-php-8 branch September 20, 2020 22:56
jrfnl added a commit to PHPCSStandards/PHPCSUtils that referenced this pull request Sep 20, 2020
Upstream PR 3063 which was merged in PHPCS 3.5.7 effectively "undoes" the PHP 8.0 tokenization for identifier names for PHPCS 3.x, while it also includes backfilling the PHP 8.0 token constants.

In PHPCS 4.x the PHP 8.0 tokenization will be backfilled instead.

This commit annotates this change in the `Collections::nameTokens()` method and updates the unit tests for various token collections to handle this correctly as well.

Refs:
* squizlabs/PHP_CodeSniffer#3041
* squizlabs/PHP_CodeSniffer#3063
jrfnl added a commit to PHPCSStandards/PHPCSUtils that referenced this pull request Sep 21, 2020
Upstream PR 3063 which was merged in PHPCS 3.5.7 effectively "undoes" the PHP 8.0 tokenization for identifier names for PHPCS 3.x, while it also includes backfilling the PHP 8.0 token constants.

In PHPCS 4.x the PHP 8.0 tokenization will be backfilled instead, but that PR has not yet been pulled.

This commit annotates the current change in the `Collections::nameTokens()` method and updates the unit tests for various token collections to handle this correctly as well.

Refs:
* squizlabs/PHP_CodeSniffer#3041
* squizlabs/PHP_CodeSniffer#3063
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
PHPCS v3 Development
Ready for Release
Development

Successfully merging this pull request may close these issues.

PHP 8.0 | Proposal for handling "namespaced names as single token"
3 participants