Merge pull request #1089 from WordPress-Coding-Standards/feature/issu…

…e-97-new-wp-discouraged-constants-sniff

New `WordPress.WP.DiscouragedConstants` sniff

WordPress-Extra/ruleset.xml

@@ -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"/>

WordPress-VIP/ruleset.xml

@@ -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>

WordPress/Sniff.php

@@ -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'] ] );
+	}
+
 }

WordPress/Sniffs/WP/DiscouragedConstantsSniff.php

@@ -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.

WordPress/Tests/WP/DiscouragedConstantsUnitTest.inc

@@ -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';