Skip to content

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

Merged
gsherwood merged 3 commits intosquizlabs:masterfrom
jrfnl:feature/3326-generic-multiplestatementalignment-bugfix
Jun 30, 2021
Merged

Tokenizer/PHP: fix tokenization of the default keyword#3351
gsherwood merged 3 commits intosquizlabs:masterfrom
jrfnl:feature/3326-generic-multiplestatementalignment-bugfix

Conversation

@jrfnl
Copy link
Copy Markdown
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
Copy Markdown
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
@gsherwood
Copy link
Copy Markdown
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

None yet

Development

Successfully merging this pull request may close these issues.

Generic.Formatting.MultipleStatementAlignment error with const DEFAULT

2 participants