From 1686286d9d99fa728e543484af3321b66455b1f7 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Tue, 22 Aug 2023 22:36:40 +0200
Subject: [PATCH] Performance/WPQueryParams: defer to the parent sniff

This commit removes the custom token target + custom logic from this sniff in favour of deferring to the logic in the parent sniff - as discussed in 589.

To that end, the keys which were handled in the custom `process_token()` logic have now been added to the `getGroups()` array.

As the logic for whether or not an error message should be thrown is different for each group, an extra `'name'` key has been added to each group to allow the `callback()` function to distinguish what group the detected key came from.

It also updates the error message being used to better cover both keys being looked for, as well as mention this only applies when the array is passed to `get_posts()` (in an attempt to remove some of the confusion reported in 672).

Note: this is a BC-break as the error codes for the two out of the three existing checks change!
* `WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn` is now `WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_post__not_in`.
* `WordPressVIPMinimum.Performance.WPQueryParams.SuppressFiltersTrue` is now `WordPressVIPMinimum.Performance.WPQueryParams.SuppressFilters_suppress_filters`.
* The `WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_exclude` error code, as introduced in 589, remains the same.

Fixes 594
---
 .../Sniffs/Performance/WPQueryParamsSniff.php | 72 +++++++------------
 1 file changed, 26 insertions(+), 46 deletions(-)

diff --git a/WordPressVIPMinimum/Sniffs/Performance/WPQueryParamsSniff.php b/WordPressVIPMinimum/Sniffs/Performance/WPQueryParamsSniff.php
index e9cabca4..301a9019 100644
--- a/WordPressVIPMinimum/Sniffs/Performance/WPQueryParamsSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Performance/WPQueryParamsSniff.php
@@ -8,30 +8,17 @@
 
 namespace WordPressVIPMinimum\Sniffs\Performance;
 
-use PHP_CodeSniffer\Util\Tokens;
 use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff;
 
 /**
  * Flag suspicious WP_Query and get_posts params.
  *
- *  @package VIPCS\WordPressVIPMinimum
+ * @link https://docs.wpvip.com/technical-references/caching/uncached-functions/
+ *
+ * @package VIPCS\WordPressVIPMinimum
  */
 class WPQueryParamsSniff extends AbstractArrayAssignmentRestrictionsSniff {
 
-	/**
-	 * Returns an array of tokens this test wants to listen for.
-	 *
-	 * @return array
-	 */
-	public function register() {
-		$targets = parent::register();
-
-		// Add the target for the "old" implementation.
-		$targets[] = T_CONSTANT_ENCAPSED_STRING;
-
-		return $targets;
-	}
-
 	/**
 	 * Groups of variables to restrict.
 	 *
@@ -39,44 +26,28 @@ public function register() {
 	 */
 	public function getGroups() {
 		return [
+			// WordPress.com: https://lobby.vip.wordpress.com/wordpress-com-documentation/uncached-functions/.
+			// VIP Go: https://wpvip.com/documentation/vip-go/uncached-functions/.
+			'SuppressFilters' => [
+				'name'    => 'SuppressFilters',
+				'type'    => 'error',
+				'message' => 'Setting `suppress_filters` to `true` is prohibited.',
+				'keys'    => [
+					'suppress_filters',
+				],
+			],
 			'PostNotIn' => [
+				'name'    => 'PostNotIn',
 				'type'    => 'warning',
-				'message' => 'Using `exclude`, which is subsequently used by `post__not_in`, should be done with caution, see https://docs.wpvip.com/how-tos/improve-performance-by-removing-usage-of-post__not_in/ for more information.',
+				'message' => 'Using exclusionary parameters, like %s, in calls to get_posts() should be done with caution, see https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information.',
 				'keys'    => [
+					'post__not_in',
 					'exclude',
 				],
 			],
 		];
 	}
 
-	/**
-	 * Process this test when one of its tokens is encountered
-	 *
-	 * @param int $stackPtr The position of the current token in the stack passed in $tokens.
-	 *
-	 * @return void
-	 */
-	public function process_token( $stackPtr ) {
-
-		if ( trim( $this->tokens[ $stackPtr ]['content'], '\'' ) === 'suppress_filters' ) {
-
-			$next_token = $this->phpcsFile->findNext( array_merge( Tokens::$emptyTokens, [ T_EQUAL, T_CLOSE_SQUARE_BRACKET, T_DOUBLE_ARROW ] ), $stackPtr + 1, null, true );
-
-			if ( $this->tokens[ $next_token ]['code'] === T_TRUE ) {
-				// https://docs.wpvip.com/technical-references/caching/uncached-functions/.
-				$message = 'Setting `suppress_filters` to `true` is prohibited.';
-				$this->phpcsFile->addError( $message, $stackPtr, 'SuppressFiltersTrue' );
-			}
-		}
-
-		if ( trim( $this->tokens[ $stackPtr ]['content'], '\'' ) === 'post__not_in' ) {
-			$message = 'Using `post__not_in` should be done with caution, see https://docs.wpvip.com/how-tos/improve-performance-by-removing-usage-of-post__not_in/ for more information.';
-			$this->phpcsFile->addWarning( $message, $stackPtr, 'PostNotIn' );
-		}
-
-		parent::process_token( $stackPtr );
-	}
-
 	/**
 	 * Callback to process a confirmed key which doesn't need custom logic, but should always error.
 	 *
@@ -88,6 +59,15 @@ public function process_token( $stackPtr ) {
 	 * @return bool FALSE if no match, TRUE if matches.
 	 */
 	public function callback( $key, $val, $line, $group ) {
-		return true;
+		switch ( $group['name'] ) {
+			case 'SuppressFilters':
+				return ( $val === 'true' );
+
+			case 'PostNotIn':
+				return true;
+
+			default:
+				return false;
+		}
 	}
 }