Conversation
Member
Author
|
I'm going to presume that nobody has looked at this PR yet nor started reviewing and will amend a couple of the commits with yet more constants to sniff for... ;-) |
03e567c to
b2d7174
Compare
… precise The `getTargetToken()` method is used to find a token in a test case file based on a preceding comment. In case the comment was not found, this method could inadvertently return a stackPtr to a completely different token, which could - by accident, not design - pass the test. In case the target token was not found before the next `/* Case ... */` comment, the method could inadvertently return a stackPtr to the token for a next case, which could - by accident, not design - pass the test. Both these cases have now been addressed. Additionally, the method now allows for optionally passing a `$tokenContent` parameter to target one specific token in a code snippet which contains several tokens of the same type.
Includes unit tests for this new method.
The list of constants is based on the various migration guides, the changelog and the reserved constants list in the PHP Manual. As the `T_STRING` token is used as a kind of "catch-all", there may initially be false positives for code structures I did not consider. All the same, chances are small as that would also require the `T_STRING` to have the same content and case as the PHP native constant name. This sniff does not automatically detect back-fills, but the error codes are modular, so if a PHP constant back-fill is in place in a certain project, that message for that particular constant can be excluded from the custom ruleset. Includes unit tests. The sniff also does not detect clashes between aliased constants and the PHP constants, i.e. `use const ABC as NEW_CONSTANT;`. It would be an interesting potential future enhancement to detect these kind of aliases and see if we can on-the-fly exclude the current file being examined for that particular error code.
…ved PHP constants Sister-sniff to the new `NewConstants` sniff.
…be available While PHPCS tries to back-fill for PHP native tokenizer constants, it does not do so consistently in all PHPCS versions and for all tokens. As PHPCompatibility still supports PHPCS 1.x and 2.x, using a little more defensive coding is warranted.
b2d7174 to
bb34630
Compare
Member
Author
|
Rebased after the merge of #525 so the build can pass ;-) |
wimg
approved these changes
Nov 12, 2017
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR> This PR adds two new sniffs,
NewConstantsandRemovedConstants, a newSniff::isUseOfGlobalConstant()utility function and improves the accuracy of theMethodTestFrame::getTargetToken()method.The PR will be easiest to review by looking at the individual commits.
The lists of constants used has been collected over time and is based on the various migration guides, the changelog and the reserved constants list in the PHP Manual. See below for more detail.
As the
T_STRINGtoken is used as a kind of "catch-all", there may initially be false positives for code structures I did not consider. All the same, chances are small as that would also require theT_STRINGto have the same content and case as the PHP native constant name.Any false positives found, should be addressed in the new
Sniff::isUseOfGlobalConstant()method and additional unit tests should be added to the dedicated unit test file for that function.Both the introduction of this new function and the initial unit tests for it are part of this PR.
Note: this sniff currently only handles global constants. It does not deal with class constants as may be provided by PHP Extensions.
This sniff does not automatically detect PHP constants which have been back-filled, but the error codes are modular, so if a PHP constant back-fill is in place in a certain project, the message for that particular constant can easily be excluded from the custom ruleset.
Includes unit tests.
Note: Most of the code for the new
Sniff::isUseOfGlobalConstant()method is code I've previously written for a slightly similar sniff in the WordPress Coding Standards.As I wrote that code and nobody else has touched it since, AFAIK we're ok regarding the licensing of the code as I'm pulling this PR. Nevertheless, I figured I ought to mention it.
The sniff also does not detect clashes between aliased constants and the PHP constants, i.e.
use const ABC as NEW_CONSTANT;.It would be an interesting potential future enhancement to detect these kind of aliases and see if we can on-the-fly exclude the current file being examined for that particular error code, but that's for a future PR.
Fixes #263
N.B.: Merging PR #525 is a prerequisite for the unit tests for this PR to be able to pass as it addresses a bug I discovered while working on this PR.
I've tested this and a passing build which includes the fix from PR #525 can be seen here: https://travis-ci.org/jrfnl/PHPCompatibility/builds/293000017
For the
NewConstantssniff the following sources have been used:For the
RemovedConstantssniff:These sniffs could be further improved by going through the "Predefined constants" pages of all PHP extensions.Done