False positive "High pagination limit" when `posts_per_page` is set to a function.

[justlevine]

Bug Description

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, and have no error shown.

Minimal Code Snippet

The issue happens when running this command:

phpcs ...

... over a file containing this code:

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

WordPress.WP.PostsPerPage.posts_per_page_posts_per_page

Custom ruleset

Environment

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?

• I have verified the issue still exists in the develop branch of WPCS.

[dingo-d]

This example is a tricky one because PHPCS isn't aware of the context. It doesn't parse your code and tracks what you used and where.

PHPCS cannot know if the $first, $lasst, and $default_min would fall in the 'acceptable' category of the post per page sniff (unless you've defined them).

EDIT

Looking at the sniff it doesn't do any sort of calculations, just compares the values here. And you don't have a value but a statement that does something (in your case returns the lowest value of the expression).

[justlevine]

@dingo-d exactly my point. Since we can't know the context, but see that posts_per_page is explicitly set, we should mute the error, same way that -1 is ignored, even though it might be more than 100.

[dingo-d]

You can easily do that either by excluding that sniff in your ruleset

<rule ref="WordPress.WP">
	<exclude name="WordPress.WP.PostsPerPage.posts_per_page_posts_per_page"/>
</rule>

Or ignoring that line with a comment

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 ), // phpcs:ignore WordPress.WP.PostsPerPage.posts_per_page_posts_per_page
  );
}

[justlevine]

Thanks, I am aware how to silence the sniff. However, I believe it to be a false positive, which is why I opened the issue.

[dingo-d]

This is odd, I'm not getting that error. I tried adding your code example to the unit test file, and I tried sniffing a test file with WordPress-Extra ruleset and nothing gets reported.

The value that is picked up in your case is min( max( $first, which will return false in the callback.

Can you share your ruleset?

[jrfnl]

I have been able to reproduce the issue and a fix will be included in WPCS 3.0.0.

@dingo-d I believe the reason you couldn't reproduce the issue is because you tested with PHP 7.x, while the behaviour in PHP for text to number comparisons has changed in PHP 8.0: https://3v4l.org/nOlYL#veol