Skip to content

Tokenizer/PHP: T_GOTO_LABEL no longer contains colon #1016

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

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Apr 15, 2025

Description

⚠️ UPDATED: This PR depends on PR #1010, which needs to be merged first. ⚠️


When support for the goto construct was introduced, it was elected to tokenize the "goto target label" as a separate T_GOTO_LABEL token, including the colon following it.

This is wrong as PHP does not enforce for the colon to be the next token and allows whitespace and comments between the goto label and the colon. In which case, the tokenization in PHPCS would be incorrect as the label would stay a T_STRING instead of becoming T_GOTO_LABEL.

This commit fixes this.

It also introduces a new T_GOTO_COLON token and re-tokenizes the colon (separate from the label) to T_GOTO_COLON. This choice was made so as not to confuse the Tokenizer (nor sniffs) with yet another usage for T_COLON, which is an ambiguous enough token as it is (alternative control structures, switch - case statements, return type statements, enum types, ternaries etc).

This change is covered by the existing (updated) tests. Includes one minor update to a sniff to allow for this change.

Refs:

Suggested changelog entry

Changed:

  • T_GOTO_LABEL tokens will no longer include the colon following it.
    • The colon belonging with a goto label will now be tokenized separately as T_GOTO_COLON.

Fixed:

  • Fixed bug : goto labels were incorrectly tokenized as T_STRING if there was whitespace and/or comments between the label and colon.

Related issues/external references

Fixes squizlabs/PHP_CodeSniffer#3161
Fixes #185

@jrfnl jrfnl added this to the 4.0.0 milestone Apr 15, 2025
@jrfnl jrfnl force-pushed the phpcs-4.0/feature/sq-1612-stdout-vs-stderr branch from a78b0dd to c3fc316 Compare April 17, 2025 09:00
@jrfnl jrfnl force-pushed the phpcs-4.0/feature/sq-1612-stdout-vs-stderr branch 4 times, most recently from 547a345 to 93ce29b Compare April 17, 2025 13:00
Base automatically changed from phpcs-4.0/feature/sq-1612-stdout-vs-stderr to 4.x April 17, 2025 13:09
When support for the `goto` construct was introduced, it was elected to tokenize the "goto target label" as a separate `T_GOTO_LABEL` token, including the colon following it.

This is wrong as PHP does not enforce for the colon to be the next token and allows whitespace and comments between the goto label and the colon. In which case, the tokenization in PHPCS would be incorrect as the label would stay a `T_STRING` instead of becoming `T_GOTO_LABEL`.

This commit fixes this.

It also introduces a new `T_GOTO_COLON` token and re-tokenizes the colon (separate from the label) to `T_GOTO_COLON`.
This choice was made so as not to confuse the Tokenizer (nor sniffs) with yet another usage for `T_COLON`, which is an ambiguous enough token as it is (alternative control structures, switch - case statements, return type statements, enum types, ternaries etc).

This change is covered by the existing (updated) tests.
Includes one minor update to a sniff to allow for this change.

Refs:
* https://www.php.net/manual/en/control-structures.goto.php
* https://3v4l.org/ccbVD

Fixes squizlabs/PHP_CodeSniffer 3161
Fixes 185
@jrfnl jrfnl force-pushed the phpcs-4.0/feature/185-tokenizer-php-goto-label-tokenize-colon-separately branch from a349947 to ebff295 Compare April 17, 2025 14:31
@jrfnl
Copy link
Member Author

jrfnl commented Apr 17, 2025

Rebased without changes after the merge of #1010, which also gets rid of the merge conflict.

@jrfnl jrfnl merged commit f22e90b into 4.x Apr 17, 2025
62 checks passed
@jrfnl jrfnl deleted the phpcs-4.0/feature/185-tokenizer-php-goto-label-tokenize-colon-separately branch April 17, 2025 15:20
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