Skip to content

Commit

Permalink
Merge pull request #2186 from WordPress/feature/directdbquery-phpcsut…
Browse files Browse the repository at this point in the history 
…ils-and-modern-php
  • Loading branch information
@GaryJones
GaryJones authored Jan 8, 2023
2 parents dcbb3ec + 0913f35 commit c8a7ac1
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 42 deletions.
77 changes: 36 additions & 41 deletions WordPress/Sniffs/DB/DirectDatabaseQuerySniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand All @@ -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' );
}
Expand All @@ -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;
}
}
}
Expand Down
43 changes: 43 additions & 0 deletions WordPress/Tests/DB/DirectDatabaseQueryUnitTest.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 -->
<?php
$next_query = 'ALTER TABLE TO ADD SOME FIELDS' ); // Should not be flagged as not in a call to $wpdb.
}

function stay_silent_for_truncate_query() {
global $wpdb;
$wpdb->query(
$wpdb->prepare(
'TRUNCATE TABLE `%1$s`',
plugin_get_table_name( 'Name' )
)
);
}
6 changes: 5 additions & 1 deletion WordPress/Tests/DB/DirectDatabaseQueryUnitTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,11 @@ public function getWarningList() {
252 => 1,
265 => 1,
269 => 1,
281 => 1,
287 => 2,
288 => 1,
300 => 1,
306 => 2,
);
}

}

0 comments on commit c8a7ac1

Please sign in to comment.