WordPress.org

Make WordPress Core

Opened 9 days ago

Last modified 2 days ago

#47678 assigned enhancement

Modernize/simplify current_user_can()

Reported by: jrf Owned by: pento
Milestone: 5.3 Priority: normal
Severity: normal Version: trunk
Component: Role/Capability Keywords: has-patch has-unit-tests commit needs-dev-note
Focuses: performance, coding-standards Cc:

Description

As support for PHP < 5.6.20 has been dropped, certain modernizations can now be applied to WP.

The `current_user_can()` function is used pretty often during a regular WP page-load and is a good candidate for modernization.

The patch I'm attaching contains this modernization.

To understand what I'm doing here, let's step through it.

Step 1: simplify the function.

The function contains the following redundant code:

<?php
        $args = array_slice( func_get_args(), 1 );
        $args = array_merge( array( $capability ), $args );

What is happening here is that:

  1. All arguments passed to the function are retrieved.
  2. The first argument - the named parameter $capability - is sliced off, so $args is now only the arguments after $capability.
  3. The arguments are merged back together in the same order as they were originally.

I've not done the code archeology to see how this code ever made into the code base as I'm not here to name and shame, but let's get rid of it.

Replacing this part of the code with the below has the exact same effect and removes two (badly performing) function calls:

<?php
        $args = func_get_args();

To prove the point, see: https://3v4l.org/BqYY0

Step 2: modernize the function declaration.

The function currently declares one named, required parameter $capability and via the above mentioned func_get_args() allows for additional arguments to be passed.
This is also documented as such in the function documentation.

<?php
/**
 * ...
 * @param string $capability Capability name.
 * @param mixed  ...$args    Optional further parameters, typically starting with an object ID.
 * @return bool Whether the current user has the given capability. If `$capability` is a meta cap and `$object_id` is
 *              passed, whether the current user has the given meta capability for the given object.
 */
function current_user_can( $capability ) {

PHP 5.6 introduced variadic functions using the spread operator.

Now, we could make $capability variadic and remove the call to func_get_args(), but that would make the $capability parameter optional.

<?php
function current_user_can( ...$capability ) {

Making the $capability parameter optional is not the intention as it:

  • Changes the function signature.
  • Could break the subsequent function call to WP_User::has_cap() to which $capability is passed, where $capability is not optional.

So, instead, we add a new variadic (and therefore by nature optional) parameter $args.

<?php
function current_user_can( $capability, ...$args ) {

This is completely in line with the existing function documentation, so no changes are needed to the docs.

As things stand at this moment, we would still be overwritting $args in the function via the call to func_get_args(), but that's ok for now, we'll get to that in step 3.

Step 3: modernize the return.

Now, let's look at the part of the function which is returning the result:

<?php
        $args = func_get_args();

        return call_user_func_array( array( $current_user, 'has_cap' ), $args );

call_user_func_array() is used here as $args would contain a variable number of arguments and prior to PHP 5.6, there was no other way to pass those on to another function which expected multiple arguments.

However, PHP 5.6 also introduced argument unpacking using the spread operator.

Now suddenly, the call to the badly performing call_user_func_array() is no longer necessary and we can call $current_user->has_cap() directly, unpacking the variadic $args in the function call:

<?php
        return $current_user->has_cap( $capability, ...$args );

So, that's how we get to the actual patch.

All together, this patch gets rid of four redundant function calls, three of which were calls to functions with a bad reputation for performance.

So, even though small, this patch should give WP a very tiny performance boost.

Unit tests

There are numerous tests in the `Tests_User_Capabilities` class covering this change already.

All of these tests still pass after this change.

And once I've uploaded this patch, I will start a Travis run just to make sure and quieten any nerves anyone may have about this patch.

/cc @pento

Attachments (34)

patch-47678.patch (1.2 KB) - added by jrf 9 days ago.
47678-map_meta_cap.patch (880 bytes) - added by jrf 9 days ago.
Use spread operator in map_meta_cap()
47678-current_user_can_for_blog.patch (1.4 KB) - added by jrf 9 days ago.
Simplify & modernize current_user_can_for_blog()
47678-author_can.patch (1.2 KB) - added by jrf 9 days ago.
Simplify & modernize author_can()
47678-user_can.patch (1.2 KB) - added by jrf 9 days ago.
Simplify & modernize user_can()
47678-WP_User-has_cap.patch (1.6 KB) - added by jrf 9 days ago.
Simplify & modernize WP_User::has_cap()
47678-Walker-walk.patch (1.0 KB) - added by jrf 9 days ago.
Use spread operator in Walker::walk()
47678-Walker-paged_walk.patch (1.3 KB) - added by jrf 9 days ago.
Use spread operator in Walker::paged_walk()
47678-add_post_type_support.patch (1.2 KB) - added by jrf 9 days ago.
Simplify & modernize add_post_type_support()
47678-walk_page_dropdown_tree.patch (1.0 KB) - added by jrf 9 days ago.
Simplify & modernize walk_page_dropdown_tree()
47678-add_theme_support.patch (1002 bytes) - added by jrf 9 days ago.
Simplify & modernize add_theme_support()
47678-get_theme_support.patch (1.1 KB) - added by jrf 9 days ago.
Simplify & modernize get_theme_support()
47678-current_theme_supports.patch (1.2 KB) - added by jrf 9 days ago.
Simplify & modernize current_theme_supports()
47678-wp_register_sidebar_widget.patch (1.3 KB) - added by jrf 9 days ago.
Simplify & modernize wp_register_sidebar_widget()
47678-wp_register_widget_control.patch (1.4 KB) - added by jrf 9 days ago.
Simplify & modernize wp_register_widget_control()
47678-_register_widget_update_callback.patch (1.4 KB) - added by jrf 9 days ago.
Simplify & modernize _register_widget_update_callback()
47678-_register_widget_form_callback.patch (1.3 KB) - added by jrf 9 days ago.
Simplify & modernize _register_widget_form_callback()
47678-wpdb-prepare.patch (3.8 KB) - added by jrf 9 days ago.
Simplify & modernize wpdb::prepare(). Includes reformatting the documentation for line length/readibility.
47678-walk_category_tree.patch (1.5 KB) - added by jrf 2 days ago.
Simplify & modernize walk_category_tree() Includes minor documentation fixes.
47678-walk_category_dropdown_tree.patch (1.5 KB) - added by jrf 2 days ago.
Simplify & modernize walk_category_dropdown_tree() Includes minor documentation fixes.
47678-_WP_Dependency-__construct.patch (1.0 KB) - added by jrf 2 days ago.
Use spread operator in _WP_Dependency::construct()
47678-_Upgrader_Skin-feedback.patch (4.2 KB) - added by jrf 2 days ago.
Simplify & modernize *_Upgrader_Skin::feedback()
47678-WP_Ajax_Upgrader_Skin-error.patch (1.6 KB) - added by jrf 2 days ago.
Simplify & modernize WP_Ajax_Upgrader_Skin::error()
47678-do_action.patch (1.5 KB) - added by jrf 2 days ago.
Simplify & modernize do_action()
47678-add_query_args.patch (942 bytes) - added by jrf 2 days ago.
Use spread operator in add_query_args()
47678- code-simplification-apply_filters.patch (1.2 KB) - added by jrf 2 days ago.
Code simplification apply_filters()
47678-unit-test-util-xml_find.patch (1.7 KB) - added by jrf 2 days ago.
Simplify & modernize unit test util xml_find()
47678-some-unit-test-functions.patch (4.1 KB) - added by jrf 2 days ago.
Simplify & modernize some unit test functions
47678-wp_sprintf.patch (1.5 KB) - added by jrf 2 days ago.
Simplify & modernize wp_sprintf()
47678-select-functions-in-wp-includes-deprecated.patch (1.3 KB) - added by jrf 2 days ago.
Simplify & modernize functions in wp-includes/deprecated.php While these functions are deprecated, they can still get a minor performance boost in case they are being called.
47678-functions-in-tools-i18n-not-gettext.patch (1.3 KB) - added by jrf 2 days ago.
Simplify & modernize functions in tools/i18n/not-gettexted.php
47678-wp_dashboard_cached_rss_widget.patch (1.5 KB) - added by jrf 31 minutes ago.
Simplify & modernize wp_dashboard_cached_rss_widget()
47678-wp_iframe.patch (1.3 KB) - added by jrf 27 minutes ago.
Simplify & modernize wp_iframe()
47678-wp_iframe.2.patch (10 bytes) - added by jrf 25 minutes ago.
Ignore/remove

Download all attachments as: .zip

Change History (64)

@jrf
9 days ago

#2 @pento
9 days ago

  • Keywords commit added; 2nd-opinion removed
  • Milestone changed from Awaiting Review to 5.3
  • Owner set to jrf
  • Status changed from new to assigned

Interestingly, this has existed since nearly the beginning of time.

I suspect that it originated as a copy/pasta bug when the feature was being written: the similar behaviour in WP_User::has_cap() was copied and modified to suit the desired behaviour of current_user_can(). Unfortunately, I haven't been able to locate any relevant discussion around it, so it seems it just slipped under the radar at the time.

Due to the giant "here be dragons" signs all over the roles/capabilities code, it's not really surprising that redundant code like this has quietly existed the entire time, given that it works.

I'm cool with both removing the redundant code, and introducing the spread operator here.

#3 @jrf
9 days ago

👍

This ticket is basically a proof of concept and the same principle can be applied to a lot more functions in WP and in the capabilities.php file in particular.

Should I open separate tickets for each of those patches ? or can I add additional patches, at least for the Capabilities related functions, to this ticket ?

@jrf
9 days ago

Use spread operator in map_meta_cap()

@jrf
9 days ago

Simplify & modernize current_user_can_for_blog()

@jrf
9 days ago

Simplify & modernize author_can()

@jrf
9 days ago

Simplify & modernize user_can()

#4 @jrf
9 days ago

I've added (and will continue to add) a number of additional patches to this ticket which each apply the same principle as applied in the original ticket to other functions.

Each patch is named after the WP function it addresses.

@jrf
9 days ago

Simplify & modernize WP_User::has_cap()

@jrf
9 days ago

Use spread operator in Walker::walk()

@jrf
9 days ago

Use spread operator in Walker::paged_walk()

@jrf
9 days ago

Simplify & modernize add_post_type_support()

@jrf
9 days ago

Simplify & modernize walk_page_dropdown_tree()

@jrf
9 days ago

Simplify & modernize add_theme_support()

@jrf
9 days ago

Simplify & modernize get_theme_support()

@jrf
9 days ago

Simplify & modernize current_theme_supports()

@jrf
9 days ago

Simplify & modernize wp_register_sidebar_widget()

@jrf
9 days ago

Simplify & modernize wp_register_widget_control()

@jrf
9 days ago

Simplify & modernize _register_widget_update_callback()

@jrf
9 days ago

Simplify & modernize _register_widget_form_callback()

@jrf
9 days ago

Simplify & modernize wpdb::prepare(). Includes reformatting the documentation for line length/readibility.

#5 @knutsp
9 days ago

Nice!

array() to [] and it will be perfect.

#6 @swissspidy
9 days ago

Short array syntax is probably best introduced in one swoop in a separate ticket. It doesn't give any advantage over the long syntax anyway. Let's focus on using the spread operator here, which I think makes for a great improvement already!

#7 @jrf
9 days ago

FYI: Including all patches attached to this ticket, the build is still passing: https://travis-ci.com/WordPress/wordpress-develop/builds/118756852

array() to [] and it will be perfect.

@knutsp Short array syntax is "sugar". It makes no functional difference, has no performance benefits and depending on whom you speak to, decreases code readability.

I'm with @swissspidy here. Let's keep this ticket focused on the spread operator which with the implementations in the patches I've submitted so far does actually have performance benefits and gets rid of a lot of unnecessary function call overhead.

Whether short array syntax should be introduced or even allowed, is a whole other discussion.
Also see the existing discussion about this here: https://make.wordpress.org/core/2019/03/26/coding-standards-updates-for-php-5-6/

#8 @Hareesh Pillai
8 days ago

  • Focuses performance added
  • Type changed from defect (bug) to enhancement

#9 @pento
8 days ago

  • Owner changed from jrf to pento

I'm going to work through these patches and get them committed. 🙂

#10 @pento
8 days ago

In 45622:

Code Modernisation: Introduce the spread operator in capabilities.php.

Rather than relying func_get_args() to retrieve arbitrary function arguments, we can now use the spread operator to assign them directly to a variable.

Props jrf.
See #47678.

#11 @pento
8 days ago

In 45623:

Code Modernisation: Introduce the spread operator in WP_User.

Rather than relying func_get_args() to retrieve arbitrary function arguments, we can now use the spread operator to assign them directly to a variable.

Props jrf.
See #47678.

#12 @pento
8 days ago

In 45624:

Code Modernisation: Introduce the spread operator in Walker.

Rather than relying func_get_args() to retrieve arbitrary function arguments, we can now use the spread operator to assign them directly to a variable.

Props jrf.
See #47678.

#13 @pento
8 days ago

In 45625:

Code Modernisation: Introduce the spread operator in add_post_type_support().

Rather than relying func_get_args() to retrieve arbitrary function arguments, we can now use the spread operator to assign them directly to a variable.

Props jrf, pento.
See #47678.

#14 follow-up: @pento
8 days ago

Note: [45625] is slightly different from 47678-add_post_type_support.patch: we don't need to use func_num_args(), as $args() will be set to an empty array if no bonus arguments are passed.

#15 @pento
8 days ago

In 45627:

Code Modernisation: Introduce the spread operator in walk_page_dropdown_tree().

Rather than relying func_get_args() to retrieve arbitrary function arguments, we can now use the spread operator to assign them directly to a variable.

Props jrf.
See #47678.

#16 @pento
8 days ago

In 45628:

Code Modernisation: Introduce the spread operator in theme.php.

Rather than relying func_get_args() to retrieve arbitrary function arguments, we can now use the spread operator to assign them directly to a variable.

Props jrf, pento.
See #47678.

#17 @pento
8 days ago

In 45629:

Code Modernisation: Introduce the spread operator in widgets.php.

Rather than relying func_get_args() to retrieve arbitrary function arguments, we can now use the spread operator to assign them directly to a variable.

Props jrf.
See #47678.

#18 @pento
8 days ago

In 45630:

Code Modernisation: Introduce the spread operator in wpdb::prepare().

Rather than relying func_get_args() to retrieve arbitrary function arguments, we can now use the spread operator to assign them directly to a variable.

Props jrf.
See #47678.

#19 in reply to: ↑ 14 @jrf
8 days ago

@pento Thanks for all the commits so far! I'll try and add the next set of patches tomorrow (my timezone).

Replying to pento:

Note: [45625] is slightly different from 47678-add_post_type_support.patch: we don't need to use func_num_args(), as $args() will be set to an empty array if no bonus arguments are passed.

True enough, though using if ( $args ) isn't actually showing what you are testing for, if ( ! empty( $args ) ) would have been my preference in that case, even if just to prevent the next person scratching their head trying to figure out what's happening ;-)

#20 follow-up: @pento
8 days ago

Technically it's only testing for if ( 0 !== count( $args ) ) ? 😉

I'm okay with the assumed knowledge that the spread operator will ensure that $args will always be an array (which is a little new to the WordPress codebase), so a falsey test of $args is checking if that array is empty or not (which is a very common pattern in the WordPress codebase).

#21 in reply to: ↑ 20 @jrf
8 days ago

Replying to pento:

Technically it's only testing for if ( 0 !== count( $args ) ) ? 😉

I'm okay with the assumed knowledge that the spread operator will ensure that $args will always be an array (which is a little new to the WordPress codebase), so a falsey test of $args is checking if that array is empty or not (which is a very common pattern in the WordPress codebase).

Well, you know me, I'm just not a fan of implicit non-descript loose comparisons ;-)
I've had to debug way too many issues with that: https://phpcheatsheets.com/test/general/

#22 follow-up: @pento
8 days ago

So, we have a bit of a problem, because PHP doesn't do method overloading.

Changing the method signature of Walker::walk() caused an explosion of PHP warnings on WordPress.org when I deployed this change. The code still works as expected, it's just a sub-optimal experience.

Example: https://3v4l.org/2iS9g

#23 in reply to: ↑ 22 ; follow-up: @jrf
8 days ago

Replying to pento:

So, we have a bit of a problem, because PHP doesn't do method overloading.

Changing the method signature of Walker::walk() caused an explosion of PHP warnings on WordPress.org when I deployed this change. The code still works as expected, it's just a sub-optimal experience.

Example: https://3v4l.org/2iS9g

Well, PHP does do method overloading, just expects as a minimum the same (number of) arguments.

There are two choices here:

  1. Revert that particular change.
  2. Update the method signature for the walk() and paged_walk() methods in child classes.

I'm very much in favour of 2. Yes, WP is notorious for its backward-compatibility, but it's not as if other methods have never received new arguments if a functional change warranted this or am I mistaken ?

And while this may not be considered a "functional change", all this does is formalize existing behaviour of the methods which child-methods should already comply with.

In reality the optional variadic additional arguments to these methods are not new, they just hadn't been made explicit in the method signature. In most cases, they already were explicit in the documentation, showing that there was full awareness that this was only not (yet) formalized because of the limitations in PHP before 5.6.

I'm very happy & willing to prepare the patch for [2], which should effectively solve this for Core, though to be honest a quick search did not yield me any classes in Core which overloaded those particular methods (so far).

As for userland code which may extend the Walker class:

  1. I have no clue how often this happens. Clearly something on wp.org does. @Ipstenu would you be able & willing to see how often extends [A-Za-z_]*Walker occurs in plugin-land code ?
  2. A dev-note on Make could alert devs to this early so they can update their code accordingly, though this would "force" userland plugins/themes to drop support for PHP < 5.6 as well.

Note: function calls to these methods should not be affected, it is only classes which extend the Walker class (or any of the other touched classes) and overload the particular method which has changed.

Last edited 6 days ago by jrf (previous) (diff)

#24 in reply to: ↑ 23 ; follow-up: @pento
5 days ago

Replying to jrf:

There are two choices here:

  1. Revert that particular change.
  2. Update the method signature for the walk() and paged_walk() methods in child classes.

I'm very much in favour of 2. Yes, WP is notorious for its backward-compatibility, but it's not as if other methods have never received new arguments if a functional change warranted this or am I mistaken ?

New arguments are added to methods extremely rarely, and offset against the effort required for plugins to release a compatibility fix. The last instance I recall was WP_Customize_Manager::__construct() gaining a parameter in WP 4.7.

There are ~260 instances of extend Walker here, but presumably more in themes, custom plugins, etc.

I think this change could still work, in combination with a dev-note, and an easy pattern for plugin authors to maintain back compat with old versions of WordPress.

For now, I'm going to revert [45624] until we have a plan laid out.

#25 @pento
5 days ago

In 45640:

Code Modernisation: Revert [45624].

Changing the method signatures on Walker causes back compat issues.

See #47678.

#26 in reply to: ↑ 24 @jrf
4 days ago

Replying to pento:

There are ~260 instances of extend Walker here, but presumably more in themes, custom plugins, etc.

I've reviewed the first 100 results and only two of these overload the walk() method.

All the others just overload the start_el(), start_lvl(), end_el() and/or end_lvl() methods, just like the WP Core native child classes do.

A little more specific search yields 26 results out of which 16 are still false positives or tagged version duplicates and only 10 actually 1) extend one of the Walker classes and 2) overload the walk() method.
Those 10 files are in the following 8 plugins:

  • Polylang (2x)
  • Thoughful Comments (1x)
  • Another WordPress Classifieds Plugin (1x)
  • BuddyPress (1x)
  • Extended Categories Widget (2x)
  • Page Menu Widget (1x)
  • WP Sitemanager (1x)
  • Custom Menu Wizard (1x)

So, while it does happen, it is extremely rare (8 plugins out of 80.000+).

I think this change could still work, in combination with a dev-note, and an easy pattern for plugin authors to maintain back compat with old versions of WordPress.

Maintaining back-compat with older versions of WP is not a problem. Overloaded methods are perfectly okay to have more parameters than the parent method. See: https://3v4l.org/P1PaI

The only issue would be that they wouldn't be able to still support older PHP versions. They would have to update their minimum supported PHP version to 5.6+ to use the spread operator.

And while we've now just looked at Walker::walk(), the same principle applies to the other public methods where the signature changed. Though I expect that, for any of those to be overloaded, would be even more rare than the overloading of Walker::walk().

Changed WP Core classes:

  • WP:User::has_cap()
  • Walker::walk() (reverted in [45640])
  • Walker::paged_walk()
  • wpdb::prepare()

And in the WP unit test classes:

  • WP_UnitTestCase_Base::assertQueryTrue()
  • MockAction::filterall()
  • Tests_WP_Customize_Manager::capture_customize_post_value_set_actions()
  • Tests_WP_Hook_Do_Action::_action_callback()

And the tools class:

  • NotGettexted::logmsg()

#27 follow-up: @peterwilsoncc
4 days ago

I've found WPdirectory does a slightly better job of finding code, searching extends [A-Za-z_]*Walker returns the following:

As custom walkers are more common in themes, I'm concerned about breaking BC as themes are less likely to be open-source than plugins. Most sites run a custom theme.

Custom walkers are required for relatively common features, for example adding data attributes and IDs to Walker_Nav_Menu > ul for use with generic JavaScript libraries.

#28 in reply to: ↑ 27 ; follow-up: @jrf
4 days ago

Replying to peterwilsoncc:

I've found WPdirectory does a slightly better job of finding code, searching extends [A-Za-z_]*Walker returns the following:

As custom walkers are more common in themes, I'm concerned about breaking BC as themes are less likely to be open-source than plugins. Most sites run a custom theme.

Custom walkers are required for relatively common features, for example adding data attributes and IDs to Walker_Nav_Menu > ul for use with generic JavaScript libraries.

Hi Peter, nice! I hadn't thought of searching that way.

I appreciate your concerns and take them seriously, so I've done some more research.

I've reviewed the code of the themes with > 5000 installs based on the links you posted, but all of those are false positives, in that - similar to my previous findings - they just overload the start_el(), start_lvl(), end_el() and/or end_lvl() methods, not the walk() or paged_walk() methods.

So, let's look at getting a little more accurate search results and use this regex function (?:paged_)?walk\s*\(\s*\$.

The results are very different then:

  • Themes: It says 11 matches, but is showing only 4 themes, 1 of which is a false positive (not in a class extending a *Walker class)
  • Plugins: It says 277 matches, but is showing only 102 plugins, with ~50% having 0 installs and most of the ones which do have installs, including ACF, being false positives, i.e. in a class not extending a Walker class, but, typically a stand-alone RWMB_Walker_Select_Tree class or a class extending acf_field. In reality only 20 of the plugins with more than 0 installs extends a *Walker class.

So 3 out 5.000+ themes and some 20 plugins out of 80.000+ in the repo.

Of course, this excludes the commercial plugins and themes, as well as private plugins and themes, but all the same, I do think it's safe to assume that the numbers will be roughly the same, which means that less than 0.05% of plugins and themes will run into this issue.

So, while I appreciate that we should be very careful about this, I think that we may be overestimating the impact a bit.

Last edited 4 days ago by jrf (previous) (diff)

#29 in reply to: ↑ 28 @peterwilsoncc
4 days ago

  • Keywords needs-dev-note added

Replying to jrf:

Of course, this excludes the commercial plugins and themes, as well as private plugins and themes, but all the same, I do think it's safe to assume that the numbers will be roughly the same, which means that less than 0.05% of plugins and themes will run into this issue.

So, while I appreciate that we should be very careful about this, I think that we may be overestimating the impact a bit.

Agreed, the common use case is for *_lvl() and *_el() modifications. @pento's suggestion of a dev-note ought to be adequate.

@jrf
2 days ago

Simplify & modernize walk_category_tree() Includes minor documentation fixes.

@jrf
2 days ago

Simplify & modernize walk_category_dropdown_tree() Includes minor documentation fixes.

@jrf
2 days ago

Use spread operator in _WP_Dependency::construct()

@jrf
2 days ago

Simplify & modernize *_Upgrader_Skin::feedback()

@jrf
2 days ago

Simplify & modernize WP_Ajax_Upgrader_Skin::error()

@jrf
2 days ago

Simplify & modernize do_action()

@jrf
2 days ago

Use spread operator in add_query_args()

@jrf
2 days ago

Code simplification apply_filters()

@jrf
2 days ago

Simplify & modernize unit test util xml_find()

@jrf
2 days ago

Simplify & modernize some unit test functions

@jrf
2 days ago

Simplify & modernize wp_sprintf()

@jrf
2 days ago

Simplify & modernize functions in wp-includes/deprecated.php While these functions are deprecated, they can still get a minor performance boost in case they are being called.

@jrf
2 days ago

Simplify & modernize functions in tools/i18n/not-gettexted.php

#30 @jrf
2 days ago

I've just added another set of patches to this ticket.
Passing build which includes all the patches in this ticket which haven't been committed yet: https://travis-ci.com/WordPress/wordpress-develop/builds/119576361

Notes for select patches in the current set

walk_category_tree() and walk_category_dropdown_tree()

Both these functions referred in the documentation to a walk() method in one of the Walker child classes, but as the child classes in WP Core don't overload the walk() method, that link in the documentation resulted in a 404.

I've changed those links now to Walker:walk() so they will resolve properly in the documentation.

Refs:

do_action()

I'm removing the complete code block quoted below.

This is PHP 4 code. In PHP 4, objects were not automatically passed by reference which is the only explanation I can come up for the current way of setting up the $args array.
As of PHP 5, objects are always passed by reference, so what was being done here has not been needed for quite some time.

<?php
        $args = array();
        if ( is_array( $arg ) && 1 == count( $arg ) && isset( $arg[0] ) && is_object( $arg[0] ) ) { // array(&$this)
                $args[] =& $arg[0];
        } else {
                $args[] = $arg;
        }
        for ( $a = 2, $num = func_num_args(); $a < $num; $a++ ) {
                $args[] = func_get_arg( $a );
        }

apply_filters()

Removing the call to func_get_args() would not have any benefits here, but we can remove the duplicate function call.

Review Status

These are the last patches for the review of all func_get_args() calls in the codebase.
For the remaining calls to func_get_args() I didn't see any real benefit in making this change for various reasons.

Based on a review of the function calls to call_user_func_array(), I expect to add yet some more patches to this ticket.

Commit Status

The following patches have not been committed yet:

  • Walker::walk() and Walker::paged_walk() - see discussion above.
  • wp_register_sidebar_widget()
  • The new patches just added.

@jrf
31 minutes ago

Simplify & modernize wp_dashboard_cached_rss_widget()

@jrf
27 minutes ago

Simplify & modernize wp_iframe()

@jrf
25 minutes ago

Ignore/remove

Note: See TracTickets for help on using tickets.