Fix missing ref array in pre_get_users

[andy]

Code docs in class-wp-user-query.php mark this as passed by reference but the actual call uses do_action instead of do_action_ref_array.

Index: wp-includes/class-wp-user-query.php
===================================================================
--- wp-includes/class-wp-user-query.php	(revision 212073)
+++ wp-includes/class-wp-user-query.php	(working copy)
@@ -227,7 +227,7 @@
 		 * @param WP_User_Query $this The current WP_User_Query instance,
 		 *                            passed by reference.
 		 */
-		do_action( 'pre_get_users', $this );
+		do_action_ref_array( 'pre_get_users', array( &$this ) );
 
 		// Ensure that query vars are filled after 'pre_get_users'.
 		$qv =& $this->query_vars;

[SergeyBiryukov]

Just noting this also applies to the pre_get_terms hook.

[spacedmonkey]

@adamsilverstein

[adamsilverstein]

Thanks for the ping @spacedmonkey.

Trac ticket:

[adamsilverstein]

@andy I noticed we have ​tests for this and also found extensive plugin use where the original $this is modified.

I believe this works because PHP passes objects by reference, however changing to do_action_ref_array would certainly be easier to read code-wise. I'll give that change a test to make sure tests still work as expected.

[adamsilverstein]

Running test suite with proposed change in ​https://github.com/WordPress/wordpress-develop/pull/478

[andy]

Hi Adam! Thanks for taking me back to school. :)

I went back and revisited the reason for the patch. The issue was that I used &$query in the action function's signature and got a warning about getting a value when a reference was expected. In fact, the reference to the object was working. I just needed to remove the & to stop the warning.

[SergeyBiryukov]

I believe the change should still be made, not for functional difference but just for consistency with the other actions:

• do_action_ref_array( 'pre_get_comments', array( &$this ) );
• do_action_ref_array( 'pre_get_networks', array( &$this ) );
• do_action_ref_array( 'pre_get_posts', array( &$this ) );
• do_action_ref_array( 'pre_get_sites', array( &$this ) );
• do_action_ref_array( 'pre_user_query', array( &$this ) );

50961.diff​ updates both pre_get_terms and pre_get_users to match.

[adamsilverstein]

I believe the change should still be made, not for functional difference but just for consistency with the other actions

Good point. 50961.diff​ looks good to me, I'll run the tests in travis to confirm.

[hellofromTonya]

Restarted ​failing Travis jobs. Passing now.

Note: PHP8 job fails. If merged, need to add this to php8 tasks.

[adamsilverstein]

@SergeyBiryukov - reviewing 50961.diff​ - does changing $this to array( &$this ) change anything in terms of filter consumers use? I guess if all the tests pass we are fine!

Trac ticket: https://core.trac.wordpress.org/ticket/50961

[desrosj]

Looks like the user query instance was introduced in [29363] (4.4), and the term query one in [37572] (4.6)

[hellofromTonya]

Notes from yesterday's scrub.

@adamsilverstein noted to this question: Any risks for it being merged this late into 5.6?

I don't think so, on the other hand this is a purely internal change, so we could easily punt to 5.7 (not a priority)

[SergeyBiryukov]

Replying to hellofromTonya:

Note: PHP8 job fails. If merged, need to add this to php8 tasks.

Just noting the ​PHP 8 failure is unrelated to the patch:

Warning: Class "PHPUnit\Util\Getopt" not found in /var/www/tests/phpunit/includes/phpunit6/compat.php on line 18
Error: Looks like you're using PHPUnit 9.4.2. WordPress requires at least PHPUnit 5.4 and is currently only compatible with PHPUnit up to 7.x.
Please use the latest PHPUnit version from the 7.x branch.

It looks like a wrong PHPUnit version was used for some reason. That does not happen on WordPress trunk.

The patch should not have any functional difference, committing shortly.

[SergeyBiryukov]

In 49637:

Users: Use do_action_ref_array() for pre_get_users and pre_get_terms actions.

This brings some consistency with the other similar actions:

• pre_get_comments
• pre_get_networks
• pre_get_posts
• pre_get_sites
• pre_user_query

Follow-up to [29363] and [37572].

Props andy, adamsilverstein, hellofromTonya, desrosj, SergeyBiryukov.
Fixes #50961.

Merged in changeset https://core.trac.wordpress.org/changeset/49637

Merged in changeset https://core.trac.wordpress.org/changeset/49637