Should WP_Query::get_posts be a private method?

[rebasaurus]

Sometimes, users call WP_Query::get_posts() incorrectly, as it is a publicly accessible method:

<?php
$args = [
// do args here
];
$my_query = new WP_Query( $args );
$my_posts = $my_query->get_posts();

Unfortunately, this ends up doing a double query to the DB, as the WP_Query::get_posts() is mistaken for a getter when used outside the scope of the constructor. The ideal way to efficiently query would be:

<?php
$args = [
// do args here
];
$my_query = new WP_Query( $args );
$my_posts = $my_query->posts;

...Or in the case where no arguments are directly passed into a new WP_Query:

<?php
$args = [
// do args here
];
$my_query = new WP_Query;
$my_posts = $my_query->query( $args );

I just want to preface that I actually don't have the historical context behind why WP_Query::get_posts() remains a public method (whether it's left for backwards-compatibility reasons or whatnot).

@jrf suggests a potential approach to this in ​https://github.com/WordPress/WordPress-Coding-Standards/issues/1860#issuecomment-768296206:

it may be more appropriate to see if this can be solved in WP Core by checking if the $query is the same as previously used and if so, short-circuiting the function to return the previously retrieved results as saved in the $posts property.

Would love to see some discussion around this if it can get resolved within WP Core.

Thanks!

[oglekler]

This method cannot be made private because it will break a lot of themes and plugins actually leading to a disaster, but I believe that there is an easy fix — this method is not getting any arguments, so, there is no way that the result can be changed (apart from a possibility that before WP_Query() object creation and this method call some data will be changed by something else, but I think it should be fine not to count in most cases this method call is the next line after object initialization. And $posts property initially is equal to null, so, if $this->posts is an array, even an empty one, we can safely return this array on the spot without processing the query again.

But if setter is used in between object initialization and get_posts(), the outcome can be different, so it is getting complicated, but any change to the query_vars can raise a flag that change had happened and send get_posts to fully processing the query, but such cases should be in minority.

@costdev what do you think? 🙏