Merge pull request #2158 from WordPress/feature/move-wpdb-method-to-h…

…elper-trait

Move "is WPDB method call" related utilities to dedicated WPDBTrait + improve

WordPress/Helpers/WPDBTrait.php

@@ -0,0 +1,116 @@
+<?php
+/**
+ * WordPress Coding Standard.
+ *
+ * @package WPCS\WordPressCodingStandards
+ * @link    https://github.com/WordPress/WordPress-Coding-Standards
+ * @license https://opensource.org/licenses/MIT MIT
+ */
+
+namespace WordPressCS\WordPress\Helpers;
+
+use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Util\Tokens;
+use PHPCSUtils\BackCompat\BCFile;
+use PHPCSUtils\Tokens\Collections;
+
+/**
+ * Helper utilities for sniffs which examine WPDB method calls.
+ *
+ * {@internal The method in this trait was previously contained in the
+ * `WordPressCS\WordPress\Sniff` class and has been moved here.}
+ *
+ * @package WPCS\WordPressCodingStandards
+ * @since   3.0.0
+ */
+trait WPDBTrait {
+
+	/**
+	 * Checks whether this is a call to a $wpdb method that we want to sniff.
+	 *
+	 * If available in the class using this trait, the $methodPtr, $i and $end properties
+	 * are automatically set to correspond to the start and end of the method call.
+	 * The $i property is also set if this is not a method call but rather the
+	 * use of a $wpdb property.
+	 *
+	 * @since 0.8.0
+	 * @since 0.9.0  The return value is now always boolean. The $end and $i member
+	 *               vars are automatically updated.
+	 * @since 0.14.0 Moved this method from the `PreparedSQL` sniff to the base WP sniff.
+	 * @since 3.0.0  - Moved from the Sniff class to this dedicated Trait.
+	 *               - The $phpcsFile parameter was added.
+	 *
+	 * {@internal This method should be refactored to not exhibit "magic" behaviour
+	 *            for properties in the sniff class(es) using it.}}
+	 *
+	 * @param \PHP_CodeSniffer\Files\File $phpcsFile      The file being scanned.
+	 * @param int                         $stackPtr       The index of the $wpdb variable.
+	 * @param array                       $target_methods Array of methods. Key(s) should be method name
+	 *                                                    in lowercase.
+	 *
+	 * @return bool Whether this is a $wpdb method call.
+	 */
+	protected function is_wpdb_method_call( File $phpcsFile, $stackPtr, $target_methods ) {
+
+		$tokens = $phpcsFile->getTokens();
+
+		// Check for wpdb.
+		if ( ( \T_VARIABLE === $tokens[ $stackPtr ]['code'] && '$wpdb' !== $tokens[ $stackPtr ]['content'] )
+			|| ( \T_STRING === $tokens[ $stackPtr ]['code'] && 'wpdb' !== strtolower( $tokens[ $stackPtr ]['content'] ) )
+		) {
+			return false;
+		}
+
+		// Check that this is a method call.
+		$is_object_call = $phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
+		if ( false === $is_object_call
+			|| isset( Collections::objectOperators()[ $tokens[ $is_object_call ]['code'] ] ) === false
+		) {
+			return false;
+		}
+
+		$methodPtr = $phpcsFile->findNext( Tokens::$emptyTokens, ( $is_object_call + 1 ), null, true, null, true );
+		if ( false === $methodPtr ) {
+			return false;
+		}
+
+		if ( \T_STRING === $tokens[ $methodPtr ]['code'] && property_exists( $this, 'methodPtr' ) ) {
+			$this->methodPtr = $methodPtr;
+		}
+
+		// Find the opening parenthesis.
+		$opening_paren = $phpcsFile->findNext( Tokens::$emptyTokens, ( $methodPtr + 1 ), null, true, null, true );
+
+		if ( false === $opening_paren ) {
+			return false;
+		}
+
+		if ( property_exists( $this, 'i' ) ) {
+			$this->i = $opening_paren;
+		}
+
+		if ( \T_OPEN_PARENTHESIS !== $tokens[ $opening_paren ]['code']
+			|| ! isset( $tokens[ $opening_paren ]['parenthesis_closer'] )
+		) {
+			return false;
+		}
+
+		// Check that this is one of the methods that we are interested in.
+		if ( ! isset( $target_methods[ strtolower( $tokens[ $methodPtr ]['content'] ) ] ) ) {
+			return false;
+		}
+
+		// Find the end of the first parameter.
+		$end = BCFile::findEndOfStatement( $phpcsFile, $opening_paren + 1 );
+
+		if ( \T_COMMA !== $tokens[ $end ]['code'] ) {
+			++$end;
+		}
+
+		if ( property_exists( $this, 'end' ) ) {
+			$this->end = $end;
+		}
+
+		return true;
+	}
+}

WordPress/Sniff.php

@@ -1582,93 +1582,6 @@ protected function is_in_array_comparison( $stackPtr ) {
 		return false;
 	}
 
-	/**
-	 * Checks whether this is a call to a $wpdb method that we want to sniff.
-	 *
-	 * If available in the child class, the $methodPtr, $i and $end properties are
-	 * automatically set to correspond to the start and end of the method call.
-	 * The $i property is also set if this is not a method call but rather the
-	 * use of a $wpdb property.
-	 *
-	 * @since 0.8.0
-	 * @since 0.9.0  The return value is now always boolean. The $end and $i member
-	 *               vars are automatically updated.
-	 * @since 0.14.0 Moved this method from the `PreparedSQL` sniff to the base WP sniff.
-	 *
-	 * {@internal This method should probably be refactored.}}
-	 *
-	 * @param int   $stackPtr       The index of the $wpdb variable.
-	 * @param array $target_methods Array of methods. Key(s) should be method name.
-	 *
-	 * @return bool Whether this is a $wpdb method call.
-	 */
-	protected function is_wpdb_method_call( $stackPtr, $target_methods ) {
-
-		// Check for wpdb.
-		if ( ( \T_VARIABLE === $this->tokens[ $stackPtr ]['code'] && '$wpdb' !== $this->tokens[ $stackPtr ]['content'] )
-			|| ( \T_STRING === $this->tokens[ $stackPtr ]['code'] && 'wpdb' !== $this->tokens[ $stackPtr ]['content'] )
-		) {
-			return false;
-		}
-
-		// Check that this is a method call.
-		$is_object_call = $this->phpcsFile->findNext(
-			array( \T_OBJECT_OPERATOR, \T_DOUBLE_COLON ),
-			( $stackPtr + 1 ),
-			null,
-			false,
-			null,
-			true
-		);
-		if ( false === $is_object_call ) {
-			return false;
-		}
-
-		$methodPtr = $this->phpcsFile->findNext( \T_WHITESPACE, ( $is_object_call + 1 ), null, true, null, true );
-		if ( false === $methodPtr ) {
-			return false;
-		}
-
-		if ( \T_STRING === $this->tokens[ $methodPtr ]['code'] && property_exists( $this, 'methodPtr' ) ) {
-			$this->methodPtr = $methodPtr;
-		}
-
-		// Find the opening parenthesis.
-		$opening_paren = $this->phpcsFile->findNext( \T_WHITESPACE, ( $methodPtr + 1 ), null, true, null, true );
-
-		if ( false === $opening_paren ) {
-			return false;
-		}
-
-		if ( property_exists( $this, 'i' ) ) {
-			$this->i = $opening_paren;
-		}
-
-		if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $opening_paren ]['code']
-			|| ! isset( $this->tokens[ $opening_paren ]['parenthesis_closer'] )
-		) {
-			return false;
-		}
-
-		// Check that this is one of the methods that we are interested in.
-		if ( ! isset( $target_methods[ $this->tokens[ $methodPtr ]['content'] ] ) ) {
-			return false;
-		}
-
-		// Find the end of the first parameter.
-		$end = $this->phpcsFile->findEndOfStatement( $opening_paren + 1 );
-
-		if ( \T_COMMA !== $this->tokens[ $end ]['code'] ) {
-			++$end;
-		}
-
-		if ( property_exists( $this, 'end' ) ) {
-			$this->end = $end;
-		}
-
-		return true;
-	}
-
 	/**
 	 * Determine whether an arbitrary T_STRING token is the use of a global constant.
 	 *

WordPress/Sniffs/DB/PreparedSQLPlaceholdersSniff.php

@@ -12,6 +12,7 @@
 use PHP_CodeSniffer\Util\Tokens;
 use PHPCSUtils\Utils\PassedParameters;
 use PHPCSUtils\Utils\TextStrings;
+use WordPressCS\WordPress\Helpers\WPDBTrait;
 use WordPressCS\WordPress\Sniff;
 
 /**
@@ -43,6 +44,8 @@
  */
 class PreparedSQLPlaceholdersSniff extends Sniff {
 
+	use WPDBTrait;
+
 	/**
 	 * These regexes were originally copied from https://www.php.net/function.sprintf#93552
 	 * and adjusted for limitations in `$wpdb->prepare()`.
@@ -168,7 +171,7 @@ public function register() {
 	 */
 	public function process_token( $stackPtr ) {
 
-		if ( ! $this->is_wpdb_method_call( $stackPtr, $this->target_methods ) ) {
+		if ( ! $this->is_wpdb_method_call( $this->phpcsFile, $stackPtr, $this->target_methods ) ) {
 			return;
 		}
 

WordPress/Sniffs/DB/PreparedSQLSniff.php

@@ -11,6 +11,7 @@
 
 use PHP_CodeSniffer\Util\Tokens;
 use PHPCSUtils\Utils\TextStrings;
+use WordPressCS\WordPress\Helpers\WPDBTrait;
 use WordPressCS\WordPress\Sniff;
 
 /**
@@ -28,6 +29,8 @@
  */
 class PreparedSQLSniff extends Sniff {
 
+	use WPDBTrait;
+
 	/**
 	 * The lists of $wpdb methods.
 	 *
@@ -124,7 +127,7 @@ public function register() {
 	 */
 	public function process_token( $stackPtr ) {
 
-		if ( ! $this->is_wpdb_method_call( $stackPtr, $this->methods ) ) {
+		if ( ! $this->is_wpdb_method_call( $this->phpcsFile, $stackPtr, $this->methods ) ) {
 			return;
 		}
 
@@ -161,7 +164,7 @@ function ( $symbol ) {
 
 			if ( \T_VARIABLE === $this->tokens[ $this->i ]['code'] ) {
 				if ( '$wpdb' === $this->tokens[ $this->i ]['content'] ) {
-					$this->is_wpdb_method_call( $this->i, $this->methods );
+					$this->is_wpdb_method_call( $this->phpcsFile, $this->i, $this->methods );
 					continue;
 				}
 

WordPress/Tests/DB/PreparedSQLUnitTest.inc

@@ -111,5 +111,14 @@ $wpdb->query( "SELECT * FROM ${wpdb->{${'a'}}} WHERE post_title LIKE '${title->{
 // More defensive variable checking
 $wpdb->query( "SELECT * FROM $wpdb" ); // Bad x 1, $wpdb on its own is not valid.
 
+$wpdb
+	-> /*comment*/ query( "SELECT * FROM $wpdb->posts WHERE post_title LIKE '" . $_GET['title'] . "';" ); // Bad.
+
+$wpdb?->query( "SELECT * FROM $wpdb->posts WHERE post_title LIKE '" . (int) $foo . "';" ); // OK.
+$wpdb?->query( "SELECT * FROM $wpdb->posts WHERE post_title LIKE '" . foo() . "';" ); // Bad.
+
+WPDB::prepare( "SELECT * FROM $wpdb->posts WHERE post_title LIKE '" . foo() . "';" ); // Bad.
+$wpdb->Query( "SELECT * FROM $wpdb->posts WHERE post_title LIKE '" . foo() . "';" ); // Bad.
+
 // Don't throw an error during live coding.
 wpdb::prepare( "SELECT * FROM $wpdb->posts

WordPress/Tests/DB/PreparedSQLUnitTest.php

@@ -53,6 +53,10 @@ public function getErrorList() {
 			108 => 1,
 			109 => 1,
 			112 => 1,
+			115 => 1,
+			118 => 1,
+			120 => 1,
+			121 => 1,
 		);
 	}
 
@@ -66,5 +70,4 @@ public function getErrorList() {
 	public function getWarningList() {
 		return array();
 	}
-
 }