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

Tokenizer/PHP: fix tokenization of the default keyword #3351

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented May 12, 2021

Generic/MultipleStatementAlignment: add extra tests

.. safeguarding the tokenizer fix which should prevent the issue as reported in #3326.

Tests: add extra tests for the default keyword tokenization

Tokenizer/PHP: fix tokenization of the default keyword

As per: #3326 (comment)

After PHP::tokenize(), the DEFAULT is still tokenized as T_DEFAULT. This causes the Tokenizer::recurseScopeMap() to assign it as the scope_opener to the ; semi-colon at the end of the constant declaration, with the class close curly brace being set as the scope_closer.
In the PHP::processAdditional() method, the DEFAULT is subsequently retokenized to T_STRING as it is preceded by a const keyword, but that is too late.

The scope_opener being set on the semi-colon is what is causing the errors to be displayed for the above code sample.

The commit fixes this by:

  1. Abstracting the list of T_STRING contexts out to a class property.
  2. Using the list from the property in all places in the Tokenizer\PHP class where keyword tokens are (potentially) being re-tokenized to T_STRING, including in the T_DEFAULT tokenization code which was added to address the PHP 8.0 match expressions.
    Note: the issue was not introduced by the match related code, however, that code being there does make it relatively easy now to fix this particular case.

While this doesn't address #3336 yet, it is a step towards addressing it and will sort out one of the most common causes for bugs.

Fixes #3326

Closes #3340 as superseded

jrfnl added 3 commits May 27, 2021 13:56
.. safeguarding the tokenizer fix which should prevent the issue as reported in 3326.
As per: squizlabs#3326 (comment)

> After `PHP::tokenize()`, the `DEFAULT` is still tokenized as `T_DEFAULT`. This causes the `Tokenizer::recurseScopeMap()` to assign it as the `scope_opener` to the `;` semi-colon at the end of the constant declaration, with the class close curly brace being set as the `scope_closer`.
> In the `PHP::processAdditional()` method, the `DEFAULT` is subsequently retokenized to `T_STRING` as it is preceded by a `const` keyword, but that is too late.
>
> The `scope_opener` being set on the semi-colon is what is causing the errors to be displayed for the above code sample.

The commit fixes this by:
1. Abstracting the list of `T_STRING` contexts out to a class property.
2. Using the list from the property in all places in the `Tokenizer\PHP` class where keyword tokens are (potentially) being re-tokenized to `T_STRING`, including in the `T_DEFAULT` tokenization code which was added to address the PHP 8.0 `match` expressions.
    Note: the issue was not introduced by `match` related code, however, that code being there does make it relatively easy now to fix this particular case.

While this doesn't address 3336 yes, it is a step towards addressing it and will sort out one of the most common causes for bugs.
@jrfnl jrfnl force-pushed the feature/3326-generic-multiplestatementalignment-bugfix branch from a2890a7 to 0d5663c Compare May 27, 2021 11:58
@jrfnl
Copy link
Contributor Author

jrfnl commented May 27, 2021

Rebased to get round merge conflict related to #3340 having been merged.

gsherwood added a commit that referenced this pull request Jun 30, 2021
@gsherwood gsherwood merged commit 7559324 into squizlabs:master Jun 30, 2021
PHPCS v3 Development automation moved this from Idea Bank to Ready for Release Jun 30, 2021
@gsherwood
Copy link
Member

Thanks. Sorry I didn't notice Closes #3340 as superseded before merging that other one.

@jrfnl jrfnl deleted the feature/3326-generic-multiplestatementalignment-bugfix branch June 30, 2021 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
PHPCS v3 Development
Ready for Release
Development

Successfully merging this pull request may close these issues.

Generic.Formatting.MultipleStatementAlignment error with const DEFAULT
2 participants