Deprecate use of the WPCS native whitelist comments #1580
Merged
Conversation
This file contains 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
…elist comments (PHPCS 3.2.0)
The WPCS native whitelist comments were introduced for lack of the ability to selectively ignore one or more sniffs for a piece of code.
PHPCS 3.2.0 introduced native PHPCS annotations which allow for selective ignoring/disabling of sniffs and while those were originally a bit buggy, all known bugs have been fixed since, making the WPCS native whitelist comments redundant.
This commit deprecates the method within WPCS which is used to check for the WPCS native whitelist comments.
It also introduces a warning which will be thrown whenever a WPCS native whitelist comment is encountered to alert people to this change and encourage them to switch over to the PHPCS native annotations.
Notes:
* No deprecation notice will be thrown when a WPCS whitelist comment is combined with a PHPCS native annotation, i.e. `// phpcs:ignore WP.Security.EscapeOutput -- WPCS: XSS ok.`
* As a sniff may be triggered several times for different tokens, a light weight (static) cache variable has been introduced to prevent the deprecation notice being thrown more than once for each WPCS whitelist comment encountered.
The `Sniff::has_whitelist_comment()` method and all calls to it should be removed in WPCS 3.0.0.
- [ ] References to the WPCS native whitelist comments need to be removed from the wiki and/or replaced with an upgrade guide.
- [ ] Open an issue about removing the `Sniff::has_whitelist_comment()` method and calls to it in WPCS 3.0.0.
Note: all unit tests testing that the WPCS native whitelist comments are being respected should also be removed at that time. Or alternatively, they can remain and it should be verified that the WPCS native whitelist comment makes no difference anymore.
Refs:
* squizlabs/PHP_CodeSniffer 604
The deprecation notice in this case is actually a false positive, however as the WPCS native whitelisting is now deprecated, fixing that false positive is a moot point.
…elist comment As the whole WPCS whitelist comment system is now deprecated, a notice will be thrown about that comment already. No need to also throw one about the comment using an old whitelist comment.
…tices A number of unit test case files contain WPCS whitelist comments to test that those are correctly being respected. As of now, a deprecation warning for use of the WPCS whitelist comments will be thrown on those lines, so we need to make sure that PHPCS expects those warnings.
GaryJones
approved these changes
Dec 21, 2018
This was referenced Dec 21, 2018
2 tasks
Draft
12 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Sniff::has_whitelist_comment(): Deprecate use of the WPCS native whitelist comments (PHPCS 3.2.0)
The WPCS native whitelist comments were introduced for lack of the ability to selectively ignore one or more sniffs for a piece of code.
PHPCS 3.2.0 introduced native PHPCS annotations which allow for selective ignoring/disabling of sniffs and while those were originally a bit buggy, all known bugs have been fixed since, making the WPCS native whitelist comments redundant.
This commit deprecates the method within WPCS which is used to check for the WPCS native whitelist comments.
It also introduces a warning which will be thrown whenever a WPCS native whitelist comment is encountered to alert people to this change and encourage them to switch over to the PHPCS native annotations.
Notes:
// phpcs:ignore WP.Security.EscapeOutput -- WPCS: XSS ok.The
Sniff::has_whitelist_comment()method and all calls to it should be removed in WPCS 3.0.0.Sniff::has_whitelist_comment()method and calls to it in WPCS 3.0.0.Note: all unit tests testing that the WPCS native whitelist comments are being respected should also be removed at that time. Or alternatively, they can remain and it should be verified that the WPCS native whitelist comment makes no difference anymore.
Refs:
WPCS native PHPCS ruleset: ignore a whitelist deprecation notice
The deprecation notice in this case is actually a false positive, however as the WPCS native whitelisting is now deprecated, fixing that false positive is a moot point.
SlowDBQuery: remove the warning about the deprecated
tax_querywhitelist commentAs the whole WPCS whitelist comment system is now deprecated, a notice will be thrown about that comment already. No need to also throw one about the comment using an old whitelist comment.
Unit tests: account for the new WPCS whitelist comment deprecation notices
A number of unit test case files contain WPCS whitelist comments to test that those are correctly being respected.
As of now, a deprecation warning for use of the WPCS whitelist comments will be thrown on those lines, so we need to make sure that PHPCS expects those warnings.