From a59b36da554ce73e3db93019d035fb7d8866cf62 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Sat, 5 Aug 2017 03:48:06 +0200
Subject: [PATCH] :sparkles: New `WordPress.WP.DiscouragedConstants` sniff

This sniff is based on a sniff previously pulled to the TRT fork of WPCS as `Theme.DeprecatedConstants`.

Related issue: WPTRT/WordPress-Coding-Standards/issues/23
Original PR: WPTRT/WordPress-Coding-Standards/pull/30
Covers the list of constants found in: Otto42/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 WPTRT/WordPress-Coding-Standards/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 WPTRT/WordPress-Coding-Standards/issues/110 once the original sniff gets removed from the TRT fork and this sniff is added to the TRT ruleset instead.
---
 WordPress-Extra/ruleset.xml                   |   1 +
 WordPress-VIP/ruleset.xml                     |   4 +
 .../Sniffs/WP/DiscouragedConstantsSniff.php   | 214 ++++++++++++++++++
 .../Tests/WP/DiscouragedConstantsUnitTest.inc |  73 ++++++
 .../Tests/WP/DiscouragedConstantsUnitTest.php |  58 +++++
 5 files changed, 350 insertions(+)
 create mode 100644 WordPress/Sniffs/WP/DiscouragedConstantsSniff.php
 create mode 100644 WordPress/Tests/WP/DiscouragedConstantsUnitTest.inc
 create mode 100644 WordPress/Tests/WP/DiscouragedConstantsUnitTest.php

diff --git a/WordPress-Extra/ruleset.xml b/WordPress-Extra/ruleset.xml
index 370dc23a5f..6260d4fabb 100644
--- a/WordPress-Extra/ruleset.xml
+++ b/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"/>
diff --git a/WordPress-VIP/ruleset.xml b/WordPress-VIP/ruleset.xml
index 23c386b6a6..33b75e585b 100644
--- a/WordPress-VIP/ruleset.xml
+++ b/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>
diff --git a/WordPress/Sniffs/WP/DiscouragedConstantsSniff.php b/WordPress/Sniffs/WP/DiscouragedConstantsSniff.php
new file mode 100644
index 0000000000..c56aed06c6
--- /dev/null
+++ b/WordPress/Sniffs/WP/DiscouragedConstantsSniff.php
@@ -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.
diff --git a/WordPress/Tests/WP/DiscouragedConstantsUnitTest.inc b/WordPress/Tests/WP/DiscouragedConstantsUnitTest.inc
new file mode 100644
index 0000000000..0f6910635f
--- /dev/null
+++ b/WordPress/Tests/WP/DiscouragedConstantsUnitTest.inc
@@ -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' );
diff --git a/WordPress/Tests/WP/DiscouragedConstantsUnitTest.php b/WordPress/Tests/WP/DiscouragedConstantsUnitTest.php
new file mode 100644
index 0000000000..876cd0d132
--- /dev/null
+++ b/WordPress/Tests/WP/DiscouragedConstantsUnitTest.php
@@ -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.