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 not applying tab replacement to heredoc/nowdoc closers #3639

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Jul 29, 2022

Since PHP 7.3, heredoc/nowdoc closers may be indented.
This indent can use either tabs or spaces and the indent is included in the T_END_HEREDOC/T_END_NOWDOC token contents as received from the PHP native tokenizer.

However, the PHPCS Tokenizer did not execute tab replacement on these token leading to unexpected 'content' and incorrect 'length' values in the File::$tokens array, which in turn could lead to incorrect sniff results and incorrect fixes.

This commit adds the T_END_HEREDOC/T_END_NOWDOC tokens to the array of tokens for which to do tab replacement to make them more consistent with the rest of PHPCS.

I also considered splitting the token into a T_WHITESPACE token and the T_END_HEREDOC/T_END_NOWDOC token, but that could potentially break sniffs which expect the T_END_HEREDOC/T_END_NOWDOC token directly after the last T_HEREDOC/T_NOWDOC token. The current fix does not contain that risk.

Includes unit tests safeguarding this change.

The tests will only run on PHP 7.3+ as flexible heredoc/nowdocs don't tokenize correctly in PHP < 7.3.

Since PHP 7.3, heredoc/nowdoc closers may be indented.
This indent can use either tabs or spaces and the indent is included in the `T_END_HEREDOC`/`T_END_NOWDOC` token contents as received from the PHP native tokenizer.

However, the PHPCS `Tokenizer` did no execute tab replacement on these token leading to unexpected `'content'` and incorrect `'length'` values in the `File::$tokens` array, which in turn could lead to incorrect sniff results and incorrect fixes.

This commit adds the `T_END_HEREDOC`/`T_END_NOWDOC` tokens to the array of tokens for which to do tab replacement to make them more consistent with the rest of PHPCS.

I also considered splitting the token into a `T_WHITESPACE` token and the `T_END_HEREDOC`/`T_END_NOWDOC`  token, but that could potentially break sniffs which expect the `T_END_HEREDOC`/`T_END_NOWDOC` token directly after the last `T_HEREDOC`/`T_NOWDOC` token. The current fix does not contain that risk.

Includes unit tests safeguarding this change.

The tests will only run on PHP 7.3+ as flexible heredoc/nowdocs don't tokenize correctly in PHP < 7.3.
@jrfnl jrfnl force-pushed the feature/tokenizer-improve-handling-php73-flexible-heredoc-nowdoc branch from e921934 to bdf5c49 Compare July 29, 2022 15:34
@gsherwood gsherwood added this to the 3.7.2 milestone Aug 14, 2022
@gsherwood gsherwood changed the title Tokenizer: apply tab replacement to heredoc/nowdoc closers Tokenizer not applying tab replacement to heredoc/nowdoc closers Oct 21, 2022
gsherwood added a commit that referenced this pull request Oct 21, 2022
@gsherwood gsherwood merged commit 64e548e into squizlabs:master Oct 21, 2022
@gsherwood
Copy link
Member

Thank you for fixing this

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.

2 participants