Add check for deprecated define variables #162
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
| $ret = true; | ||
|
|
||
| $checks = array( | ||
| array( "NO_HEADER_TEXT" => "add_theme_support( 'custom-header' )", '3.4' ), |
There was a problem hiding this comment.
The WP coding standards is to use single quotes in array instead of double.
|
It may be easier to extend an existing class constants.php |
| checkcount(); | ||
| $key = key( $check ); | ||
| $alt = $check[ $key ]; | ||
| if ( preg_match( '/' . $key . '/', $phpfile, $matches ) ) { |
There was a problem hiding this comment.
You could try preg_match( '/(?:'|")' . $key . '(?:'|")/', $phpfile, $matches )
jrfnl
added a commit
to WordPress/WordPress-Coding-Standards
that referenced
this pull request
Aug 5, 2017
This sniff is based on a sniff previously pulled to the TRT fork of WPCS as `Theme.DeprecatedConstants`. Related issue: WPTT/WPThemeReview/issues/23 Original PR: WPTT/WPThemeReview/pull/30 Covers the list of constants found in: WordPress/theme-check/pull/162 Differences between that sniff and this one: * This sniff is a **_lot_** more comprehensive in preventing false positives. The original sniff was one of the first ones I wrote and it's kind of sweet to look back at that code which I wrote over a year ago and see how much I've learned about writing sniffs in the mean time. * The original sniff was called `Theme.DeprecatedConstants`, but in reality, most of these constants aren't officially deprecated, though their usage is discouraged and the constants being addressed are not 100% related to themes either. With that in mind, I've renamed the sniff to `WP.DiscouragedConstants` and changed the error level from Error to Warning. Also see: https://core.trac.wordpress.org/ticket/18298 * This version of the sniff also addresses the concerns raised in WPTT/WPThemeReview/issues/110 about themes `define`-ing any of these constants and leverages the `AbstractFunctionParameterSniff` to do so. Includes quite extensive unit tests. More are definitely welcome, especially to prevent false positives. I've added the sniff to the `WordPress-Extra` ruleset. Fixes 97 Will also fix WPTT/WPThemeReview/issues/110 once the original sniff gets removed from the TRT fork and this sniff is added to the TRT ruleset instead.
jrfnl
added a commit
to WordPress/WordPress-Coding-Standards
that referenced
this pull request
Aug 5, 2017
This sniff is based on a sniff previously pulled to the TRT fork of WPCS as `Theme.DeprecatedConstants`. Related issue: WPTT/WPThemeReview/issues/23 Original PR: WPTT/WPThemeReview/pull/30 Covers the list of constants found in: WordPress/theme-check/pull/162 Differences between that sniff and this one: * This sniff is a **_lot_** more comprehensive in preventing false positives. The original sniff was one of the first ones I wrote and it's kind of sweet to look back at that code which I wrote over a year ago and see how much I've learned about writing sniffs in the mean time. * The original sniff was called `Theme.DeprecatedConstants`, but in reality, most of these constants aren't officially deprecated, though their usage is discouraged and the constants being addressed are not 100% related to themes either. With that in mind, I've renamed the sniff to `WP.DiscouragedConstants` and changed the error level from Error to Warning. Also see: https://core.trac.wordpress.org/ticket/18298 * This version of the sniff also addresses the concerns raised in WPTT/WPThemeReview/issues/110 about themes `define`-ing any of these constants and leverages the `AbstractFunctionParameterSniff` to do so. Includes quite extensive unit tests. More are definitely welcome, especially to prevent false positives. I've added the sniff to the `WordPress-Extra` ruleset. Based on https://vip.wordpress.com/documentation/code-review-what-we-look-for/#using-theme-constants and https://core.trac.wordpress.org/ticket/18298#comment:2, I've also added the sniff to the `WordPress-VIP` ruleset. /cc @sboisvert @david-binda @ntwb Fixes 97 Will also fix WPTT/WPThemeReview/issues/110 once the original sniff gets removed from the TRT fork and this sniff is added to the TRT ruleset instead.
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.
See #152
Current limitation:
Theme's functions.php
Output:
Although there is no
HEADER_IMAGEvariable, regex matches withHEADER_IMAGE_WIDTH. Any suggestion how to tighten the regex check?