Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New WordPress.WP.DiscouragedConstants sniff #1089

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions WordPress-Extra/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
<rule ref="WordPress.WP.DeprecatedClasses"/>
<rule ref="WordPress.WP.DeprecatedParameters"/>
<rule ref="WordPress.WP.AlternativeFunctions"/>
<rule ref="WordPress.WP.DiscouragedConstants"/>
<rule ref="WordPress.WP.DiscouragedFunctions"/>

<rule ref="Squiz.PHP.Eval"/>
Expand Down
4 changes: 4 additions & 0 deletions WordPress-VIP/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,8 @@
<rule ref="WordPress.WP.AlternativeFunctions.file_get_contents">
<message>%s() is highly discouraged, please use vip_safe_wp_remote_get() instead.</message>
</rule>

<!-- https://vip.wordpress.com/documentation/code-review-what-we-look-for/#using-theme-constants -->
<rule ref="WordPress.WP.DiscouragedConstants"/>

</ruleset>
58 changes: 58 additions & 0 deletions WordPress/Sniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -2220,4 +2220,62 @@ public function has_html_open_tag( $tag_name, $stackPtr = null, $content = false
return false;
}

/**
* Check whether a T_CONST token is a class constant declaration.
*
* @since 0.14.0
*
* @param int $stackPtr The position in the stack of the T_CONST token to verify.
*
* @return bool
*/
public function is_class_constant( $stackPtr ) {
if ( ! isset( $this->tokens[ $stackPtr ] ) || T_CONST !== $this->tokens[ $stackPtr ]['code'] ) {
return false;
}

// Note: traits can not declare constants.
$valid_scopes = array(
'T_CLASS' => true,
'T_ANON_CLASS' => true,
'T_INTERFACE' => true,
);

return $this->valid_direct_scope( $stackPtr, $valid_scopes );
}

/**
* Check whether the direct wrapping scope of a token is within a limited set of
* acceptable tokens.
*
* Used to check, for instance, if a T_CONST is a class constant.
*
* @since 0.14.0
*
* @param int $stackPtr The position in the stack of the token to verify.
* @param array $valid_scopes Array of token types.
* Keys should be the token types in string format
* to allow for newer token types.
* Value is irrelevant.
*
* @return bool
*/
protected function valid_direct_scope( $stackPtr, array $valid_scopes ) {
if ( empty( $this->tokens[ $stackPtr ]['conditions'] ) ) {
return false;
}

/*
* Check only the direct wrapping scope of the token.
*/
$conditions = array_keys( $this->tokens[ $stackPtr ]['conditions'] );
$ptr = array_pop( $conditions );

if ( ! isset( $this->tokens[ $ptr ] ) ) {
return false;
}

return isset( $valid_scopes[ $this->tokens[ $ptr ]['type'] ] );
}

}
221 changes: 221 additions & 0 deletions WordPress/Sniffs/WP/DiscouragedConstantsSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
<?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_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;
}

if ( false !== $prev
&& T_CONST === $this->tokens[ $prev ]['code']
&& true === $this->is_class_constant( $prev )
) {
// Class 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.
72 changes: 72 additions & 0 deletions WordPress/Tests/WP/DiscouragedConstantsUnitTest.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<?php

/*
* Make sure that global constants are correctly identified.
*
* The below are all ok.
*/
// Ok, not a (global) 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 {}
$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.

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' );
const STYLESHEETPATH = 'something';
Loading