Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DB/DirectDatabaseQuery: implement PHPCSUtils and support modern PHP #2186

Merged
merged 7 commits into from
Jan 8, 2023
Merged
77 changes: 36 additions & 41 deletions WordPress/Sniffs/DB/DirectDatabaseQuerySniff.php
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
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
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,
);
}

}