Performance/WPQueryParams: prevent false positives for 'exclude' with…

… get_users()

As reported in 672 and 729, the `get_users()` function also takes an array parameter which takes an `'exclude'` key. That key is not our target, so should not be flagged.

This commit adds a hard-coded exception specifically for that situation.

If at a later point in time, more situations which need exceptions would be discovered, this solution can be made more flexible, but for now, there is no need (or insight into where the flexibility should be).

As the `AbstractArrayAssignmentRestrictionsSniff::callback()` method does not have access to the `$stackPtr`, the logic which can be used in the `callback()` is limited. Also see the review notes from upstream 2266, which basically already pointed out this exact problem.

To get round this, I'm overloading the `process_token()` method to set a temporary `$in_get_users` property, which can then be read out in the `callback()` method to be used in the actual determination of whether the exception should be made or not.

Includes tests.

Fixes 672

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,
 		];
 	}
 }