-
-
Notifications
You must be signed in to change notification settings - Fork 82
Tokenizer/PHP: improved tokenization of fully qualified exit/die/true/false/null #1206
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: improved tokenization of fully qualified exit/die/true/false/null #1206
Conversation
…/false/null As described in more detail in 1201, if the `exit`/`die`/`true`/`false`/`null` keywords were used in their fully qualified form, this was previously tokenized as `T_NS_SEPARATOR` + `T_STRING`, which was rarely, if ever, handled correctly by sniffs. This commit now changes the tokenization of fully qualified `exit`/`die`/`true`/`false`/`null` to be `T_NS_SEPARATOR` + the relevant dedicated token, i.e `T_EXIT`/`T_TRUE`/`T_FALSE` or `T_NULL`. Includes plenty of tests, including tests to safeguard against regressions which could be caused by this change in the "context sensitive keywords" layer and the "undo PHP 8.0+ namespaced names" layer. Fixes 1201 (for PHPCS 3.x).
This sniff checks for `if`/`elseif` which only contain "true" or "false", i.e. don't actually compare to anything. This commit fixes the sniff to still function correctly when it encounters "fully qualified true/false". Includes tests.
This commit fixes the sniff to handle "fully qualified true/false" the same as unqualified true/false. Includes tests.
…`null` These are the only two sniffs explicitly targetting `true`, `false` and `null`. This commit adds some tests to safeguard these sniffs function correctly on "fully qualified true/false/null".
This commit fixes the sniff to handle "fully qualified null" as a default value, the same as unqualified null. Includes tests.
This commit fixes the sniff to handle "fully qualified exit/die" the same as unqualified exit/die. Includes tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have looked through these changes on a call with @jrfnl.
FYI: I've also opened a PR to update the Dev upgrade guide for PHPCS 4.0 in the wiki: PHPCSStandards/PHP_CodeSniffer-documentation#53 |
The additional changes for 4.x have been applied in the "merge-up" commit for this PR: 5dace4b |
Description
This PR actions proposal #1201 and, for the sniffs, largely backports the tests previously added in the 4.x branch via #1042, though with different sniff changes.
Note: the previously made sniff changes for #1042 in the 4.x branch will for the most part be reverted when this PR is merged up into 4.x, as they will no longer be needed.
Additionally when this PR will be merged up into the 4.x branch:
Commits
Tokenizer/PHP: improved tokenization of fully qualified exit/die/true/false/null
As described in more detail in #1201, if the
exit
/die
/true
/false
/null
keywords were used in their fully qualified form, this was previously tokenized asT_NS_SEPARATOR
+T_STRING
, which was rarely, if ever, handled correctly by sniffs.This commit now changes the tokenization of fully qualified
exit
/die
/true
/false
/null
to beT_NS_SEPARATOR
+ the relevant dedicated token, i.eT_EXIT
/T_TRUE
/T_FALSE
orT_NULL
.Includes plenty of tests, including tests to safeguard against regressions which could be caused by this change in the "context sensitive keywords" layer and the "undo PHP 8.0+ namespaced names" layer.
Fixes #1201 (for PHPCS 3.x).
Related to #734
Generic/UnconditionalIfStatement: handle FQN true/false/null
This sniff checks for
if
/elseif
which only contain "true" or "false", i.e. don't actually compare to anything.This commit fixes the sniff to still function correctly when it encounters "fully qualified true/false".
Includes tests.
Generic/DisallowYodaConditions: handle FQN true/false/null
This commit fixes the sniff to handle "fully qualified true/false" the same as unqualified true/false.
Includes tests.
Generic/[Lower|Upper]CaseConstant: add tests with FQN true/false/
null
These are the only two sniffs explicitly targetting
true
,false
andnull
.This commit adds some tests to safeguard these sniffs function correctly on "fully qualified true/false/null".
PEAR/ValidDefaultValue: handle FQN null
This commit fixes the sniff to handle "fully qualified null" as a default value, the same as unqualified null.
Includes tests.
Squiz/ComparisonOperatorUsage: adds tests with FQN true/false/null
Squiz/NonExecutableCode: handle FQN exit/die
This commit fixes the sniff to handle "fully qualified exit/die" the same as unqualified exit/die.
Includes tests.
Related to #734
Suggested changelog entry
Added:
exit
/die
can now be used as fully qualified "function calls".Fixed:
true
/false
/null
was incorrectly tokenized asT_NS_SEPARATOR
+T_STRING
instead of asT_NS_SEPARATOR
+T_TRUE
/T_FALSE
/T_NULL
.true
/false
/null
correctly:Related issues/external references
Fixes #1201