Flag posts_per_page value of -1

[rebeccahum]

What problem would the enhancement address for VIP?

On VIP, we strongly recommend not having no limit queries, as they are not scalable on larger sites: https://lobby.vip.wordpress.com/wordpress-com-documentation/code-review-what-we-look-for/#no-limit-queries

When we moved over the deprecated WPCS VIP sniffs over to VPCS here, the former PostsPerPageSniff on WPCS VIP became known as NoPagingSniff on VIPCS and the checks for posts_per_page were removed, as the WordPress.WP.PostsPerPage.posts_per_page_posts_per_page sniff was intended to be used instead. However, at the time, it wasn't realized that the WPCS PostsPerPage sniff did not check for values of -1.

Describe the solution you'd like

1) It would be nice to add this upstream onto WPCS's side: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/1638

2) But if not for 1), we should have a VIPCS sniff that flags this.

What code should be reported as a violation?

$args = [ 'posts_per_page' => -1 ];

What could should not be reported as a violation?

$args = [ 'posts_per_page' => 1 ];

Additional context

Also possibly related: https://github.com/Automattic/VIP-Coding-Standards/issues/29

[GaryJones]

After a brief discussion with @jrfnl, the approach may be to refactor the upstream sniff to allow for a range of values to check for, rather than a boolean property for checking against -1.

Regardless, no action is going to happen before 2.2.0 is released, so I'm punting to 2.3.0.

[rebeccahum]

After talking to @jrfnl, we'll wait for WPCS 3.0 to be released.

[rebeccahum]

Another code snippet I'd like to see flagged:

$query = get_posts( 
    array( 
      'post_type'   => 'module', 
      'post_status' => 'publish', 
      'post_parent' => 0,
      'numberposts' => -1, // Error
    ) 
  );

[GaryJones]

numberposts isn't used with WP_Query - see https://developer.wordpress.org/reference/classes/wp_query/#pagination-parameters which shows posts_per_page and nopaging.

numberposts is used with get_posts() - see https://developer.wordpress.org/reference/functions/get_posts/#parameters

get_comments() and get_pages() have a number argument, but the default is for all items to be returned, so folks can just leave the number out.

[rebeccahum]

Er, whoops, fixed. It should be flagged in a get_posts() context regardless.