Skip to content

Commit d00302f

Browse files
committed
feat: flag posts_per_page/numberposts set to -1
WPCS's PostsPerPageSniff uses a `> 100` comparison to flag high values, which means `-1` (WordPress's "no limit" sentinel) slips through uncaught. The NoPaging sniff is the natural home for this check since setting posts_per_page to -1 is functionally equivalent to nopaging. Adds a `posts_per_page` group to NoPagingSniff covering both `posts_per_page` and `numberposts` keys, with a callback that specifically targets the `-1` value via TextStrings::stripQuotes(). Also expands NoPaging test coverage substantially (#533): nopaging with integer 1, false, 0, variable assignments, and short array syntax are now all exercised. Both ruleset test files updated to account for the new test cases. Refs #364, #533.
1 parent ac9c5fc commit d00302f

File tree

7 files changed

+159
-70
lines changed

7 files changed

+159
-70
lines changed

WordPress-VIP-Go/ruleset-test.inc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,9 @@ $args = array(
466466
'nopaging' => true, // Error.
467467
);
468468
_query_posts( 'nopaging=true' ); // Error.
469+
$args = array(
470+
'posts_per_page' => -1, // Error.
471+
);
469472

470473
// WordPressVIPMinimum.Performance.OrderByRand
471474
$args = array(

WordPress-VIP-Go/ruleset-test.php

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,13 @@
8888
462 => 1,
8989
466 => 1,
9090
468 => 1,
91-
472 => 1,
92-
474 => 1,
93-
480 => 1,
94-
486 => 1,
95-
494 => 1,
96-
507 => 1,
97-
511 => 1,
98-
512 => 1,
99-
513 => 1,
91+
470 => 1,
92+
475 => 1,
93+
477 => 1,
94+
483 => 1,
95+
489 => 1,
96+
497 => 1,
97+
510 => 1,
10098
514 => 1,
10199
515 => 1,
102100
516 => 1,
@@ -105,16 +103,19 @@
105103
519 => 1,
106104
520 => 1,
107105
521 => 1,
108-
525 => 1,
109-
527 => 1,
110-
545 => 1,
111-
560 => 1,
112-
564 => 1,
113-
565 => 1,
114-
566 => 1,
106+
522 => 1,
107+
523 => 1,
108+
524 => 1,
109+
528 => 1,
110+
530 => 1,
111+
548 => 1,
112+
563 => 1,
115113
567 => 1,
116-
572 => 1,
117-
574 => 1,
114+
568 => 1,
115+
569 => 1,
116+
570 => 1,
117+
575 => 1,
118+
577 => 1,
118119
],
119120
'warnings' => [
120121
7 => 1,
@@ -224,14 +225,14 @@
224225
454 => 1,
225226
455 => 1,
226227
456 => 1,
227-
502 => 1,
228-
503 => 1,
229-
530 => 1,
228+
505 => 1,
229+
506 => 1,
230230
533 => 1,
231-
540 => 1,
232-
550 => 1,
233-
556 => 1,
234-
579 => 1,
231+
536 => 1,
232+
543 => 1,
233+
553 => 1,
234+
559 => 1,
235+
582 => 1,
235236
],
236237
'messages' => [
237238
7 => [

WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@
99

1010
namespace WordPressVIPMinimum\Sniffs\Performance;
1111

12+
use PHPCSUtils\Utils\TextStrings;
1213
use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff;
1314

1415
/**
15-
* Flag returning high or infinite posts_per_page.
16+
* Flag disabling pagination via `nopaging` or `posts_per_page`/`numberposts` set to `-1`.
1617
*
1718
* @link https://docs.wpvip.com/technical-references/code-review/#no-limit-queries
1819
*
@@ -29,11 +30,19 @@ public function getGroups() {
2930
return [
3031
'nopaging' => [
3132
'type' => 'error',
32-
'message' => 'Disabling pagination is prohibited in VIP context, do not set `%s` to `%s` ever.',
33+
'message' => 'Disabling pagination is prohibited in VIP context, do not set `%s` to `%s`.',
3334
'keys' => [
3435
'nopaging',
3536
],
3637
],
38+
'posts_per_page' => [
39+
'type' => 'error',
40+
'message' => 'Setting `%s` to `%s` disables pagination and is prohibited in VIP context.',
41+
'keys' => [
42+
'posts_per_page',
43+
'numberposts',
44+
],
45+
],
3746
];
3847
}
3948

@@ -50,6 +59,12 @@ public function getGroups() {
5059
public function callback( $key, $val, $line, $group ) {
5160
$key = strtolower( $key );
5261

53-
return ( $key === 'nopaging' && ( $val === 'true' || $val === '1' ) );
62+
if ( $key === 'nopaging' ) {
63+
return ( $val === 'true' || $val === '1' );
64+
}
65+
66+
// posts_per_page / numberposts: flag -1 (no limit).
67+
$val = TextStrings::stripQuotes( $val );
68+
return ( $val === '-1' );
5469
}
5570
}
Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,72 @@
11
<?php
22

3+
// === nopaging tests ===
4+
35
$args = array(
46
'nopaging' => true, // Bad.
57
);
68

79
_query_posts( 'nopaging=true' ); // Bad.
810

9-
$query_args['my_posts_per_page'] = -1; // OK.
11+
$query_args['my_posts_per_page'] = -1; // OK - not a recognized key.
1012

1113
// Verify handling with no trailing comma at end of array.
1214
$args = array(
1315
'nopaging' => true // Bad.
1416
);
1517
$args = [ 'nopaging' => true ]; // Bad.
18+
19+
// nopaging with integer 1.
20+
$args = [ 'nopaging' => 1 ]; // Bad.
21+
22+
// nopaging with false - should not trigger.
23+
$args = [ 'nopaging' => false ]; // OK.
24+
25+
// nopaging with integer 0 - should not trigger.
26+
$args = [ 'nopaging' => 0 ]; // OK.
27+
28+
// nopaging via variable assignment.
29+
$args['nopaging'] = true; // Bad.
30+
31+
// nopaging via variable assignment with false.
32+
$args['nopaging'] = false; // OK.
33+
34+
// === posts_per_page tests ===
35+
36+
// posts_per_page set to -1 in long array syntax.
37+
$args = array(
38+
'posts_per_page' => -1, // Bad.
39+
);
40+
41+
// posts_per_page set to -1 in short array syntax.
42+
$args = [ 'posts_per_page' => -1 ]; // Bad.
43+
44+
// posts_per_page set to -1 as string.
45+
$args = [ 'posts_per_page' => '-1' ]; // Bad.
46+
47+
// posts_per_page set to a normal value - should not trigger.
48+
$args = [ 'posts_per_page' => 50 ]; // OK.
49+
50+
// posts_per_page set to 1 - should not trigger.
51+
$args = [ 'posts_per_page' => 1 ]; // OK.
52+
53+
// numberposts set to -1.
54+
$args = [ 'numberposts' => -1 ]; // Bad.
55+
56+
// numberposts set to a normal value - should not trigger.
57+
$args = [ 'numberposts' => 10 ]; // OK.
58+
59+
// posts_per_page via variable assignment.
60+
$args['posts_per_page'] = -1; // Bad.
61+
62+
// posts_per_page via variable assignment with normal value.
63+
$args['posts_per_page'] = 10; // OK.
64+
65+
// Query string with posts_per_page=-1.
66+
_query_posts( 'posts_per_page=-1&orderby=date' ); // Bad.
67+
68+
// Query string with posts_per_page set to a normal value.
69+
_query_posts( 'posts_per_page=50' ); // OK.
70+
71+
// Query string with numberposts=-1.
72+
_query_posts( 'numberposts=-1' ); // Bad.

WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.php

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,19 @@ class NoPagingUnitTest extends AbstractSniffUnitTest {
2525
*/
2626
public function getErrorList() {
2727
return [
28-
4 => 1,
29-
7 => 1,
30-
13 => 1,
28+
6 => 1,
29+
9 => 1,
3130
15 => 1,
31+
17 => 1,
32+
20 => 1,
33+
29 => 1,
34+
38 => 1,
35+
42 => 1,
36+
45 => 1,
37+
54 => 1,
38+
60 => 1,
39+
66 => 1,
40+
72 => 1,
3241
];
3342
}
3443

WordPressVIPMinimum/ruleset-test.inc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,9 @@ $args = array( // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.Un
463463
'nopaging' => true, // Error.
464464
);
465465
_query_posts( 'nopaging=true' ); // Error.
466+
$args = array( // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable
467+
'posts_per_page' => -1, // Error.
468+
);
466469

467470
// WordPressVIPMinimum.Performance.OrderByRand
468471
$args = array( // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable

WordPressVIPMinimum/ruleset-test.php

Lines changed: 39 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -156,15 +156,13 @@
156156
451 => 1,
157157
463 => 1,
158158
465 => 1,
159-
469 => 1,
160-
471 => 1,
161-
477 => 1,
162-
483 => 1,
163-
491 => 1,
164-
505 => 1,
165-
509 => 1,
166-
510 => 1,
167-
511 => 1,
159+
467 => 1,
160+
472 => 1,
161+
474 => 1,
162+
480 => 1,
163+
486 => 1,
164+
494 => 1,
165+
508 => 1,
168166
512 => 1,
169167
513 => 1,
170168
514 => 1,
@@ -173,29 +171,32 @@
173171
517 => 1,
174172
518 => 1,
175173
519 => 1,
176-
523 => 1,
177-
525 => 1,
178-
550 => 1,
179-
551 => 1,
174+
520 => 1,
175+
521 => 1,
176+
522 => 1,
177+
526 => 1,
178+
528 => 1,
179+
553 => 1,
180180
554 => 1,
181-
569 => 1,
182-
570 => 1,
181+
557 => 1,
182+
572 => 1,
183183
573 => 1,
184-
574 => 1,
185-
575 => 1,
184+
576 => 1,
185+
577 => 1,
186186
578 => 1,
187187
581 => 1,
188-
582 => 1,
189-
583 => 1,
190-
588 => 1,
191-
590 => 1,
192-
594 => 1,
193-
595 => 1,
194-
596 => 1,
188+
584 => 1,
189+
585 => 1,
190+
586 => 1,
191+
591 => 1,
192+
593 => 1,
195193
597 => 1,
196-
612 => 1,
197-
614 => 1,
198-
621 => 1,
194+
598 => 1,
195+
599 => 1,
196+
600 => 1,
197+
615 => 1,
198+
617 => 1,
199+
624 => 1,
199200
],
200201
'warnings' => [
201202
32 => 1,
@@ -274,21 +275,21 @@
274275
457 => 1,
275276
458 => 1,
276277
459 => 1,
277-
499 => 1,
278-
500 => 1,
279-
504 => 1,
280-
528 => 1,
281-
529 => 1,
282-
530 => 1,
278+
502 => 1,
279+
503 => 1,
280+
507 => 1,
283281
531 => 1,
284282
532 => 1,
283+
533 => 1,
284+
534 => 1,
285285
535 => 1,
286286
538 => 1,
287-
545 => 1,
288-
559 => 1,
289-
565 => 1,
290-
589 => 1,
291-
618 => 1,
287+
541 => 1,
288+
548 => 1,
289+
562 => 1,
290+
568 => 1,
291+
592 => 1,
292+
621 => 1,
292293
],
293294
'messages' => [
294295
130 => [

0 commit comments

Comments
 (0)