diff --git a/WordPress-VIP-Go/ruleset-test.inc b/WordPress-VIP-Go/ruleset-test.inc index c4ec4eeb..5a17d238 100644 --- a/WordPress-VIP-Go/ruleset-test.inc +++ b/WordPress-VIP-Go/ruleset-test.inc @@ -466,6 +466,9 @@ $args = array( 'nopaging' => true, // Error. ); _query_posts( 'nopaging=true' ); // Error. +$args = array( + 'posts_per_page' => -1, // Error. +); // WordPressVIPMinimum.Performance.OrderByRand $args = array( diff --git a/WordPress-VIP-Go/ruleset-test.php b/WordPress-VIP-Go/ruleset-test.php index 83cb5514..f7af1404 100644 --- a/WordPress-VIP-Go/ruleset-test.php +++ b/WordPress-VIP-Go/ruleset-test.php @@ -87,15 +87,13 @@ 462 => 1, 466 => 1, 468 => 1, - 472 => 1, - 474 => 1, - 480 => 1, - 486 => 1, - 494 => 1, - 507 => 1, - 511 => 1, - 512 => 1, - 513 => 1, + 470 => 1, + 475 => 1, + 477 => 1, + 483 => 1, + 489 => 1, + 497 => 1, + 510 => 1, 514 => 1, 515 => 1, 516 => 1, @@ -104,16 +102,19 @@ 519 => 1, 520 => 1, 521 => 1, - 525 => 1, - 527 => 1, - 545 => 1, - 560 => 1, - 564 => 1, - 565 => 1, - 566 => 1, + 522 => 1, + 523 => 1, + 524 => 1, + 528 => 1, + 530 => 1, + 548 => 1, + 563 => 1, 567 => 1, - 572 => 1, - 574 => 1, + 568 => 1, + 569 => 1, + 570 => 1, + 575 => 1, + 577 => 1, ], 'warnings' => [ 7 => 1, @@ -223,14 +224,14 @@ 454 => 1, 455 => 1, 456 => 1, - 502 => 1, - 503 => 1, - 530 => 1, + 505 => 1, + 506 => 1, 533 => 1, - 540 => 1, - 550 => 1, - 556 => 1, - 579 => 1, + 536 => 1, + 543 => 1, + 553 => 1, + 559 => 1, + 582 => 1, ], 'messages' => [ 7 => [ diff --git a/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php b/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php index 311951d4..d3814514 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php @@ -10,10 +10,11 @@ namespace WordPressVIPMinimum\Sniffs\Performance; +use PHPCSUtils\Utils\TextStrings; use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff; /** - * Flag returning high or infinite posts_per_page. + * Flag disabling pagination via `nopaging` or `posts_per_page`/`numberposts` set to `-1`. * * @link https://docs.wpvip.com/technical-references/code-review/#no-limit-queries */ @@ -28,11 +29,19 @@ public function getGroups() { return [ 'nopaging' => [ 'type' => 'error', - 'message' => 'Disabling pagination is prohibited in VIP context, do not set `%s` to `%s` ever.', + 'message' => 'Disabling pagination is prohibited in VIP context, do not set `%s` to `%s`.', 'keys' => [ 'nopaging', ], ], + 'posts_per_page' => [ + 'type' => 'error', + 'message' => 'Setting `%s` to `%s` disables pagination and is prohibited in VIP context.', + 'keys' => [ + 'posts_per_page', + 'numberposts', + ], + ], ]; } @@ -49,6 +58,12 @@ public function getGroups() { public function callback( $key, $val, $line, $group ) { $key = strtolower( $key ); - return ( $key === 'nopaging' && ( $val === 'true' || $val === '1' ) ); + if ( $key === 'nopaging' ) { + return ( $val === 'true' || $val === '1' ); + } + + // posts_per_page / numberposts: flag -1 (no limit). + $val = TextStrings::stripQuotes( $val ); + return ( $val === '-1' ); } } diff --git a/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.inc b/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.inc index 9af35243..8272e8e2 100644 --- a/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.inc @@ -1,15 +1,85 @@ true, // Bad. ); _query_posts( 'nopaging=true' ); // Bad. -$query_args['my_posts_per_page'] = -1; // OK. +$query_args['my_posts_per_page'] = -1; // OK - not a recognized key. // Verify handling with no trailing comma at end of array. $args = array( 'nopaging' => true // Bad. ); $args = [ 'nopaging' => true ]; // Bad. + +// nopaging with integer 1. +$args = [ 'nopaging' => 1 ]; // Bad. + +// nopaging with false - should not trigger. +$args = [ 'nopaging' => false ]; // OK. + +// nopaging with integer 0 - should not trigger. +$args = [ 'nopaging' => 0 ]; // OK. + +// nopaging via variable assignment. +$args['nopaging'] = true; // Bad. + +// nopaging via variable assignment with false. +$args['nopaging'] = false; // OK. + +// === posts_per_page tests === + +// posts_per_page set to -1 in long array syntax. +$args = array( + 'posts_per_page' => -1, // Bad. +); + +// posts_per_page set to -1 in short array syntax. +$args = [ 'posts_per_page' => -1 ]; // Bad. + +// posts_per_page set to -1 as string. +$args = [ 'posts_per_page' => '-1' ]; // Bad. + +// posts_per_page set to a normal value - should not trigger. +$args = [ 'posts_per_page' => 50 ]; // OK. + +// posts_per_page set to 1 - should not trigger. +$args = [ 'posts_per_page' => 1 ]; // OK. + +// numberposts set to -1. +$args = [ 'numberposts' => -1 ]; // Bad. + +// numberposts set to a normal value - should not trigger. +$args = [ 'numberposts' => 10 ]; // OK. + +// posts_per_page via variable assignment. +$args['posts_per_page'] = -1; // Bad. + +// posts_per_page via variable assignment with normal value. +$args['posts_per_page'] = 10; // OK. + +// Query string with posts_per_page=-1. +_query_posts( 'posts_per_page=-1&orderby=date' ); // Bad. + +// Query string with posts_per_page set to a normal value. +_query_posts( 'posts_per_page=50' ); // OK. + +// Query string with numberposts=-1. +_query_posts( 'numberposts=-1' ); // Bad. + +// === Ignoring with phpcs:ignore comments === + +// phpcs:ignore WordPressVIPMinimum.Performance.NoPaging.posts_per_page_posts_per_page -- Intentional: fetching all items for export. +$args = [ 'posts_per_page' => -1 ]; // OK - ignored. + +$args = [ + // phpcs:ignore WordPressVIPMinimum.Performance.NoPaging.posts_per_page_numberposts -- Intentional: legacy API requires all results. + 'numberposts' => -1, // OK - ignored. +]; + +// phpcs:ignore WordPressVIPMinimum.Performance.NoPaging.nopaging_nopaging -- Intentional: sitemap generation. +$args = [ 'nopaging' => true ]; // OK - ignored. diff --git a/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.php b/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.php index 514df012..89e9e267 100644 --- a/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.php +++ b/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.php @@ -23,10 +23,19 @@ class NoPagingUnitTest extends AbstractSniffUnitTest { */ public function getErrorList() { return [ - 4 => 1, - 7 => 1, - 13 => 1, + 6 => 1, + 9 => 1, 15 => 1, + 17 => 1, + 20 => 1, + 29 => 1, + 38 => 1, + 42 => 1, + 45 => 1, + 54 => 1, + 60 => 1, + 66 => 1, + 72 => 1, ]; } diff --git a/WordPressVIPMinimum/ruleset-test.inc b/WordPressVIPMinimum/ruleset-test.inc index 3a104038..b47ae03c 100644 --- a/WordPressVIPMinimum/ruleset-test.inc +++ b/WordPressVIPMinimum/ruleset-test.inc @@ -463,6 +463,9 @@ $args = array( // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.Un 'nopaging' => true, // Error. ); _query_posts( 'nopaging=true' ); // Error. +$args = array( // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable + 'posts_per_page' => -1, // Error. +); // WordPressVIPMinimum.Performance.OrderByRand $args = array( // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable diff --git a/WordPressVIPMinimum/ruleset-test.php b/WordPressVIPMinimum/ruleset-test.php index 4fc80840..4d31b841 100644 --- a/WordPressVIPMinimum/ruleset-test.php +++ b/WordPressVIPMinimum/ruleset-test.php @@ -155,15 +155,13 @@ 451 => 1, 463 => 1, 465 => 1, - 469 => 1, - 471 => 1, - 477 => 1, - 483 => 1, - 491 => 1, - 505 => 1, - 509 => 1, - 510 => 1, - 511 => 1, + 467 => 1, + 472 => 1, + 474 => 1, + 480 => 1, + 486 => 1, + 494 => 1, + 508 => 1, 512 => 1, 513 => 1, 514 => 1, @@ -172,29 +170,32 @@ 517 => 1, 518 => 1, 519 => 1, - 523 => 1, - 525 => 1, - 550 => 1, - 551 => 1, + 520 => 1, + 521 => 1, + 522 => 1, + 526 => 1, + 528 => 1, + 553 => 1, 554 => 1, - 569 => 1, - 570 => 1, + 557 => 1, + 572 => 1, 573 => 1, - 574 => 1, - 575 => 1, + 576 => 1, + 577 => 1, 578 => 1, 581 => 1, - 582 => 1, - 583 => 1, - 588 => 1, - 590 => 1, - 594 => 1, - 595 => 1, - 596 => 1, + 584 => 1, + 585 => 1, + 586 => 1, + 591 => 1, + 593 => 1, 597 => 1, - 612 => 1, - 614 => 1, - 621 => 1, + 598 => 1, + 599 => 1, + 600 => 1, + 615 => 1, + 617 => 1, + 624 => 1, ], 'warnings' => [ 32 => 1, @@ -273,21 +274,21 @@ 457 => 1, 458 => 1, 459 => 1, - 499 => 1, - 500 => 1, - 504 => 1, - 528 => 1, - 529 => 1, - 530 => 1, + 502 => 1, + 503 => 1, + 507 => 1, 531 => 1, 532 => 1, + 533 => 1, + 534 => 1, 535 => 1, 538 => 1, - 545 => 1, - 559 => 1, - 565 => 1, - 589 => 1, - 618 => 1, + 541 => 1, + 548 => 1, + 562 => 1, + 568 => 1, + 592 => 1, + 621 => 1, ], 'messages' => [ 130 => [