Merge pull request #784 from Automattic/3.0/fix/672-wpqueryparams-prevent-false-positives-get_users

Author: GaryJones

WordPressVIPMinimum/Sniffs/Performance/WPQueryParamsSniff.php

@@ -9,6 +9,7 @@
 namespace WordPressVIPMinimum\Sniffs\Performance;
 
 use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff;
+use WordPressCS\WordPress\Helpers\ContextHelper;
 
 /**
  * Flag suspicious WP_Query and get_posts params.
@@ -19,6 +20,13 @@
  */
 class WPQueryParamsSniff extends AbstractArrayAssignmentRestrictionsSniff {
 
+	/**
+	 * Whether the current $stackPtr being scanned is nested in a function call to get_users().
+	 *
+	 * @var bool
+	 */
+	private $in_get_users = false;
+
 	/**
 	 * Groups of variables to restrict.
 	 *
@@ -48,6 +56,20 @@ public function getGroups() {
 		];
 	}
 
+	/**
+	 * Processes this test, when one of its tokens is encountered.
+	 *
+	 * @param int $stackPtr The position of the current token in the stack.
+	 *
+	 * @return void
+	 */
+	public function process_token( $stackPtr ) {
+		$this->in_get_users = ContextHelper::is_in_function_call( $this->phpcsFile, $stackPtr, [ 'get_users' => true ], true, true );
+
+		parent::process_token( $stackPtr );
+	}
+
+
 	/**
 	 * Callback to process a confirmed key which doesn't need custom logic, but should always error.
 	 *
@@ -64,6 +86,11 @@ public function callback( $key, $val, $line, $group ) {
 				return ( $val === 'true' );
 
 			case 'PostNotIn':
+				if ( $key === 'exclude' && $this->in_get_users !== false ) {
+					// This is not an array used by get_posts(). See #672.
+					return false;
+				}
+
 				return true;
 
 			default:

WordPressVIPMinimum/Tests/Performance/WPQueryParamsUnitTest.inc

@@ -21,3 +21,11 @@ $q = new WP_query( $query_args );
 get_posts( [ 'exclude' => $post_ids ] ); // Warning.
 
 $exclude = [ 1, 2, 3 ];
+
+// Issue #672 / #729.
+get_users( [ 'exclude' => $post_ids ] ); // OK.
+get_users( My\get_args( [ 'exclude' => $post_ids ] ) ); // OK - arbitrary as the call to `My\get_args()` on its own would be flagged, but let's allow it.
+
+$context_unknown = [ 'exclude' => $post_ids ]; // Warning.
+other_fn_calls_still_throw_warning( [ 'exclude' => $post_ids ] ); // Warning.
+get_users( [ 'suppress_filters' => true ] ); // Error - not necessarily valid, but the exception being made is specifically about `exclude`.

WordPressVIPMinimum/Tests/Performance/WPQueryParamsUnitTest.php

@@ -27,6 +27,7 @@ public function getErrorList() {
 		return [
 			5  => 1,
 			17 => 1,
+			31 => 1,
 		];
 	}
 
@@ -40,6 +41,8 @@ public function getWarningList() {
 			4  => 1,
 			11 => 1,
 			21 => 1,
+			29 => 1,
+			30 => 1,
 		];
 	}
 }