Skip to content

Commit

Permalink
✨ New WordPress.WP.DiscouragedConstants sniff
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jrfnl committed Aug 5, 2017
1 parent d2f45d8 commit 3c7d0c4
Show file tree
Hide file tree
Showing 4 changed files with 346 additions and 0 deletions.
1 change: 1 addition & 0 deletions WordPress-Extra/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
<rule ref="WordPress.WP.DeprecatedParameters"/>
<rule ref="WordPress.WP.AlternativeFunctions"/>
<rule ref="WordPress.WP.DiscouragedFunctions"/>
<rule ref="WordPress.WP.DiscouragedConstants"/>

<rule ref="Squiz.PHP.Eval"/>
<rule ref="Squiz.PHP.Eval.Discouraged">
Expand Down
214 changes: 214 additions & 0 deletions WordPress/Sniffs/WP/DiscouragedConstantsSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
<?php
/**
* WordPress Coding Standard.
*
* @package WPCS\WordPressCodingStandards
* @link https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards
* @license https://opensource.org/licenses/MIT MIT
*/

namespace WordPress\Sniffs\WP;

use WordPress\AbstractFunctionParameterSniff;
use PHP_CodeSniffer_Tokens as Tokens;

/**
* Warns against usage of discouraged WP CONSTANTS and recommends alternatives.
*
* @package WPCS\WordPressCodingStandards
*
* @since 0.14.0
*/
class DiscouragedConstantsSniff extends AbstractFunctionParameterSniff {

/**
* List of discouraged WP constants and their replacements.
*
* @since 0.14.0
*
* @var array
*/
protected $discouraged_constants = array(
'STYLESHEETPATH' => 'get_stylesheet_directory()',
'TEMPLATEPATH' => 'get_template_directory()',
'PLUGINDIR' => 'WP_PLUGIN_DIR',
'MUPLUGINDIR' => 'WPMU_PLUGIN_DIR',
'HEADER_IMAGE' => 'add_theme_support( \'custom-header\' )',
'NO_HEADER_TEXT' => 'add_theme_support( \'custom-header\' )',
'HEADER_TEXTCOLOR' => 'add_theme_support( \'custom-header\' )',
'HEADER_IMAGE_WIDTH' => 'add_theme_support( \'custom-header\' )',
'HEADER_IMAGE_HEIGHT' => 'add_theme_support( \'custom-header\' )',
'BACKGROUND_COLOR' => 'add_theme_support( \'custom-background\' )',
'BACKGROUND_IMAGE' => 'add_theme_support( \'custom-background\' )',
);

/**
* Array of functions to check.
*
* @since 0.14.0
*
* @var array <string function name> => <int parameter position>
*/
protected $target_functions = array(
'define' => 1,
);

/**
* Array of tokens which if found preceding the $stackPtr indicate that a T_STRING is not a constant.
*
* @var array
*/
private $preceding_tokens_to_ignore = array(
T_NAMESPACE => true,
T_USE => true,
T_CLASS => true,
T_TRAIT => true,
T_INTERFACE => true,
T_EXTENDS => true,
T_IMPLEMENTS => true,
T_NEW => true,
T_FUNCTION => true,
T_CONST => true,
T_DOUBLE_COLON => true,
T_OBJECT_OPERATOR => true,
T_INSTANCEOF => true,
T_GOTO => true,

);

/**
* Processes this test, when one of its tokens is encountered.
*
* @since 0.14.0
*
* @param int $stackPtr The position of the current token in the stack.
*
* @return int|void Integer stack pointer to skip forward or void to continue
* normal file processing.
*/
public function process_token( $stackPtr ) {
if ( isset( $this->target_functions[ strtolower( $this->tokens[ $stackPtr ]['content'] ) ] ) ) {
// Disallow excluding function groups for this sniff.
$this->exclude = '';

return parent::process_token( $stackPtr );

} else {
return $this->process_arbitrary_tstring( $stackPtr );
}
}

/**
* Process an arbitrary T_STRING token to determine whether it is one of the target constants.
*
* @since 0.14.0
*
* @param int $stackPtr The position of the current token in the stack.
*
* @return void
*/
public function process_arbitrary_tstring( $stackPtr ) {
$content = $this->tokens[ $stackPtr ]['content'];

if ( ! isset( $this->discouraged_constants[ $content ] ) ) {
return;
}

$next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
if ( false !== $next && T_OPEN_PARENTHESIS === $this->tokens[ $next ]['code'] ) {
// Function call or declaration.
return;
}

$prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true );
if ( false !== $prev && isset( $this->preceding_tokens_to_ignore[ $this->tokens[ $prev ]['code'] ] ) ) {
// Not the use of a constant.
return;
}

if ( false !== $prev
&& T_NS_SEPARATOR === $this->tokens[ $prev ]['code']
&& T_STRING === $this->tokens[ ( $prev - 1 ) ]['code']
) {
// Namespaced constant of the same name.
return;
}

/*
* Deal with a number of variations of use statements.
*/
for ( $i = $stackPtr; $i > 0; $i-- ) {
if ( $this->tokens[ $i ]['line'] !== $this->tokens[ $stackPtr ]['line'] ) {
break;
}
}

$first_on_line = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $i + 1 ), null, true );
if ( false !== $first_on_line && T_USE === $this->tokens[ $first_on_line ]['code'] ) {
$next_on_line = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $first_on_line + 1 ), null, true );
if ( false !== $next_on_line ) {
if ( ( T_STRING === $this->tokens[ $next_on_line ]['code']
&& 'const' === $this->tokens[ $next_on_line ]['content'] )
|| T_CONST === $this->tokens[ $next_on_line ]['code'] // Happens in some PHPCS versions.
) {
$has_ns_sep = $this->phpcsFile->findNext( T_NS_SEPARATOR, ( $next_on_line + 1 ), $stackPtr );
if ( false !== $has_ns_sep ) {
// Namespaced const (group) use statement.
return;
}
} else {
// Not a const use statement.
return;
}
}
}

// Ok, this is really one of the discouraged constants.
$this->phpcsFile->addWarning(
'Found usage of constant "%s". Use %s instead.',
$stackPtr,
'UsageFound',
array(
$content,
$this->discouraged_constants[ $content ],
)
);
}

/**
* Process the parameters of a matched `define` function call.
*
* @since 0.14.0
*
* @param int $stackPtr The position of the current token in the stack.
* @param array $group_name The name of the group which was matched.
* @param string $matched_content The token content (function name) which was matched.
* @param array $parameters Array with information about the parameters.
*
* @return void
*/
public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) {
$function_name = strtolower( $matched_content );
$target_param = $this->target_functions[ $function_name ];

// Was the target parameter passed ?
if ( ! isset( $parameters[ $target_param ] ) ) {
return;
}

$raw_content = $this->strip_quotes( $parameters[ $target_param ]['raw'] );

if ( isset( $this->discouraged_constants[ $raw_content ] ) ) {
$this->phpcsFile->addWarning(
'Found declaration of constant "%s". Use %s instead.',
$stackPtr,
'DeclarationFound',
array(
$raw_content,
$this->discouraged_constants[ $raw_content ],
)
);
}
}

} // End class.
73 changes: 73 additions & 0 deletions WordPress/Tests/WP/DiscouragedConstantsUnitTest.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<?php

/*
* Make sure that global constants are correctly identified.
*
* The below are all ok.
*/
// Ok, not a constant.
namespace STYLESHEETPATH {}
namespace MY\OTHER\STYLESHEETPATH\NAMESPACE {}
use STYLESHEETPATH;
use Something, STYLESHEETPATH, SomethingElse;
class STYLESHEETPATH {
const STYLESHEETPATH = 'something';
private function STYLESHEETPATH() {}
}
class ABC extends STYLESHEETPATH {}
class DEF implements STYLESHEETPATH {}
interface STYLESHEETPATH {}
trait STYLESHEETPATH {}
const STYLESHEETPATH = 'something';
$a = new STYLESHEETPATH;
$a = new STYLESHEETPATH();
function STYLESHEETPATH() {}
echo STYLESHEETPATH();
echo My\UsedAsNamespace\STYLESHEETPATH\something;
My\UsedAsNamespace\STYLESHEETPATH\something::something_else();
if ( $abc instanceof STYLESHEETPATH ) {}

goto STYLESHEETPATH;
// Something.
STYLESHEETPATH:
// Something.

// Ok, not a global constant.
echo \mynamespace\STYLESHEETPATH;
echo My_Class::STYLESHEETPATH;
echo $this->STYLESHEETPATH;
use const SomeNamespace\STYLESHEETPATH as SSP; // PHP 5.6+
use const SomeNamespace\{STYLESHEETPATH, TEMPLATEPATH}; // PHP 7.0+
define( 'My\STYLESHEETPATH', 'something' );

// Ok, not usage of the constant as such.
if ( defined( 'STYLESHEETPATH' ) ) { // Ok.
// Do something unrelated.
}


/*
* These are all bad.
*/
echo STYLESHEETPATH;
echo \STYLESHEETPATH; // Global constant.
$folder = basename( TEMPLATEPATH );
include PLUGINDIR . '/js/myfile.js';
echo MUPLUGINDIR;
echo HEADER_IMAGE;
echo NO_HEADER_TEXT;
echo HEADER_TEXTCOLOR;
echo HEADER_IMAGE_WIDTH;
echo HEADER_IMAGE_HEIGHT;
echo BACKGROUND_COLOR;
echo BACKGROUND_IMAGE;

use const STYLESHEETPATH as SSP;
use const ABC as STYLESHEETPATH;

switch( STYLESHEETPATH ) {
case STYLESHEETPATH:
break;
}

define( 'STYLESHEETPATH', 'something' );
58 changes: 58 additions & 0 deletions WordPress/Tests/WP/DiscouragedConstantsUnitTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php
/**
* Unit test class for WordPress Coding Standard.
*
* @package WPCS\WordPressCodingStandards
* @link https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards
* @license https://opensource.org/licenses/MIT MIT
*/

namespace WordPress\Tests\WP;

use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;

/**
* Unit test class for the WP_DiscouragedConstants sniff.
*
* @package WPCS\WordPressCodingStandards
* @since 0.14.0
*/
class DiscouragedConstantsUnitTest extends AbstractSniffUnitTest {

/**
* Returns the lines where errors should occur.
*
* @return array <int line number> => <int number of errors>
*/
public function getErrorList() {
return array(
52 => 1,
53 => 1,
54 => 1,
55 => 1,
56 => 1,
57 => 1,
58 => 1,
59 => 1,
60 => 1,
61 => 1,
62 => 1,
63 => 1,
65 => 1,
66 => 1,
68 => 1,
69 => 1,
73 => 1,
);
}

/**
* Returns the lines where warnings should occur.
*
* @return array <int line number> => <int number of warnings>
*/
public function getWarningList() {
return array();
}

} // End class.

0 comments on commit 3c7d0c4

Please sign in to comment.