Merge pull request #851 from Automattic/feature/constants-constantstring-sniff-review

Author: GaryJones

WordPressVIPMinimum/Sniffs/Constants/ConstantStringSniff.php

@@ -11,51 +11,43 @@
 
 use PHP_CodeSniffer\Util\Tokens;
 use PHPCSUtils\Utils\PassedParameters;
-use WordPressVIPMinimum\Sniffs\Sniff;
+use WordPressCS\WordPress\AbstractFunctionParameterSniff;
 
 /**
  * Sniff for properly using constant name when checking whether a constant is defined.
  */
-class ConstantStringSniff extends Sniff {
+class ConstantStringSniff extends AbstractFunctionParameterSniff {
 
 	/**
-	 * Returns an array of tokens this test wants to listen for.
+	 * The group name for this group of functions.
 	 *
-	 * @return array<int|string>
+	 * @var string
 	 */
-	public function register() {
-		return [
-			T_STRING,
-		];
-	}
+	protected $group_name = 'constant_functions';
+
+	/**
+	 * Functions this sniff is looking for.
+	 *
+	 * @var array<string, bool> Key is the function name, value irrelevant.
+	 */
+	protected $target_functions = [
+		'define'  => true,
+		'defined' => true,
+	];
 
 	/**
-	 * Process this test when one of its tokens is encountered.
+	 * Process the parameters of a matched function.
 	 *
-	 * @param int $stackPtr The position of the current token in the stack passed in $tokens.
+	 * @param int    $stackPtr        The position of the current token in the stack.
+	 * @param string $group_name      The name of the group which was matched.
+	 * @param string $matched_content The token content (function name) which was matched
+	 *                                in lowercase.
+	 * @param array  $parameters      Array with information about the parameters.
 	 *
 	 * @return void
 	 */
-	public function process_token( $stackPtr ) {
-
-		if ( in_array( $this->tokens[ $stackPtr ]['content'], [ 'define', 'defined' ], true ) === false ) {
-			return;
-		}
-
-		// Find the next non-empty token.
-		$nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true, null, true );
-
-		if ( $this->tokens[ $nextToken ]['code'] !== T_OPEN_PARENTHESIS ) {
-			// Not a function call.
-			return;
-		}
-
-		if ( isset( $this->tokens[ $nextToken ]['parenthesis_closer'] ) === false ) {
-			// Not a function call.
-			return;
-		}
-
-		$param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1, 'constant_name' );
+	public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) {
+		$param = PassedParameters::getParameterFromStack( $parameters, 1, 'constant_name' );
 		if ( $param === false ) {
 			// Target parameter not found.
 			return;
@@ -72,7 +64,7 @@ public function process_token( $stackPtr ) {
 
 		$tstring_token = $this->phpcsFile->findNext( T_STRING, $param['start'], $param['end'] + 1 );
 
-		$message = 'Constant name, as a string, should be used along with `%s()`.';
+		$message = 'The `%s()` function expects to be passed the constant name as a text string.';
 		$data    = [ $this->tokens[ $stackPtr ]['content'] ];
 		$this->phpcsFile->addError( $message, $tstring_token, 'NotCheckingConstantName', $data );
 	}

WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.inc

@@ -31,3 +31,38 @@ if ( ! defined($generator->get()) { // OK.
 
 $defined = defined(); // OK, ignore.
 $defined = defined( /*comment*/ ); // OK, ignore.
+
+// Safeguard that calls to namespaced functions or methods are not confused with the global functions.
+if ( ! Not\defined( WPCOM_VIP ) ) { // Okay.
+	$obj->define( WPCOM_VIP, true ); // Okay.
+}
+
+if ( ! \Fully\Qualified\defined( WPCOM_VIP ) ) { // Okay.
+	namespace\define( WPCOM_VIP, true ); // Okay.
+}
+
+// Safeguard that fully qualified function calls are handled correctly.
+if ( ! \defined( 'WPCOM_VIP' ) ) { // Okay.
+	\define( 'WPCOM_VIP', true, ); // Okay.
+}
+
+if ( ! \defined( WPCOM_VIP, ) ) { // Error.
+	\define( WPCOM_VIP, true ); // Error.
+}
+
+// Safeguard correct handling of case-insensitivity of function names.
+if ( ! DEFINED( WPCOM_VIP ) ) { // Error.
+	Define( WPCOM_VIP, true ); // Error.
+}
+
+// Safeguard correct handling of function calls using PHP 8.0+ named parameters.
+\defined( constant_nam: WPCOM_VIP ); // Okay, well not really, typo in param name, but that's not the concern of the sniff.
+define(value: true, constant_name: 'WPCOM_VIP', ); // Okay.
+define(case_insensitive: true, constant_name: WPCOM_VIP ); // Error.
+
+// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute.
+#[Define('name', 'value')]
+function foo() {}
+
+define(...$params); // PHP 5.6 argument unpacking will be ignored.
+add_action('my_action', define(...)); // PHP 8.1 first class callable will be ignored.

WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.php

@@ -23,8 +23,13 @@ class ConstantStringUnitTest extends AbstractSniffUnitTest {
 	 */
 	public function getErrorList() {
 		return [
-			7 => 1,
-			8 => 1,
+			7  => 1,
+			8  => 1,
+			49 => 1,
+			50 => 1,
+			54 => 1,
+			55 => 1,
+			61 => 1,
 		];
 	}