Repository: WordPress/WordPress-Coding-Standards (php)

Issue:
DirectDatabaseQuery: don't demand caching or forbid direct DB calls for `TRUNCATE`
## Bug Description
Given the following code snippet:
```php
$wpdb->query(
	$wpdb->prepare(
		'TRUNCATE TABLE `%1$s`',
		plugin_get_table_name( 'Name' )
	)
);
```

WPCS will currently throw the following warnings:
```
 # | WARNING | Usage of a direct database call is discouraged. (WordPress.DB.DirectDatabaseQuery.DirectQuery)
 # | WARNING | Direct database call without caching detected. Consider using wp_cache_get() / wp_cache_set() or wp_cache_delete().
     |         | (WordPress.DB.DirectDatabaseQuery.NoCaching)
```

IMO a query to truncate a table may be dangerous and could warrant a warning be thrown for that reason, however, it *is* a valid query to use a direct database call for and it is also a query for which caching of the query is irrelevant.

The "cache" related issue here is that any existing "old" cache should probably be thrown away, but that can not be done based on the `truncate` query.

Possibly a recommendation to query the truncated table afterwards with caching to clear any caches could be given, but that still wouldn't catch all potentially set caches.

## Environment

| Question               | Answer
| ------------------------| -------
| PHP version             | 7.4.8
| PHP_CodeSniffer version | `master`
| WPCS version            | `develop`
| WPCS install type       | git clones
| IDE (if relevant)       | N/A


## Additional Context (optional)
Loosely related to #1903

## Tested Against `develop` branch?
- [x] I have verified the issue still exists in the `develop` branch of WPCS.


Pull request:
DB/DirectDatabaseQuery: implement PHPCSUtils and support modern PHP 
### DB/DirectDatabaseQuery: remove redundant code

This code is a remnant of the old-style custom ignore comment mechanism which was removed in #2063.

### DB/DirectDatabaseQuery: add missing test case

... to cover the one uncovered line.

### DB/DirectDatabaseQuery: implement PHPCSUtils

👉 This change will be easier to review while ignoring whitespace changes.

### DB/DirectDatabaseQuery: add support for PHP 8.0+ nullsafe object operator

Prevent false negatives in case someone would use the PHP 8.0 nullsafe object operator on the `$wpdb` variable.

Also makes the token walking more stable by preventing an unrelated object operator from being recognized as if it were an operator operating on the `$wpdb` object (false positive).

Includes tests.

### DB/DirectDatabaseQuery: bug fix - code style independent token walking

Don't allow the sniff to get confused by comments.

Related to #763

Includes tests.

### DB/DirectDatabaseQuery: bug fix - finding end of statement

Prevent false positives from walking too far when a database query statement is ended by a PHP close tag.

Includes unit tests.

### DB/DirectDatabaseQuery: bug fix - ignore TRUNCATE queries

Prevent unsolvable false positives for `TRUNCATE` queries. Those cannot be cached and need a direct DB query. **(second opinion appreciated!)**

Includes unit tests.

Fixes #1947

Patch:
diff --git a/WordPress/Sniffs/DB/DirectDatabaseQuerySniff.php b/WordPress/Sniffs/DB/DirectDatabaseQuerySniff.php
index 56ca4ef6..2f4d8637 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 @@ class DirectDatabaseQuerySniff extends Sniff {
 			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 @@ class DirectDatabaseQuerySniff extends Sniff {
 			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 @@ class DirectDatabaseQuerySniff extends Sniff {
 
 		$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;
 						}
 					}
 				}


Test patch:
diff --git a/WordPress/Tests/DB/DirectDatabaseQueryUnitTest.inc b/WordPress/Tests/DB/DirectDatabaseQueryUnitTest.inc
index 88f1fba1..2e75d64e 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 -->
+	<?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' )
+		)
+	);
+}
diff --git a/WordPress/Tests/DB/DirectDatabaseQueryUnitTest.php b/WordPress/Tests/DB/DirectDatabaseQueryUnitTest.php
index 02117429..8f1d9015 100644
--- a/WordPress/Tests/DB/DirectDatabaseQueryUnitTest.php
+++ b/WordPress/Tests/DB/DirectDatabaseQueryUnitTest.php
@@ -84,7 +84,11 @@ class DirectDatabaseQueryUnitTest extends AbstractSniffUnitTest {
 			252 => 1,
 			265 => 1,
 			269 => 1,
+			281 => 1,
+			287 => 2,
+			288 => 1,
+			300 => 1,
+			306 => 2,
 		);
 	}
-
 }