Constants/ConstantString: prevent more false positives

The `define()` and `defined()` functions expect the name of a constant as a text string to be passed as the first parameter.
This sniff is intended to guard against a coding mistake where the name of the constant would be passed as a_constant_.

While passing the name of the constant as a constant _could_ be perfectly valid when that constant contains the name of another constant as a text string, more often than not, it will be a coding error.

As things were, the sniff did not handle parameters build up using more than one token into account. It also did not handle valid situations, like passing the constant name as a text string via a variable to the functions, correctly.

As outlined in #422 (comment) (Proposal 1), this limits the sniff to _only_ trigger an error when a plain constant has been passed as the "constant name" parameter and ignore all other cases.

Note: The current implementation of this fix, uses the WPCS `get_function_call_parameter()` method to retrieve the stack pointer to the start and end of the parameter. Once PHPCSUtils is in place, this function call should be replaced by using the PHPCSUtils version of that method.

Fixes 422
Fixes 480

Author: jrfnl

WordPressVIPMinimum/Sniffs/Constants/ConstantStringSniff.php

@@ -55,20 +55,26 @@ public function process_token( $stackPtr ) {
 			return;
 		}
 
-		$nextToken     = $this->phpcsFile->findNext( Tokens::$emptyTokens, $nextToken + 1, null, true, null, true );
-		$nextNextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $nextToken + 1, null, true, null, true );
-
-		if ( T_NS_C === $this->tokens[ $nextToken ]['code'] && T_STRING_CONCAT === $this->tokens[ $nextNextToken ]['code'] ) {
-			// Namespacing being used, skip to next.
-			$nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $nextNextToken + 1, null, true, null, true );
+		$param = $this->get_function_call_parameter( $stackPtr, 1 );
+		if ( $param === false ) {
+			// Target parameter not found.
+			return;
 		}
 
-		if ( $this->tokens[ $nextToken ]['code'] !== T_CONSTANT_ENCAPSED_STRING ) {
-			$message = 'Constant name, as a string, should be used along with `%s()`.';
-			$data    = [ $this->tokens[ $stackPtr ]['content'] ];
-			$this->phpcsFile->addError( $message, $nextToken, 'NotCheckingConstantName', $data );
+		$search             = Tokens::$emptyTokens;
+		$search[ T_STRING ] = T_STRING;
+
+		$has_only_tstring = $this->phpcsFile->findNext( $search, $param['start'], $param['end'] + 1, true );
+		if ( $has_only_tstring !== false ) {
+			// Came across something other than a T_STRING token. Ignore.
 			return;
 		}
+
+		$tstring_token = $this->phpcsFile->findNext( T_STRING, $param['start'], $param['end'] + 1 );
+
+		$message = 'Constant name, as a string, should be used along with `%s()`.';
+		$data    = [ $this->tokens[ $stackPtr ]['content'] ];
+		$this->phpcsFile->addError( $message, $tstring_token, 'NotCheckingConstantName', $data );
 	}
 
 }

WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.inc

@@ -5,11 +5,29 @@ if ( ! defined( 'WPCOM_VIP' ) ) { // Okay.
 }
 
 if ( ! defined( WPCOM_VIP ) ) { // Error.
-	define( WPCOM_VIP ); // Error.
+	define( WPCOM_VIP, true ); // Error.
 }
 
 namespace Foo\Bar;
 const REST_ALLOWED_META_PREFIXES = [ 'foo-', 'bar-', 'baz-' ];
 if ( defined( __NAMESPACE__ . '\REST_ALLOWED_META_PREFIXES' ) && in_array( 'foo-', REST_ALLOWED_META_PREFIXES, true ) ) { // Ok.
-	define( __NAMESPACE__ . REST_ALLOWED_META_PREFIXES ); // Error.
+	define( __NAMESPACE__ . '\\' . REST_ALLOWED_META_PREFIXES[1], $value ); // OK.
 }
+
+define( __NAMESPACE__ . '\PLUGIN_URL', \plugins_url( '/', __FILE__ ) ); // OK.
+if ( defined( __NAMESPACE__ . '\\LOADED' ) ) {} // OK.
+
+if ( defined( $obj->constant_name_property ) === false ) { // OK.
+   define( $variable_containing_constant_name, $constant_value ); // OK.
+}
+
+if ( defined( MY_PREFIX . '_CONSTANT_NAME' ) === false ) { // OK.
+   define( 'PREFIX_' . $variable_part, $constant_value ); // OK.
+}
+
+if ( ! defined($generator->get()) { // OK.
+	define( $generator->getLast(), 'value'); // OK.
+}
+
+$defined = defined(); // OK, ignore.
+$defined = defined( /*comment*/ ); // OK, ignore.

WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.php

@@ -25,9 +25,8 @@ class ConstantStringUnitTest extends AbstractSniffUnitTest {
 	 */
 	public function getErrorList() {
 		return [
-			7  => 1,
-			8  => 1,
-			14 => 1,
+			7 => 1,
+			8 => 1,
 		];
 	}