Merge pull request #589 from Automattic/fix/369-exclude-arg-in-get_posts

WPQueryParams: sniff for `exclude` array key being set

Author: rebeccahum

WordPressVIPMinimum/Sniffs/Performance/WPQueryParamsSniff.php

@@ -8,24 +8,54 @@
 
 namespace WordPressVIPMinimum\Sniffs\Performance;
 
-use WordPressVIPMinimum\Sniffs\Sniff;
+use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff;
 use PHP_CodeSniffer\Util\Tokens;
 
 /**
  * Flag suspicious WP_Query and get_posts params.
  *
  *  @package VIPCS\WordPressVIPMinimum
  */
-class WPQueryParamsSniff extends Sniff {
+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.
+	 * This should be overridden in extending classes.
+	 *
+	 * Example: groups => array(
+	 *  'groupname' => array(
+	 *      'type'     => 'error' | 'warning',
+	 *      'message'  => 'Dont use this one please!',
+	 *      'keys'     => array( 'key1', 'another_key' ),
+	 *      'callback' => array( 'class', 'method' ), // Optional.
+	 *  )
+	 * )
+	 *
+	 * @return array
+	 */
+	public function getGroups() {
 		return [
-			T_CONSTANT_ENCAPSED_STRING,
+			'PostNotIn' => [
+				'type'    => 'warning',
+				'message' => 'Using `exclude`, which is subsequently used by `post__not_in`, should be done with caution, see https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information.',
+				'keys'    => [
+					'exclude',
+				],
+			],
 		];
 	}
 
@@ -54,6 +84,22 @@ public function process_token( $stackPtr ) {
 			$message = 'Using `post__not_in` should be done with caution, see https://wpvip.com/documentation/performance-improvements-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.
+	 *
+	 * @param  string $key   Array index / key.
+	 * @param  mixed  $val   Assigned value.
+	 * @param  int    $line  Token line.
+	 * @param  array  $group Group definition.
+	 * @return mixed         FALSE if no match, TRUE if matches, STRING if matches
+	 *                       with custom error message passed to ->process().
+	 */
+	public function callback( $key, $val, $line, $group ) {
+		return true;
 	}
 
 }

WordPressVIPMinimum/Tests/Performance/WPQueryParamsUnitTest.inc

@@ -16,4 +16,8 @@ $q = new WP_Query( $query_args );
 
 $query_args[ 'suppress_filters' ] = true;
 
-$q = new WP_query( $query_args );
+$q = new WP_query( $query_args );
+
+get_posts( [ 'exclude' => $post_ids ] ); // Warning.
+
+$exclude = [ 1, 2, 3 ];

WordPressVIPMinimum/Tests/Performance/WPQueryParamsUnitTest.php

@@ -39,6 +39,7 @@ public function getWarningList() {
 		return [
 			4  => 1,
 			11 => 1,
+			21 => 1,
 		];
 	}