Repository: WordPress/WordPress-Coding-Standards (php)

Issue:
False positive "High pagination limit" when `posts_per_page` is set to a function.
<!--
PLEASE FILL OUT THE TEMPLATE COMPLETELY.
BUG REPORTS WHICH CANNOT BE REPRODUCED BASED ON THE INFORMATION PROVIDED WILL BE CLOSED.
-->

## Bug Description
<!--
Please provide a clear and concise description of what the bug is.

What did you expect to happen? What actually happened?
-->

When setting `posts_per_page` to a function, wpcs logs "Detected high pagination limit, `posts_per_page` is set to..." warning.
It should [be treated like `-1`](https://github.com/WordPress/WordPress-Coding-Standards/issues/1638#issuecomment-459141404), and have no error shown.

## Minimal Code Snippet
<!-- Please provide example code that allows us to reproduce the issue. Do NOT paste screenshots of code! -->

The issue happens when running this command:
```bash
phpcs ...
```

... over a file containing this code:
```php
function get_query_args( int $first, int $last, int $default_min = 10 ) : array {
  return array(
    // other query args...
    'posts_per_page' => min( max( $first, $last ), $default_min ), 
  );
}
```

## Error Code
<!--
The error code for the sniff that is (or should be) being triggered (you
can see the sniff error codes by running `phpcs` with the `-s` flag).
e.g. `WordPress.PHP.NoSilencedErrors.Discouraged`

You can leave this section empty if you are reporting a false negative.
-->
`WordPress.WP.PostsPerPage.posts_per_page_posts_per_page`

## Custom ruleset
<!--
If the issue cannot be reproduced when using `--standard=WordPress` on the command line,
please post the relevant part of your custom ruleset here.
-->

## Environment
<!--
To find out the versions used:
* PHP: run `php -v`.
* PHPCS: run `[vendor/bin/]phpcs --version`
* WPCS: run `composer [global] info` for a Composer install.
-->

| Question               | Answer
| ------------------------| -------
| PHP version             | 8.0.15
| PHP_CodeSniffer version | 3.6.2
| WPCS version            | 2.3.0, dev-develop
| WPCS install type       | Composer project local
| IDE (if relevant)       | VSCode on wsl2


## Tested Against `develop` branch?
- [x] I have verified the issue still exists in the `develop` branch of WPCS.

AbstractArrayAssignmentRestrictionsSniff doesn't correctly capture array values with no trailing comma
## Bug Description
Sniff classes that extend the [`AbstractArrayAssignmentRestrictionsSniff` class](https://github.com/WordPress/WordPress-Coding-Standards/blob/develop/WordPress/AbstractArrayAssignmentRestrictionsSniff.php) don't capture the correct value when the array item has no trailing comma.

## Minimal Code Snippet

### Example 1

Testing the [`PostPerPage` sniff](https://github.com/WordPress/WordPress-Coding-Standards/blob/develop/WordPress/Sniffs/WP/PostsPerPageSniff.php) which extends `AbstractArrayAssignmentRestrictionsSniff`. With this test file:

```
<?php

$args = array(
	'posts_per_page' => 999,
);

$args = array(
	'posts_per_page' => 999
);
```

...and this command (run inside a Git clone of WPCS):
```
$ ./vendor/bin/phpcs --standard=WordPress ../phpcs-test.php
```

The output includes:

```
 1 | ERROR   | [ ] Missing file doc comment (Squiz.Commenting.FileComment.Missing)
 4 | WARNING | [ ] Detected high pagination limit, `posts_per_page` is set to `999`
   |         |     (WordPress.WP.PostsPerPage.posts_per_page_posts_per_page)
 8 | WARNING | [ ] Detected high pagination limit, `posts_per_page` is set to `999
   |         |     )` (WordPress.WP.PostsPerPage.posts_per_page_posts_per_page)
 8 | ERROR   | [x] Each array item in a multi-line array declaration must end in a comma (WordPress.Arrays.CommaAfterArrayItem.NoComma)
```

Line 4 and line 8 (the `999` lines) are both captured as a violation, though interestingly, the "value" captured for line 8 seems to be `999 )` (with a line break as well), as though it hasn't recognised the line break as being the intended end of the line/value. I suspect there's some casting to int going on where `999\n(` is seen as `999`, which is sufficient to make it a violation.

----

### Example 2

From the [VIPCS `NoPaging` sniff](https://github.com/Automattic/VIP-Coding-Standards/blob/develop/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php), which also extends WPCS's `AbstractArrayAssignmentRestrictionsSniff`, a test file of:

```
<?php

$args = array(
	'nopaging' => true,
);

$args = array(
	'nopaging' => true
);
```
...and a command of:

```
$ ./vendor/bin/phpcs --standard=WordPress-VIP-Go -s ../phpcs-test.php
```
...only captures the first instance:

```
 4 | ERROR | Disabling pagination is prohibited in VIP context, do not set `nopaging` to `true` ever.
   |       | (WordPressVIPMinimum.Performance.NoPaging.nopaging_nopaging)
```

...because I suspect the second value is seen as `true )` (with a line break), which doesn't resolve to true (unlike the int casting from Example 1).

Testing with `$args = array('nopaging=true');` and the violation is correctly captured.


## Environment
<!--
To find out the versions used:
* PHP: run `php -v`.
* PHPCS: run `[vendor/bin/]phpcs --version`
* WPCS: run `composer [global] info` for a Composer install.
-->

| Question               | Answer
| ------------------------| -------
| PHP version             | 7.4 and 8.1
| PHP_CodeSniffer version | 3.7.1
| WPCS version            | `develop`
| WPCS install type       | git clone for development

## Additional Context (optional)
Originally reported in https://github.com/Automattic/VIP-Coding-Standards/issues/713.

## Tested Against `develop` branch?
- [x] I have verified the issue still exists in the `develop` branch of WPCS.


Pull request:
AbstractArrayAssignmentRestrictions: implement PHPCSUtils, bug fixes, modern PHP and more
## Review notes

This PR is the result of a comprehensive review of the `AbstractArrayAssignmentRestrictionsSniff` class.

While this PR makes significant improvements, there is still more wrong with this abstract:

* The text quotes are stripped off the value before it gets passed to the callback, meaning the callback can no longer determine what type of value was received, which could lead to false positives/negatives.
    Example: https://3v4l.org/YH5Rp (and yes, this means that even though numeric literals with underscores should be supported, we cannot do so reliably).
* A compound key, like `[ $prefix . 'posts_per_page' ]`, will be flagged as if the key were `'posts_per_page'` as the key does not get captured in its entirety.
* Along the same lines, the fact that the callback method gets a string value for `$key` and `$value` is wrong as it doesn't allow the sniffs to walk the tokens for a proper analysis of the key/value.

To fix all of that, would IMO require changing the method signature for the `callback()` method.
Adjusting the method signature is non-trivial as it would instantly cause fatal errors for all sniffs using the abstract due to the signature mismatch: https://3v4l.org/0tf0c

Alternatively, adding a second callback method option which would receive stack pointers could be considered.

In both cases, that would be quite a big breaking change and even though we are at 3.0.0, I have not seen any issues reported asking for a change along these lines, so I don't think making that breaking change is justified.

If anything, I think we may need to introduce a replacement for this abstract at some point (which passes the tokens to the callback) and deprecate this one. Then dev-users can migrate to the replacement at their own pace.

That is outside the scope of this PR however. This PR attempts to make _what's there_ better in the full knowledge that this is an abstract which shouldn't exist in its current form.


## Commit details

### AbstractArrayAssignmentRestrictions: add some extra tests

... to raise code coverage and cover some situations previously untested.

Tests have been added to the `WordPress.WP.PostsPerPage` sniff, which contains a `@covers` tag for the abstract.

### AbstractArrayAssignmentRestrictions: improve documentation

Most notably: the value for `'message'` as was appears to have led to sniff devs not using the key, which was never the intention.

### AbstractArrayAssignmentRestrictions: minor readability fix

### AbstractArrayAssignmentRestrictions: implement PHPCSUtils

### AbstractArrayAssignmentRestrictions: bug fix - improve comment tolerance [1]

Non-code style sniffs should ignore most whitespace and comment tokens.

The `AbstractArrayAssignmentRestrictions` did not do so correctly, which could lead to false negatives.

This commit fixes the handling of comments between the "key" and the assignment operator by the sniff.

Includes tests via the `WordPress.WP.PostsPerPage` sniff.

Related to #763

### AbstractArrayAssignmentRestrictions: bug fix - improve comment tolerance [2]

Non-code style sniffs should ignore most whitespace and comment tokens.

The `AbstractArrayAssignmentRestrictions` did not do so correctly, which could lead to false negatives.

This commit fixes the handling of comments between the assignment operator and the assigned value.

Includes tests via the `WordPress.WP.PostsPerPage` sniff.

Related to #763

### AbstractArrayAssignmentRestrictions: minor efficiency tweak [1]

Get rid of two unnecessary `in_array()` function calls.

### AbstractArrayAssignmentRestrictions: minor efficiency tweak [2]

The position of the `T_EQUAL` token has already been determined before, so let's use the predetermined value for efficiency.

Includes renaming the local variable to be slightly more descriptive.

### AbstractArrayAssignmentRestrictions: minor efficiency tweak [3]

No need to start looping the inner array if the `$key` is not one of the target keys in the current group.

### AbstractArrayAssignmentRestrictions: precision (bug) fix

Check if the array "key" token found is a text string token as well as checking that the content is non-numeric.

This is also an efficiency fix as code like `$query_args[] = 300;` would previously be analyzed with key `[`, which is clearly nonsense.

Along the same lines, a numeric string key would previously also be analyzed, due to the quotes around the number, while PHP auto-casts numeric string keys to integers (see: https://3v4l.org/uVFsO ), which means they are not the target of this sniff and therefore should be disregarded.
This also meant that the `$key` passed to the `callback()` method could previously potentially be an integer (as the same type casts happens when we store the numeric string key to the `$inst` array), while it is documented as `string` only.

Note: this didn't lead to false positives as nobody would set the "key" to search for to `[`, but it was still wrong.

Includes tests via the `WordPress.WP.PostsPerPage` sniff.

### AbstractArrayAssignmentRestrictions: bug fix - simplify the $inst(ances) array

As things were, the sniff would store the encountered "instances" in a multi-dimensional array looking like this:
```
array(
    (string) 'key' => array(
        [0] => array(
            [0] => (string) 'value',
            [1] => (int) line number,
        ),
        ...
    )
)
```

Now, the sniffs listens to four different tokens:
* `T_DOUBLE_ARROW`
* `T_CLOSE_SQUARE_BRACKET`
* `T_CONSTANT_ENCAPSED_STRING`
* `T_DOUBLE_QUOTED_STRING`

For the first two, there could only ever be one entry in the lowest level of the array as it looks at each array item individually due to the use of the `T_DOUBLE_ARROW` token for array declarations and the `T_CLOSE_SQUARE_BRACKET` token being used for assignments to an existing array.

For the last two, the found string is analyzed to see if it is a query string and parsed if it is. In that case, there could be multiple entries in the array.

However.... when PHP parses a query string, there will never be multiple entries for the same key: https://3v4l.org/n3Iv9

So, I cannot fanthom why having multiple entries for the same key would be useful for this sniff. Only the last found entry should be taken into account.
There were also no tests (anywhere) covering a query string containing a duplicate key.

So, taking the above into account, this commit simplifies the entries are stored in the `$inst` array to this:
```
array(
    (string) 'key' => array(
        [0] => (string) 'value',
        [1] => (int) line number,
    )
)
```

This array format change will now prevent incorrect messages about a "high" value which is subsequently overwritten by a "low" value anyway.

Includes tests via the `WordPress.WP.PostsPerPage` sniff.

### AbstractArrayAssignmentRestrictions: make the contents of $inst more comprehensible

Even though the array is only used locally, let's use keys for the entries to make the code more readable/comprehensible.

### AbstractArrayAssignmentRestrictions: report the error for array assignments on the key pointer

For array assignments, the error would previously be reported on a `]` or `=>` token, while those tokens in and of themselves are not where the problem lies.

As the sniff reports based on the key (with an optional value check in the `callback()` method), I think it is more appropriate and more correct for the sniff to report on the stack pointer to the token identified as the key.

To achieve this, a new array entry is added to the `$inst` array so the loop throwing the error has access to that information.

For query strings, the error will continue to be thrown on the text string token containing the query string.

The difference this commit makes can be seen when using the `code` report offered by PHPCS, like so `--report=code`.

Includes tests via the `WordPress.WP.PostsPerPage` sniff.

### AbstractArrayAssignmentRestrictions: allow for PHP 7.4+ null coalesce equals

While other assignment operators all lead to calculations, which means we cannot reliably determine the actual value, the null coalesce assignment operator provides a default value, which means we can analyze the value as if it were the value set.

Includes tests via the `WordPress.WP.PostsPerPage` sniff.

### AbstractArrayAssignmentRestrictions: add test with PHP 8.0 match

Includes tests via the `WordPress.WP.PostsPerPage` sniff.

### AbstractArrayAssignmentRestrictions: bug fix - improve value capturing

This commit improves the way the `AbstractArrayAssignmentRestrictionsSniff` captures the value of an array key assignment before it gets passed on to the callback for further examination.

As things were previously, the sniff would go wrong in two cases:
1. Without a trailing comma after an array item, the long/short array close bracket would be included in the value, while it shouldn't be.
    This was reported in issue #2211.
2. The sniff would not always capture a complete value when the value contained a function call, sub-array, closure etc; basically anything which can contain a comma or semi-colon, but where that comma/semi-colon is in an "inner" construct and not the "outer" construct which is being captured.
    This was reported in issue #2027

The fix included in this commit fixes the first issue completely and a partial fix for the second issue.

Includes tests via the `WordPress.WP.PostsPerPage` sniff.

Fixes #2211

Note: a follow-up PR which builds onto this commit will address #2027.

---

### 🆕 AbstractArrayAssignmentRestrictionsSniff: don't strip quotes off value

As the value capturing is now more likely to (correctly) contain multiple tokens, stripping the quotes of a value is plain incorrect as wit would lead to a value like this:
```php
$var['key'] = 'my' . 10 . 'something';
```
... to be passed on as `my' . 10 . 'something` - i.e. without the surrounding quotes, but with the non-matching quotes inside the value.

I'm proposing to remove the call to `TextStrings::stripQuotes()` to allow the `callback()` to receive a more accurate and actionable value.

This will be a potentially breaking change for _some_ extenders, so will need a clear entry in the dev upgrade guide. (It doesn't impact the WPCS native sniffs extending the abstract at this time)

Patch:
diff --git a/WordPress/AbstractArrayAssignmentRestrictionsSniff.php b/WordPress/AbstractArrayAssignmentRestrictionsSniff.php
index 223b5e1b..13615429 100644
--- a/WordPress/AbstractArrayAssignmentRestrictionsSniff.php
+++ b/WordPress/AbstractArrayAssignmentRestrictionsSniff.php
@@ -9,6 +9,8 @@
 
 namespace WordPressCS\WordPress;
 
+use PHP_CodeSniffer\Util\Tokens;
+use PHPCSUtils\Utils\GetTokensAsString;
 use PHPCSUtils\Utils\MessageHelper;
 use PHPCSUtils\Utils\TextStrings;
 use WordPressCS\WordPress\Helpers\RulesetPropertyHelper;
@@ -36,7 +38,7 @@ abstract class AbstractArrayAssignmentRestrictionsSniff extends Sniff {
 	 * @since 1.0.0 This property now expects to be passed an array.
 	 *              Previously a comma-delimited string was expected.
 	 *
-	 * @var array
+	 * @var string[]
 	 */
 	public $exclude = array();
 
@@ -45,7 +47,7 @@ abstract class AbstractArrayAssignmentRestrictionsSniff extends Sniff {
 	 * Don't use this in extended classes, override getGroups() instead.
 	 * This is only used for Unit tests.
 	 *
-	 * @var array
+	 * @var array<string, array>
 	 */
 	public static $groups = array();
 
@@ -54,7 +56,7 @@ abstract class AbstractArrayAssignmentRestrictionsSniff extends Sniff {
 	 *
 	 * @since 0.11.0
 	 *
-	 * @var array
+	 * @var array<string, bool>
 	 */
 	protected $excluded_groups = array();
 
@@ -63,7 +65,7 @@ abstract class AbstractArrayAssignmentRestrictionsSniff extends Sniff {
 	 *
 	 * @since 0.13.0
 	 *
-	 * @var array
+	 * @var array<string, array>
 	 */
 	protected $groups_cache = array();
 
@@ -94,13 +96,13 @@ abstract class AbstractArrayAssignmentRestrictionsSniff extends Sniff {
 	 * Example: groups => array(
 	 *  'groupname' => array(
 	 *      'type'     => 'error' | 'warning',
-	 *      'message'  => 'Dont use this one please!',
+	 *      'message'  => 'Descriptive error message. The error message will be passed the $key and $val of the current array assignment.',
 	 *      'keys'     => array( 'key1', 'another_key' ),
 	 *      'callback' => array( 'class', 'method' ), // Optional.
 	 *  )
 	 * )
 	 *
-	 * @return array
+	 * @return array<string, array>
 	 */
 	abstract public function getGroups();
 
@@ -145,13 +147,16 @@ abstract class AbstractArrayAssignmentRestrictionsSniff extends Sniff {
 		$token = $this->tokens[ $stackPtr ];
 
 		if ( \T_CLOSE_SQUARE_BRACKET === $token['code'] ) {
-			$equal = $this->phpcsFile->findNext( \T_WHITESPACE, ( $stackPtr + 1 ), null, true );
-			if ( \T_EQUAL !== $this->tokens[ $equal ]['code'] ) {
-				return; // This is not an assignment!
+			$equalPtr = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
+			if ( \T_EQUAL !== $this->tokens[ $equalPtr ]['code']
+				&& \T_COALESCE_EQUAL !== $this->tokens[ $equalPtr ]['code']
+			) {
+				// This is not an assignment. Bow out.
+				return;
 			}
 		}
 
-		// Instances: Multi-dimensional array, keyed by line.
+		// Instances: Multi-dimensional array.
 		$inst = array();
 
 		/*
@@ -159,28 +164,48 @@ abstract class AbstractArrayAssignmentRestrictionsSniff extends Sniff {
 		 * `$foo = array( 'bar' => 'taz' );`
 		 * `$foo['bar'] = $taz;`
 		 */
-		if ( \in_array( $token['code'], array( \T_CLOSE_SQUARE_BRACKET, \T_DOUBLE_ARROW ), true ) ) {
+		if ( \T_CLOSE_SQUARE_BRACKET === $token['code'] || \T_DOUBLE_ARROW === $token['code'] ) {
 			$operator = $stackPtr; // T_DOUBLE_ARROW.
 			if ( \T_CLOSE_SQUARE_BRACKET === $token['code'] ) {
-				$operator = $this->phpcsFile->findNext( \T_EQUAL, ( $stackPtr + 1 ) );
+				$operator = $equalPtr;
 			}
 
-			$keyIdx = $this->phpcsFile->findPrevious( array( \T_WHITESPACE, \T_CLOSE_SQUARE_BRACKET ), ( $operator - 1 ), null, true );
-			if ( ! is_numeric( $this->tokens[ $keyIdx ]['content'] ) ) {
-				$key            = TextStrings::stripQuotes( $this->tokens[ $keyIdx ]['content'] );
-				$valStart       = $this->phpcsFile->findNext( array( \T_WHITESPACE ), ( $operator + 1 ), null, true );
-				$valEnd         = $this->phpcsFile->findNext( array( \T_COMMA, \T_SEMICOLON ), ( $valStart + 1 ), null, false, null, true );
-				$val            = $this->phpcsFile->getTokensAsString( $valStart, ( $valEnd - $valStart ) );
-				$val            = TextStrings::stripQuotes( $val );
-				$inst[ $key ][] = array( $val, $token['line'] );
+			$keyIdx = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true );
+			if ( isset( Tokens::$stringTokens[ $this->tokens[ $keyIdx ]['code'] ] )
+				&& ! is_numeric( $this->tokens[ $keyIdx ]['content'] )
+			) {
+				$key      = TextStrings::stripQuotes( $this->tokens[ $keyIdx ]['content'] );
+				$valStart = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $operator + 1 ), null, true );
+				$valEnd   = $this->phpcsFile->findEndOfStatement( $valStart, \T_COLON );
+				if ( \T_COMMA === $this->tokens[ $valEnd ]['code']
+					|| \T_SEMICOLON === $this->tokens[ $valEnd ]['code']
+				) {
+					// FindEndOfStatement includes the comma/semi-colon if that's the end of the statement.
+					// That's not what we want (and inconsistent), so remove it.
+					--$valEnd;
+				}
+
+				$val          = trim( GetTokensAsString::compact( $this->phpcsFile, $valStart, $valEnd, true ) );
+				$inst[ $key ] = array(
+					'value'  => $val,
+					'line'   => $token['line'],
+					'keyptr' => $keyIdx,
+				);
 			}
-		} elseif ( \in_array( $token['code'], array( \T_CONSTANT_ENCAPSED_STRING, \T_DOUBLE_QUOTED_STRING ), true ) ) {
-			// Covers assignments via query parameters: `$foo = 'bar=taz&other=thing';`.
+		} elseif ( isset( Tokens::$stringTokens[ $token['code'] ] ) ) {
+			/*
+			 * Covers assignments via query parameters: `$foo = 'bar=taz&other=thing';`.
+			 */
 			if ( preg_match_all( '#(?:^|&)([a-z_]+)=([^&]*)#i', TextStrings::stripQuotes( $token['content'] ), $matches ) <= 0 ) {
 				return; // No assignments here, nothing to check.
 			}
-			foreach ( $matches[1] as $i => $_k ) {
-				$inst[ $_k ][] = array( $matches[2][ $i ], $token['line'] );
+
+			foreach ( $matches[1] as $match_nr => $key ) {
+				$inst[ $key ] = array(
+					'value'  => $matches[2][ $match_nr ],
+					'line'   => $token['line'],
+					'keyptr' => $stackPtr,
+				);
 			}
 		}
 
@@ -196,33 +221,29 @@ abstract class AbstractArrayAssignmentRestrictionsSniff extends Sniff {
 
 			$callback = ( isset( $group['callback'] ) && is_callable( $group['callback'] ) ) ? $group['callback'] : array( $this, 'callback' );
 
-			foreach ( $inst as $key => $assignments ) {
-				foreach ( $assignments as $occurance ) {
-					list( $val, $line ) = $occurance;
-
-					if ( ! \in_array( $key, $group['keys'], true ) ) {
-						continue;
-					}
-
-					$output = \call_user_func( $callback, $key, $val, $line, $group );
-
-					if ( ! isset( $output ) || false === $output ) {
-						continue;
-					} elseif ( true === $output ) {
-						$message = $group['message'];
-					} else {
-						$message = $output;
-					}
-
-					MessageHelper::addMessage(
-						$this->phpcsFile,
-						$message,
-						$stackPtr,
-						( 'error' === $group['type'] ),
-						MessageHelper::stringToErrorcode( $groupName . '_' . $key ),
-						array( $key, $val )
-					);
+			foreach ( $inst as $key => $assignment ) {
+				if ( ! \in_array( $key, $group['keys'], true ) ) {
+					continue;
 				}
+
+				$output = \call_user_func( $callback, $key, $assignment['value'], $assignment['line'], $group );
+
+				if ( ! isset( $output ) || false === $output ) {
+					continue;
+				} elseif ( true === $output ) {
+					$message = $group['message'];
+				} else {
+					$message = $output;
+				}
+
+				MessageHelper::addMessage(
+					$this->phpcsFile,
+					$message,
+					$assignment['keyptr'],
+					( 'error' === $group['type'] ),
+					MessageHelper::stringToErrorcode( $groupName . '_' . $key ),
+					array( $key, $assignment['value'] )
+				);
 			}
 		}
 	}
@@ -232,12 +253,13 @@ abstract class AbstractArrayAssignmentRestrictionsSniff extends Sniff {
 	 *
 	 * This method must be extended to add the logic to check assignment value.
 	 *
-	 * @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().
+	 * @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().
 	 */
 	abstract public function callback( $key, $val, $line, $group );
 }


Test patch:
diff --git a/WordPress/Tests/WP/PostsPerPageUnitTest.inc b/WordPress/Tests/WP/PostsPerPageUnitTest.inc
index 40b480b5..cddd9797 100644
--- a/WordPress/Tests/WP/PostsPerPageUnitTest.inc
+++ b/WordPress/Tests/WP/PostsPerPageUnitTest.inc
@@ -28,3 +28,56 @@ $query_args['posts_per_page'] = 100; // OK.
 $query_args['posts_per_page'] = 200; // OK.
 $query_args['posts_per_page'] = 300; // Bad.
 // phpcs:set WordPress.WP.PostsPerPage posts_per_page 100
+
+// phpcs:set WordPress.WP.PostsPerPage exclude[] posts_per_page
+$query_args['posts_per_page'] = 300; // OK, group excluded.
+// phpcs:set WordPress.WP.PostsPerPage exclude[]
+
+// Ensure there will be no false positive for array access brackets when not used for an assignment.
+$var = $query_args['posts_per_page']; // OK.
+$firstChars = $text[0] . $text[1]; // OK.
+
+// Text strings which are not query strings should be ignored.
+_query_posts( 'numberposts' ); // OK.
+
+// Assignments to non-string keys should be ignored. Note: PHP will auto-cast numeric strings to ints, so those should also be disregarded.
+$var[10] = 300; // OK.
+$var[] = 400; // OK.
+$var['239'] = 500; // OK.
+
+// Ensure the sniff disregards comments.
+$query_args['posts_per_page' /* high */ ] = 999; // Bad.
+
+$query_args['posts_per_page'] /* high */  = 999; // Bad.
+
+$args = array(
+	'posts_per_page' /* high */ => 999, // Bad.
+);
+
+$query_args['posts_per_page'] = /* high */ 999; // Bad.
+$args = array(
+	'posts_per_page' => /* high */ 999, // Bad.
+);
+
+// Safeguard that when a query string contains duplicate key, the value of the last one is used.
+_query_posts( 'posts_per_page=999&nopaging=true&posts_per_page=50' ); // OK.
+
+// Ensure the error gets reported on the key pointer.
+$query_args[
+	'posts_per_page'
+] = 300; // Bad, error should be reported on the above line.
+
+// Ensure that PHP 7.4 null coalesce equals get picked up on.
+$query_args['posts_per_page'] ??= 50; // OK.
+$query_args['posts_per_page'] ??= 200; // Bad.
+
+// Ensure that the sniff does not report on PHP 8.0 match arms.
+$val = match($val) {
+	'posts_per_page' => 999, // OK, not an array assignment.
+};
+
+// Verify handling of arrays without trailing comma after the last array item.
+$args = array( 'posts_per_page' => 999 ); // Bad.
+$args = [
+	'posts_per_page' => 999
+]; // Bad.
diff --git a/WordPress/Tests/WP/PostsPerPageUnitTest.php b/WordPress/Tests/WP/PostsPerPageUnitTest.php
index 442ae259..7dbd98b3 100644
--- a/WordPress/Tests/WP/PostsPerPageUnitTest.php
+++ b/WordPress/Tests/WP/PostsPerPageUnitTest.php
@@ -51,6 +51,15 @@ final class PostsPerPageUnitTest extends AbstractSniffUnitTest {
 			23 => 1,
 			24 => 1,
 			29 => 1,
+			49 => 1,
+			51 => 1,
+			54 => 1,
+			57 => 1,
+			59 => 1,
+			67 => 1,
+			72 => 1,
+			80 => 1,
+			82 => 1,
 		);
 	}
 }