Variables/ServerVariables: various sniff improvements#850
Merged
Conversation
* Expand the tests safeguarding against false positives.
* Ensure all five indices are covered by at least one test.
* Add some more variations to the pre-existing tests:
- Unconventional spacing.
- Comments in unexpected places.
The sniff looks for specific indexes/keys for array access. This commit fixes some documentation and variable names used to use this terminology correctly.
As things were, quotes _within_ a text string would also be stripped. Not that it's very likely for any of the `$_SERVER` keys to ever have these, but that's beside the point.
…ccess While it should probably be considered bad practice, it is allowed to declare an OO property called `$_SERVER` and this sniff should not get confused by that. Includes tests.
The code to find the array index was flawed and could walk beyond the brackets of this array access. Additionally, array access keys comprised of multiple tokens were not handled correctly. Includes tests. Note: WordPressCS has helper functions to retrieve the array access name, but those are marked as internal, which is the reason to introduce a custom function.
…SERVER']` The `$GLOBALS['_SERVER']` superglobals access is equivalent to using `$_SERVER`, so should be examined too. Includes tests.
jrfnl
added
Type: Enhancement
Type: Maintenance
Type: False positive
PHPCSUtils
Collaborator
Author
|
Note: It could be considered to also sniff for array indexes provided as nowdoc or heredoc without interpolation, but I expect that will rarely, if ever, be used, so I deemed updating the sniff to go that far over the top, especially considering this is not a security sniff. |
GaryJones
approved these changes
Jul 21, 2025
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.
Variables/ServerVariables: improve the tests
Variables/ServerVariables: use the correct terminology
The sniff looks for specific indexes/keys for array access.
This commit fixes some documentation and variable names used to use this terminology correctly.
Variables/ServerVariables: bug fix - incorrect quote stripping
As things were, quotes within a text string would also be stripped.
Not that it's very likely for any of the
$_SERVERkeys to ever have these, but that's beside the point.Variables/ServerVariables: bug fix - false positives on OO property access
While it should probably be considered bad practice, it is allowed to declare an OO property called
$_SERVERand this sniff should not get confused by that.Includes tests.
Variables/ServerVariables: bug fix - faulty array key determination
The code to find the array index was flawed and could walk beyond the brackets of this array access.
Additionally, array access keys comprised of multiple tokens were not handled correctly.
Includes tests.
Note: WordPressCS has helper functions to retrieve the array access name, but those are marked as internal, which is the reason to introduce a custom function.
Variables/ServerVariables: bug fix - false negatives for
$GLOBALS['_SERVER']The
$GLOBALS['_SERVER']superglobals access is equivalent to using$_SERVER, so should be examined too.Includes tests.
Closes #550