Skip to content

Commit

Permalink
WP/PostsPerPage: bug fix - non-numeric values for "posts_per_page" sh…
Browse files Browse the repository at this point in the history
…ould be disregarded

This expands the tests for the "value capturing" fix in the previous commit.

Additionally, it adds a safeguard to the `WordPress.WP.PostsPerPage` sniff `callback()` method to prevent reporting an error when the value captured is not purely numeric.

This safeguard is needed as the way [text to number comparisons](https://wiki.php.net/rfc/string_to_number_comparison) are executed has changed in PHP 8.0 and that change affects this sniff.
See: https://3v4l.org/nOlYL#veol

Fixes 2027
  • Loading branch information
jrfnl committed Jul 4, 2023
1 parent 738f336 commit 0d37c0c
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 18 deletions.
9 changes: 8 additions & 1 deletion WordPress/Sniffs/WP/PostsPerPageSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace WordPressCS\WordPress\Sniffs\WP;

use PHPCSUtils\Utils\TextStrings;
use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff;

/**
Expand Down Expand Up @@ -67,6 +68,12 @@ public function getGroups() {
* @return bool FALSE if no match, TRUE if matches.
*/
public function callback( $key, $val, $line, $group ) {
return ( $val > (int) $this->posts_per_page );
$val = TextStrings::stripQuotes( $val );
if ( preg_match( '`^[-]?[0-9]+$`', $val ) !== 1 ) {
// Not a purely numeric value, so any comparison would be a false comparison.
return false;
}

return ( (int) $val > (int) $this->posts_per_page );
}
}
21 changes: 21 additions & 0 deletions WordPress/Tests/WP/PostsPerPageUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,24 @@ $args = array( 'posts_per_page' => 999 ); // Bad.
$args = [
'posts_per_page' => 999
]; // Bad.

// Verify that the complete value is being captured correctly and that non-numeric values are disregarded.
$args = array(
'posts_per_page' => min( max( $first, $last ), $default_min ), // Should be ignored as undetermined.
'posts_per_page' => 10 + $extra, // Should be ignored as undetermined.
'posts_per_page' => $value[0][1], // Should be ignored as undetermined.
'posts_per_page' => $value ? 10 * $value : 300, // Should be ignored as undetermined.
'posts_per_page' => get_value( name: 'post_per_page', type: 'query' ), // Should be ignored as undetermined.
'posts_per_page' => function($a): int {
return do_something( $a );
}, // Should be ignored as undetermined.
'posts_per_page' => [ 100, 200, 300 ], // Should be ignored as undetermined.
'posts_per_page' => array(100, 200, 300), // Should be ignored as undetermined.
);
$query_args['posts_per_page'] = my\get_posts_per_page(); // Should be ignored as undetermined.
$query_args['posts_per_page'] = '1e3'; // Should be ignored as undetermined. Would evaluate to 1000 with an int cast, but WP doesn't cast the value.

// Purely numeric strings should probably be accepted still as this won't make a difference for the database query.
$query_args['posts_per_page'] = '50'; // OK.
$query_args['posts_per_page'] = '200'; // Bad.
$query_args['posts_per_page'] = "200"; // Bad.
36 changes: 19 additions & 17 deletions WordPress/Tests/WP/PostsPerPageUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,25 @@ public function getErrorList() {
*/
public function getWarningList() {
return array(
4 => 1,
10 => 1,
11 => 1,
13 => 1,
22 => 1,
23 => 1,
24 => 1,
29 => 1,
49 => 1,
51 => 1,
54 => 1,
57 => 1,
59 => 1,
67 => 1,
72 => 1,
80 => 1,
82 => 1,
4 => 1,
10 => 1,
11 => 1,
13 => 1,
22 => 1,
23 => 1,
24 => 1,
29 => 1,
49 => 1,
51 => 1,
54 => 1,
57 => 1,
59 => 1,
67 => 1,
72 => 1,
80 => 1,
82 => 1,
103 => 1,
104 => 1,
);
}
}

0 comments on commit 0d37c0c

Please sign in to comment.