Skip to content

Commit 67c8068

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 b19c94b commit 67c8068

File tree

7 files changed

+172
-70
lines changed

7 files changed

+172
-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
@@ -87,15 +87,13 @@
8787
462 => 1,
8888
466 => 1,
8989
468 => 1,
90-
472 => 1,
91-
474 => 1,
92-
480 => 1,
93-
486 => 1,
94-
494 => 1,
95-
507 => 1,
96-
511 => 1,
97-
512 => 1,
98-
513 => 1,
90+
470 => 1,
91+
475 => 1,
92+
477 => 1,
93+
483 => 1,
94+
489 => 1,
95+
497 => 1,
96+
510 => 1,
9997
514 => 1,
10098
515 => 1,
10199
516 => 1,
@@ -104,16 +102,19 @@
104102
519 => 1,
105103
520 => 1,
106104
521 => 1,
107-
525 => 1,
108-
527 => 1,
109-
545 => 1,
110-
560 => 1,
111-
564 => 1,
112-
565 => 1,
113-
566 => 1,
105+
522 => 1,
106+
523 => 1,
107+
524 => 1,
108+
528 => 1,
109+
530 => 1,
110+
548 => 1,
111+
563 => 1,
114112
567 => 1,
115-
572 => 1,
116-
574 => 1,
113+
568 => 1,
114+
569 => 1,
115+
570 => 1,
116+
575 => 1,
117+
577 => 1,
117118
],
118119
'warnings' => [
119120
7 => 1,
@@ -223,14 +224,14 @@
223224
454 => 1,
224225
455 => 1,
225226
456 => 1,
226-
502 => 1,
227-
503 => 1,
228-
530 => 1,
227+
505 => 1,
228+
506 => 1,
229229
533 => 1,
230-
540 => 1,
231-
550 => 1,
232-
556 => 1,
233-
579 => 1,
230+
536 => 1,
231+
543 => 1,
232+
553 => 1,
233+
559 => 1,
234+
582 => 1,
234235
],
235236
'messages' => [
236237
7 => [

WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php

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

1111
namespace WordPressVIPMinimum\Sniffs\Performance;
1212

13+
use PHPCSUtils\Utils\TextStrings;
1314
use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff;
1415

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

@@ -49,6 +58,12 @@ public function getGroups() {
4958
public function callback( $key, $val, $line, $group ) {
5059
$key = strtolower( $key );
5160

52-
return ( $key === 'nopaging' && ( $val === 'true' || $val === '1' ) );
61+
if ( $key === 'nopaging' ) {
62+
return ( $val === 'true' || $val === '1' );
63+
}
64+
65+
// posts_per_page / numberposts: flag -1 (no limit).
66+
$val = TextStrings::stripQuotes( $val );
67+
return ( $val === '-1' );
5368
}
5469
}
Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,85 @@
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.
73+
74+
// === Ignoring with phpcs:ignore comments ===
75+
76+
// phpcs:ignore WordPressVIPMinimum.Performance.NoPaging.posts_per_page_posts_per_page -- Intentional: fetching all items for export.
77+
$args = [ 'posts_per_page' => -1 ]; // OK - ignored.
78+
79+
$args = [
80+
// phpcs:ignore WordPressVIPMinimum.Performance.NoPaging.posts_per_page_numberposts -- Intentional: legacy API requires all results.
81+
'numberposts' => -1, // OK - ignored.
82+
];
83+
84+
// phpcs:ignore WordPressVIPMinimum.Performance.NoPaging.nopaging_nopaging -- Intentional: sitemap generation.
85+
$args = [ 'nopaging' => true ]; // OK - ignored.

WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.php

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

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
@@ -155,15 +155,13 @@
155155
451 => 1,
156156
463 => 1,
157157
465 => 1,
158-
469 => 1,
159-
471 => 1,
160-
477 => 1,
161-
483 => 1,
162-
491 => 1,
163-
505 => 1,
164-
509 => 1,
165-
510 => 1,
166-
511 => 1,
158+
467 => 1,
159+
472 => 1,
160+
474 => 1,
161+
480 => 1,
162+
486 => 1,
163+
494 => 1,
164+
508 => 1,
167165
512 => 1,
168166
513 => 1,
169167
514 => 1,
@@ -172,29 +170,32 @@
172170
517 => 1,
173171
518 => 1,
174172
519 => 1,
175-
523 => 1,
176-
525 => 1,
177-
550 => 1,
178-
551 => 1,
173+
520 => 1,
174+
521 => 1,
175+
522 => 1,
176+
526 => 1,
177+
528 => 1,
178+
553 => 1,
179179
554 => 1,
180-
569 => 1,
181-
570 => 1,
180+
557 => 1,
181+
572 => 1,
182182
573 => 1,
183-
574 => 1,
184-
575 => 1,
183+
576 => 1,
184+
577 => 1,
185185
578 => 1,
186186
581 => 1,
187-
582 => 1,
188-
583 => 1,
189-
588 => 1,
190-
590 => 1,
191-
594 => 1,
192-
595 => 1,
193-
596 => 1,
187+
584 => 1,
188+
585 => 1,
189+
586 => 1,
190+
591 => 1,
191+
593 => 1,
194192
597 => 1,
195-
612 => 1,
196-
614 => 1,
197-
621 => 1,
193+
598 => 1,
194+
599 => 1,
195+
600 => 1,
196+
615 => 1,
197+
617 => 1,
198+
624 => 1,
198199
],
199200
'warnings' => [
200201
32 => 1,
@@ -273,21 +274,21 @@
273274
457 => 1,
274275
458 => 1,
275276
459 => 1,
276-
499 => 1,
277-
500 => 1,
278-
504 => 1,
279-
528 => 1,
280-
529 => 1,
281-
530 => 1,
277+
502 => 1,
278+
503 => 1,
279+
507 => 1,
282280
531 => 1,
283281
532 => 1,
282+
533 => 1,
283+
534 => 1,
284284
535 => 1,
285285
538 => 1,
286-
545 => 1,
287-
559 => 1,
288-
565 => 1,
289-
589 => 1,
290-
618 => 1,
286+
541 => 1,
287+
548 => 1,
288+
562 => 1,
289+
568 => 1,
290+
592 => 1,
291+
621 => 1,
291292
],
292293
'messages' => [
293294
130 => [

0 commit comments

Comments
 (0)