Merge pull request #863 from Automattic/feature/functions-restrictedfunctions-sniff-review

Author: GaryJones

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,
 		];
 	}