Functions/RestrictedFunctions::is_targetted_token(): improve the method

The `RestrictedFunctions::is_targetted_token()` method was basically a duplicate of the upstream/parent method with some additional code to handle one specific situation (method calls on a specific variable).

However, the parent method has been updated significantly since the code was copied and this sniff wasn't benefitting from that.

This commit rewrites the `RestrictedFunctions::is_targetted_token()` method to defer to the parent method in all cases, except for the one specific situation we want to account for.

Additionally, it stabilizes and improves the token walking done for that specific situation, as well as takes the PHP 8.0+ nullsafe object operator into account.

Includes a number of tests, some to cover the improvements inherited from the parent method, some to cover the improved code in the overloaded method.

Includes documenting how PHP 8.1+ first class callables are handled by the sniff.

Author: jrfnl

WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php

@@ -303,47 +303,27 @@ public function getGroups() {
 	 * @return bool
 	 */
 	public function is_targetted_token( $stackPtr ) {
-		// Exclude function definitions, class methods, and namespaced calls.
-		if ( $this->tokens[ $stackPtr ]['code'] === \T_STRING && isset( $this->tokens[ $stackPtr - 1 ] ) ) {
-			// Check if this is really a function.
-			$next = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true );
-			if ( $next !== false && $this->tokens[ $next ]['code'] !== T_OPEN_PARENTHESIS ) {
-				return false;
-			}
+		if ( empty( $this->groups[ $this->tokens[ $stackPtr ]['content'] ]['object_var'] ) ) {
+			return parent::is_targetted_token( $stackPtr );
+		}
 
-			$prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $stackPtr - 1, null, true );
-			if ( $prev !== false ) {
+		// Start difference to parent class method.
+		// Check to see if the token is a method call on a specific object variable.
+		$next = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true );
+		if ( $next === false || $this->tokens[ $next ]['code'] !== T_OPEN_PARENTHESIS ) {
+			return false;
+		}
 
-				// Start difference to parent class method.
-				// Check to see if function is a method on a specific object variable.
-				if ( ! empty( $this->groups[ $this->tokens[ $stackPtr ]['content'] ]['object_var'] ) ) {
-					$prevPrev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $stackPtr - 2, null, true );
+		$prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $stackPtr - 1, null, true );
+		if ( $this->tokens[ $prev ]['code'] !== T_OBJECT_OPERATOR
+			&& $this->tokens[ $prev ]['code'] !== T_NULLSAFE_OBJECT_OPERATOR
+		) {
+			return false;
+		}
 
-					return $this->tokens[ $prev ]['code'] === \T_OBJECT_OPERATOR && isset( $this->groups[ $this->tokens[ $stackPtr ]['content'] ]['object_var'][ $this->tokens[ $prevPrev ]['content'] ] );
-				} // End difference to parent class method.
+		$prevPrev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $prev - 1, null, true );
 
-				// Skip sniffing if calling a same-named method, or on function definitions.
-				$skipped = [
-					\T_FUNCTION        => \T_FUNCTION,
-					\T_CLASS           => \T_CLASS,
-					\T_AS              => \T_AS, // Use declaration alias.
-					\T_DOUBLE_COLON    => \T_DOUBLE_COLON,
-					\T_OBJECT_OPERATOR => \T_OBJECT_OPERATOR,
-					\T_NEW             => \T_NEW,
-				];
-				if ( isset( $skipped[ $this->tokens[ $prev ]['code'] ] ) ) {
-					return false;
-				}
-				// Skip namespaced functions, ie: `\foo\bar()` not `\bar()`.
-				if ( $this->tokens[ $prev ]['code'] === \T_NS_SEPARATOR ) {
-					$pprev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $prev - 1, null, true );
-					if ( $pprev !== false && $this->tokens[ $pprev ]['code'] === \T_STRING ) {
-						return false;
-					}
-				}
-			}
-			return true;
-		}
-		return false;
+		return $this->tokens[ $prevPrev ]['code'] === T_VARIABLE
+			&& isset( $this->groups[ $this->tokens[ $stackPtr ]['content'] ]['object_var'][ $this->tokens[ $prevPrev ]['content'] ] );
 	}
 }

WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc

@@ -230,3 +230,19 @@ $popular = custom_stats_get_csv( 'postviews', [ 'days'  => 2,   'limit' => 20 ]
 
 $foo = new Link; // OK, class, not function.
 $foo = new Mail(); // OK, class, not function.
+
+class ReturnByRef {
+	function &chmod($a) {} // OK, method declaration, not function call.
+}
+
+// Class instantiations in PHP 8.0+ attributes are not function calls.
+class AttributesShouldBeIgnored {
+	#[WP_Is_Mobile()] // OK.
+	public function foo() {}
+}
+
+$wp_rewrite->flush_rules; // OK, property access, not function call.
+$wp_rewrite /*comment*/ -> /*comment*/ flush_rules(); // Error.
+$wp_rewrite?->flush_rules(); // Error.
+
+array_walk($roles, add_role(...)); // Error. PHP 8.1 first class callable.

WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php

@@ -97,6 +97,9 @@ public function getErrorList() {
 			199 => 1,
 			200 => 1,
 			228 => 1,
+			245 => 1,
+			246 => 1,
+			248 => 1,
 		];
 	}