From ce822662ee3ca7d28b451174cee720a00215d1fc Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 27 Jul 2025 07:42:17 +0200 Subject: [PATCH] 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. --- .../Functions/RestrictedFunctionsSniff.php | 56 ++++++------------- .../Functions/RestrictedFunctionsUnitTest.inc | 16 ++++++ .../Functions/RestrictedFunctionsUnitTest.php | 3 + 3 files changed, 37 insertions(+), 38 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php index a5a4ecec..bff5ce8d 100644 --- a/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php +++ b/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'] ] ); } } diff --git a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc index 19cba6b3..4a3f57b4 100644 --- a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc +++ b/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. diff --git a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php index 0b4fcc5c..1c727fac 100644 --- a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php @@ -97,6 +97,9 @@ public function getErrorList() { 199 => 1, 200 => 1, 228 => 1, + 245 => 1, + 246 => 1, + 248 => 1, ]; }