From 02672aff73be2c654d78ecb076e6c441ed473b32 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 16 Apr 2023 13:30:03 +0200 Subject: [PATCH 1/2] Move `is_token_namespaced()` utility method to dedicated `ContextHelper` The `is_token_namespaced()` utility method is only used by a small set of sniffs, so is better placed in a dedicated class. This commit moves the `is_token_namespaced()` method to the new `WordPressCS\WordPress\Helpers\ContextHelper` class and starts using that class in the relevant sniffs. Related to 1465 This method is tested via the `WordPress.WP.DiscouragedFunctions` sniff. --- .../AbstractFunctionRestrictionsSniff.php | 2 +- WordPress/Helpers/ContextHelper.php | 40 +++++++++++++++++ WordPress/Sniff.php | 43 ++----------------- .../Security/NonceVerificationSniff.php | 2 +- .../Sniffs/WP/DiscouragedConstantsSniff.php | 5 ++- .../Tests/WP/DiscouragedFunctionsUnitTest.inc | 7 +++ .../Tests/WP/DiscouragedFunctionsUnitTest.php | 6 ++- 7 files changed, 59 insertions(+), 46 deletions(-) diff --git a/WordPress/AbstractFunctionRestrictionsSniff.php b/WordPress/AbstractFunctionRestrictionsSniff.php index 9c42343292..e3ac2b1185 100644 --- a/WordPress/AbstractFunctionRestrictionsSniff.php +++ b/WordPress/AbstractFunctionRestrictionsSniff.php @@ -224,7 +224,7 @@ public function is_targetted_token( $stackPtr ) { return false; } - if ( $this->is_token_namespaced( $stackPtr ) === true ) { + if ( ContextHelper::is_token_namespaced( $this->phpcsFile, $stackPtr ) === true ) { return false; } diff --git a/WordPress/Helpers/ContextHelper.php b/WordPress/Helpers/ContextHelper.php index dae6af6755..2fdc468012 100644 --- a/WordPress/Helpers/ContextHelper.php +++ b/WordPress/Helpers/ContextHelper.php @@ -48,4 +48,44 @@ public static function has_object_operator_before( File $phpcsFile, $stackPtr ) return isset( Collections::objectOperators()[ $tokens[ $before ]['code'] ] ); } + + /** + * Check if a particular token is prefixed with a namespace. + * + * @internal This will give a false positive if the file is not namespaced and the token is prefixed + * with `namespace\`. + * + * @since 2.1.0 + * @since 3.0.0 - Moved from the Sniff class to this class. + * - The method visibility was changed from `protected` to `public static`. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. + * @param int $stackPtr The index of the token in the stack. + * + * @return bool + */ + public static function is_token_namespaced( File $phpcsFile, $stackPtr ) { + $prev = $phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true, null, true ); + if ( false === $prev ) { + return false; + } + + $tokens = $phpcsFile->getTokens(); + if ( \T_NS_SEPARATOR !== $tokens[ $prev ]['code'] ) { + return false; + } + + $before_prev = $phpcsFile->findPrevious( Tokens::$emptyTokens, ( $prev - 1 ), null, true, null, true ); + if ( false === $before_prev ) { + return false; + } + + if ( \T_STRING !== $tokens[ $before_prev ]['code'] + && \T_NAMESPACE !== $tokens[ $before_prev ]['code'] + ) { + return false; + } + + return true; + } } diff --git a/WordPress/Sniff.php b/WordPress/Sniff.php index 90366447b8..fbd654b9c2 100644 --- a/WordPress/Sniff.php +++ b/WordPress/Sniff.php @@ -508,43 +508,6 @@ protected function is_in_isset_or_empty( $stackPtr ) { return false; } - /** - * Check if a particular token is prefixed with a namespace. - * - * @internal This will give a false positive if the file is not namespaced and the token is prefixed - * with `namespace\`. - * - * @since 2.1.0 - * - * @param int $stackPtr The index of the token in the stack. - * - * @return bool - */ - protected function is_token_namespaced( $stackPtr ) { - $prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true, null, true ); - - if ( false === $prev ) { - return false; - } - - if ( \T_NS_SEPARATOR !== $this->tokens[ $prev ]['code'] ) { - return false; - } - - $before_prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $prev - 1 ), null, true, null, true ); - if ( false === $before_prev ) { - return false; - } - - if ( \T_STRING !== $this->tokens[ $before_prev ]['code'] - && \T_NAMESPACE !== $this->tokens[ $before_prev ]['code'] - ) { - return false; - } - - return true; - } - /** * Check if a token is (part of) a parameter for a function call to a select list of functions. * @@ -614,7 +577,7 @@ protected function is_in_function_call( $stackPtr, $valid_functions, $global_fun continue; } - if ( $this->is_token_namespaced( $prev_non_empty ) === true ) { + if ( ContextHelper::is_token_namespaced( $this->phpcsFile, $prev_non_empty ) === true ) { continue; } @@ -1004,7 +967,7 @@ protected function is_validated( $stackPtr, $array_keys = array(), $in_condition continue 2; } - if ( $this->is_token_namespaced( $i ) === true ) { + if ( ContextHelper::is_token_namespaced( $this->phpcsFile, $i ) === true ) { // Namespaced function call. continue 2; } @@ -1180,7 +1143,7 @@ public function is_use_of_global_constant( $stackPtr ) { return false; } - if ( $this->is_token_namespaced( $stackPtr ) === true ) { + if ( ContextHelper::is_token_namespaced( $this->phpcsFile, $stackPtr ) === true ) { // Namespaced constant of the same name. return false; } diff --git a/WordPress/Sniffs/Security/NonceVerificationSniff.php b/WordPress/Sniffs/Security/NonceVerificationSniff.php index 7bc5ecdb9d..957e1eebdc 100644 --- a/WordPress/Sniffs/Security/NonceVerificationSniff.php +++ b/WordPress/Sniffs/Security/NonceVerificationSniff.php @@ -274,7 +274,7 @@ private function has_nonce_check( $stackPtr ) { continue; } - if ( $this->is_token_namespaced( $i ) === true ) { + if ( ContextHelper::is_token_namespaced( $this->phpcsFile, $i ) === true ) { continue; } diff --git a/WordPress/Sniffs/WP/DiscouragedConstantsSniff.php b/WordPress/Sniffs/WP/DiscouragedConstantsSniff.php index 729272ece0..4d6bb403a1 100644 --- a/WordPress/Sniffs/WP/DiscouragedConstantsSniff.php +++ b/WordPress/Sniffs/WP/DiscouragedConstantsSniff.php @@ -9,13 +9,14 @@ namespace WordPressCS\WordPress\Sniffs\WP; -use WordPressCS\WordPress\AbstractFunctionParameterSniff; use PHP_CodeSniffer\Util\Tokens; use PHPCSUtils\Tokens\Collections; use PHPCSUtils\Utils\MessageHelper; use PHPCSUtils\Utils\PassedParameters; use PHPCSUtils\Utils\Scopes; use PHPCSUtils\Utils\TextStrings; +use WordPressCS\WordPress\AbstractFunctionParameterSniff; +use WordPressCS\WordPress\Helpers\ContextHelper; /** * Warns against usage of discouraged WP CONSTANTS and recommends alternatives. @@ -140,7 +141,7 @@ public function process_arbitrary_tstring( $stackPtr ) { return; } - if ( $this->is_token_namespaced( $stackPtr ) === true ) { + if ( ContextHelper::is_token_namespaced( $this->phpcsFile, $stackPtr ) === true ) { // Namespaced constant of the same name. return; } diff --git a/WordPress/Tests/WP/DiscouragedFunctionsUnitTest.inc b/WordPress/Tests/WP/DiscouragedFunctionsUnitTest.inc index 2693377ea4..62b8a3678c 100644 --- a/WordPress/Tests/WP/DiscouragedFunctionsUnitTest.inc +++ b/WordPress/Tests/WP/DiscouragedFunctionsUnitTest.inc @@ -11,3 +11,10 @@ wp_reset_query(); // Warning, use wp_reset_postdata instead. $obj->query_posts(); // OK, not the global function. MyClass::wp_reset_query(); // OK, not the global function. $obj?->query_posts(); // OK, not the global function. + +// Ensure the sniff doesn't act on namespaced calls. +MyNamespace\query_posts(); // OK, not the global function. +namespace\query_posts(); // OK, not the global function. + +// ... but does act on fully qualified function calls. +\query_posts(); // Warning. diff --git a/WordPress/Tests/WP/DiscouragedFunctionsUnitTest.php b/WordPress/Tests/WP/DiscouragedFunctionsUnitTest.php index d30f11cc10..4b46ca0933 100644 --- a/WordPress/Tests/WP/DiscouragedFunctionsUnitTest.php +++ b/WordPress/Tests/WP/DiscouragedFunctionsUnitTest.php @@ -21,6 +21,7 @@ * * @covers \WordPressCS\WordPress\AbstractFunctionRestrictionsSniff * @covers \WordPressCS\WordPress\Helpers\ContextHelper::has_object_operator_before + * @covers \WordPressCS\WordPress\Helpers\ContextHelper::is_token_namespaced * @covers \WordPressCS\WordPress\Sniffs\WP\DiscouragedFunctionsSniff */ final class DiscouragedFunctionsUnitTest extends AbstractSniffUnitTest { @@ -41,8 +42,9 @@ public function getErrorList() { */ public function getWarningList() { return array( - 3 => 1, - 4 => 1, + 3 => 1, + 4 => 1, + 20 => 1, ); } } From 16da3dbe8065723b1db2cb7d4b5e238e75eec656 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 16 Apr 2023 13:32:40 +0200 Subject: [PATCH 2/2] ContextHelper::is_token_namespaced(): minor simplifications * A non-inline HTML token will always have another non-empty token before it, if nothing else, the PHP open tag, so checking if `$prev` is `false` is redundant. * Same for the second time this is done, as the same applies for the `T_NS_SEPARATOR` token. --- WordPress/Helpers/ContextHelper.php | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/WordPress/Helpers/ContextHelper.php b/WordPress/Helpers/ContextHelper.php index 2fdc468012..19994c422c 100644 --- a/WordPress/Helpers/ContextHelper.php +++ b/WordPress/Helpers/ContextHelper.php @@ -65,21 +65,14 @@ public static function has_object_operator_before( File $phpcsFile, $stackPtr ) * @return bool */ public static function is_token_namespaced( File $phpcsFile, $stackPtr ) { - $prev = $phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true, null, true ); - if ( false === $prev ) { - return false; - } - $tokens = $phpcsFile->getTokens(); - if ( \T_NS_SEPARATOR !== $tokens[ $prev ]['code'] ) { - return false; - } + $prev = $phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true ); - $before_prev = $phpcsFile->findPrevious( Tokens::$emptyTokens, ( $prev - 1 ), null, true, null, true ); - if ( false === $before_prev ) { + if ( \T_NS_SEPARATOR !== $tokens[ $prev ]['code'] ) { return false; } + $before_prev = $phpcsFile->findPrevious( Tokens::$emptyTokens, ( $prev - 1 ), null, true ); if ( \T_STRING !== $tokens[ $before_prev ]['code'] && \T_NAMESPACE !== $tokens[ $before_prev ]['code'] ) {