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

NewKeywords: fix too aggressive accounting for PHPCS cross-compat #627

Merged
merged 1 commit into from
Mar 23, 2018

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Mar 19, 2018

A number of tokens are not correctly back-filled across PHP versions by various PHPCS versions.
To that end, a token translation table is used to recognize tokens tokenized as T_STRING as the keyword anyway.

At the same time, PHPCS sometimes retokenizes too aggressively, changing T_STRING tokens to a keyword token when it shouldn't.

Most PHP keywords can be safely used as variable (and property) names and as of PHP 7, as the name of class constants.

Using them as variable names is generally OK, but could lead to confusion.

Ref: http://php.net/manual/en/reserved.keywords.php

As property names when referenced and class constant names, both when referenced as well as when declared, are tokenized as T_STRING, this leads to false positives from this sniff.

At the same time, keywords in PHP are case-insensitive, so the cross-compat layer should allow for mixed case / uppercase use of the (non magic-constant) keywords as well.

This commit fixes both.
Includes unit tests.

A number of tokens are not correctly back-filled across PHP versions by PHPCS.
To that end, a _token translation table_ is used to recognize tokens tokenized as `T_STRING` as the keyword anyway.

At the same time, PHPCS retokenizes sometimes too agressively.

Having said that, most PHP keywords can be safely used as variable (and property) names and as of PHP 7, as the name of class constants.
However, these are tokenized as `T_STRING` which leads to false positives from this sniff.

At the same time, keywords in PHP are case-insensitive, so the cross-compat layer should allow for mixed case / uppercase use of the (non magic-constant) keywords as well.

This commit fixes both.
Includes unit tests.
@jrfnl jrfnl added the bug label Mar 19, 2018
@jrfnl jrfnl added this to the 8.2.0 milestone Mar 19, 2018
@jrfnl jrfnl requested a review from wimg March 19, 2018 04:04
@wimg wimg merged commit 129afa0 into master Mar 23, 2018
@wimg wimg deleted the feature/newkeywords-bugfix branch March 23, 2018 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants