diff --git a/WordPress/Sniffs/DB/DirectDatabaseQuerySniff.php b/WordPress/Sniffs/DB/DirectDatabaseQuerySniff.php index 56ca4ef699..2f4d86372b 100644 --- a/WordPress/Sniffs/DB/DirectDatabaseQuerySniff.php +++ b/WordPress/Sniffs/DB/DirectDatabaseQuerySniff.php @@ -10,6 +10,9 @@ namespace WordPressCS\WordPress\Sniffs\DB; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Tokens\Collections; +use PHPCSUtils\Utils\Conditions; +use PHPCSUtils\Utils\TextStrings; use WordPressCS\WordPress\Helpers\RulesetPropertyHelper; use WordPressCS\WordPress\Sniff; @@ -172,12 +175,16 @@ public function process_token( $stackPtr ) { return; } - $is_object_call = $this->phpcsFile->findNext( \T_OBJECT_OPERATOR, ( $stackPtr + 1 ), null, false, null, true ); - if ( false === $is_object_call ) { - return; // This is not a call to the wpdb object. + $is_object_call = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); + if ( false === $is_object_call + || ( \T_OBJECT_OPERATOR !== $this->tokens[ $is_object_call ]['code'] + && \T_NULLSAFE_OBJECT_OPERATOR !== $this->tokens[ $is_object_call ]['code'] ) + ) { + // This is not a call to the wpdb object. + return; } - $methodPtr = $this->phpcsFile->findNext( array( \T_WHITESPACE ), ( $is_object_call + 1 ), null, true, null, true ); + $methodPtr = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $is_object_call + 1 ), null, true ); $method = $this->tokens[ $methodPtr ]['content']; $this->mergeFunctionLists(); @@ -186,26 +193,20 @@ public function process_token( $stackPtr ) { return; } - $endOfStatement = $this->phpcsFile->findNext( \T_SEMICOLON, ( $stackPtr + 1 ), null, false, null, true ); - $endOfLineComment = ''; - for ( $i = ( $endOfStatement + 1 ); $i < $this->phpcsFile->numTokens; $i++ ) { - - if ( $this->tokens[ $i ]['line'] !== $this->tokens[ $endOfStatement ]['line'] ) { - break; - } - - if ( \T_COMMENT === $this->tokens[ $i ]['code'] ) { - $endOfLineComment .= $this->tokens[ $i ]['content']; - } - } + $endOfStatement = $this->phpcsFile->findNext( array( \T_SEMICOLON, \T_CLOSE_TAG ), ( $stackPtr + 1 ) ); - // Check for Database Schema Changes. + // Check for Database Schema Changes/ table truncation. for ( $_pos = ( $stackPtr + 1 ); $_pos < $endOfStatement; $_pos++ ) { - $_pos = $this->phpcsFile->findNext( Tokens::$textStringTokens, $_pos, $endOfStatement, false, null, true ); + $_pos = $this->phpcsFile->findNext( Tokens::$textStringTokens, $_pos, $endOfStatement ); if ( false === $_pos ) { break; } + if ( strpos( TextStrings::stripQuotes( $this->tokens[ $_pos ]['content'] ), 'TRUNCATE ' ) === 0 ) { + // Ignore queries to truncate the database as caching those is irrelevant and they need a direct db query. + return; + } + if ( preg_match( '#\b(?:ALTER|CREATE|DROP)\b#i', $this->tokens[ $_pos ]['content'] ) > 0 ) { $this->phpcsFile->addWarning( 'Attempting a database schema change is discouraged.', $_pos, 'SchemaChange' ); } @@ -219,36 +220,30 @@ public function process_token( $stackPtr ) { $cached = false; $wp_cache_get = false; - if ( ! empty( $this->tokens[ $stackPtr ]['conditions'] ) ) { - $scope_function = $this->phpcsFile->getCondition( $stackPtr, \T_FUNCTION ); - if ( false === $scope_function ) { - $scope_function = $this->phpcsFile->getCondition( $stackPtr, \T_CLOSURE ); - } - - if ( false !== $scope_function ) { - $scopeStart = $this->tokens[ $scope_function ]['scope_opener']; - $scopeEnd = $this->tokens[ $scope_function ]['scope_closer']; + $scope_function = Conditions::getLastCondition( $this->phpcsFile, $stackPtr, Collections::functionDeclarationTokens() ); + if ( false !== $scope_function ) { + $scopeStart = $this->tokens[ $scope_function ]['scope_opener']; + $scopeEnd = $this->tokens[ $scope_function ]['scope_closer']; - for ( $i = ( $scopeStart + 1 ); $i < $scopeEnd; $i++ ) { - if ( \T_STRING === $this->tokens[ $i ]['code'] ) { + for ( $i = ( $scopeStart + 1 ); $i < $scopeEnd; $i++ ) { + if ( \T_STRING === $this->tokens[ $i ]['code'] ) { - if ( isset( $this->cacheDeleteFunctions[ $this->tokens[ $i ]['content'] ] ) ) { + if ( isset( $this->cacheDeleteFunctions[ $this->tokens[ $i ]['content'] ] ) ) { - if ( \in_array( $method, array( 'query', 'update', 'replace', 'delete' ), true ) ) { - $cached = true; - break; - } - } elseif ( isset( $this->cacheGetFunctions[ $this->tokens[ $i ]['content'] ] ) ) { + if ( \in_array( $method, array( 'query', 'update', 'replace', 'delete' ), true ) ) { + $cached = true; + break; + } + } elseif ( isset( $this->cacheGetFunctions[ $this->tokens[ $i ]['content'] ] ) ) { - $wp_cache_get = true; + $wp_cache_get = true; - } elseif ( isset( $this->cacheSetFunctions[ $this->tokens[ $i ]['content'] ] ) ) { + } elseif ( isset( $this->cacheSetFunctions[ $this->tokens[ $i ]['content'] ] ) ) { - if ( $wp_cache_get ) { - $cached = true; - break; - } + if ( $wp_cache_get ) { + $cached = true; + break; } } } diff --git a/WordPress/Tests/DB/DirectDatabaseQueryUnitTest.inc b/WordPress/Tests/DB/DirectDatabaseQueryUnitTest.inc index 88f1fba18d..2e75d64ece 100644 --- a/WordPress/Tests/DB/DirectDatabaseQueryUnitTest.inc +++ b/WordPress/Tests/DB/DirectDatabaseQueryUnitTest.inc @@ -274,3 +274,46 @@ EOD wp_cache_add( 'baz', $baz ); } } + +// Non-cachable call should bow out after `DirectQuery` warning. +function non_cachable() { + global $wpdb; + $wpdb->insert( 'table', array( 'column' => 'foo', 'field' => 'bar' ) ); // Warning direct DB call. +} + +// Safeguard recognition of PHP 8.0+ nullsafe object operator. +function nullsafe_object_operator() { + global $wpdb; + $listofthings = $wpdb?->get_col( 'SELECT something FROM somewhere WHERE someotherthing = 1' ); // Warning x 2. + $last = $wpdb?->insert( $wpdb->users, $data, $where ); // Warning x 1. +} + +function stabilize_token_walking($other_db) { + global $wpdb; + // Overwritting $wpdb is bad, of course, but that's not the concern of this sniff. + // This test is making sure that the `$other_db->query` code is not flagged as if it were a call to a `$wpdb` method. + $wpdb = $other_db->query; +} + +function ignore_comments() { + global $wpdb; + $wpdb-> // Let's pretend this is a method-chain (this is the comment which should not confuse the sniff). + insert( 'table', array( 'column' => 'foo', 'field' => 'bar' ) ); // Warning direct DB call. +} + +function correctly_determine_end_of_statement() { + global $wpdb; + $wpdb->get_col( $query, $col ) ?><-- Warning x 2 --> + query( + $wpdb->prepare( + 'TRUNCATE TABLE `%1$s`', + plugin_get_table_name( 'Name' ) + ) + ); +} diff --git a/WordPress/Tests/DB/DirectDatabaseQueryUnitTest.php b/WordPress/Tests/DB/DirectDatabaseQueryUnitTest.php index 02117429f3..8f1d90150c 100644 --- a/WordPress/Tests/DB/DirectDatabaseQueryUnitTest.php +++ b/WordPress/Tests/DB/DirectDatabaseQueryUnitTest.php @@ -84,7 +84,11 @@ public function getWarningList() { 252 => 1, 265 => 1, 269 => 1, + 281 => 1, + 287 => 2, + 288 => 1, + 300 => 1, + 306 => 2, ); } - }