Uncovering A WordPress Walker Class Inconsistency

Took an hour tonight to figure out why the new Store Locator Plus Premier Button Bar is not working properly.   The odd thing is this WAS working properly two weeks ago but both our own code for the base plugin and Premier add on changed as well as a new WordPress release.    Maybe something changed in WP Core to trigger the problem in our add on or maybe something we fixed elsewhere in the code exposed this weak spot.

The real issue here is undocumented parameters in a documented WP Core function and the use of a non-standard call to PHP’s func_get_args() to bypass a defined PHP class method’s parameters.   That’s not nice but as someone maintaining a legacy PHP app with tens-of-thousands of user, I get it.

Here is what I found , why it is “not nice” and how I fixed it.   Maybe it will help other plugin/theme devs with their custom Walker classes.

Use Case

Building the “button bar” UI element in the Store Locator Plus 4.9 Premier add on by leveraging the wp_terms_checklist() functionality with a custom Walker.

Our Custom Walk

class SLP_Premier_Category_Walker_Checklist extends Walker {

...

    /**
     * Walk the walk...
     *
     * @param array $elements
     * @param int $max_depth
     *
     * @return string|void
     */
    public function walk( $elements, $max_depth ) {
        global $slplus;
        if ( $slplus->SmartOptions->hide_empty->is_true) {
            foreach ( $elements as $k => $wp_term ) {
                if ( ! property_exists( $wp_term , 'location_count' ) ){
                    return;
                }
                if ( empty( $wp_term->location_count ) ) {
                    unset( $elements[ $k ] );
                }
            }
        }
        return parent::walk( $elements , $max_depth );
    }

...

}

Our Call To wp_terms_checklist

...

$HTML = $this->get_div_html( $has_label , wp_terms_checklist( 0, $this->get_taxonomy_query_args() ) );


...

    /**
     * Set the arguments needed for a taxonomy query.
     *
     * @return array
     */
    private function get_taxonomy_query_args() {
        return array(
            'taxonomy'              => SLPLUS::locationTaxonomy,
            'checked_ontop'         => false,
            'echo'                  => false,
            'walker'                => new SLP_Premier_Category_Walker_Checklist
        );      
    }

The Issue

Parameter mismatch between wp_terms_checklist() and the various Walker::walk methods.

WP Core \wp_terms_checklist

wp_terms_checklist allows for a second parameter that is an argument array.  It can include a custom taxonomy and a custom Walker to replace Walker_Category_Checklist (the default Walker class).

wp_terms_checklist() then uses PHP’s call_user_func_array() to invoke the defined Walker::walk method sending it THREE arguments ($categories, 0 , $args ).  

This is a potential issue as the default Walker_Category_Checklist extends Walker and falls back to the WP Core Walker::walk method which accepts TWO parameters defined as array() $elements and int $max_depth per the phpDocs and param list.

/**
 * Output an unordered list of checkbox input elements labelled with term names.
 *
 * Taxonomy-independent version of wp_category_checklist().
 *
 * @since 3.0.0
 * @since 4.4.0 Introduced the `$echo` argument.
 *
 * @param int          $post_id Optional. Post ID. Default 0.
 * @param array|string $args {
 *     Optional. Array or string of arguments for generating a terms checklist. Default empty array.
 *
 *     @type int    $descendants_and_self ID of the category to output along with its descendants.
 *                                        Default 0.
 *     @type array  $selected_cats        List of categories to mark as checked. Default false.
 *     @type array  $popular_cats         List of categories to receive the "popular-category" class.
 *                                        Default false.
 *     @type object $walker               Walker object to use to build the output.
 *                                        Default is a Walker_Category_Checklist instance.
 *     @type string $taxonomy             Taxonomy to generate the checklist for. Default 'category'.
 *     @type bool   $checked_ontop        Whether to move checked items out of the hierarchy and to
 *                                        the top of the list. Default true.
 *     @type bool   $echo                 Whether to echo the generated markup. False to return the markup instead
 *                                        of echoing it. Default true.
 * }
 */
function wp_terms_checklist( $post_id = 0, $args = array() ) {
...
    if ( empty( $r['walker'] ) || ! ( $r['walker'] instanceof Walker ) ) {
        $walker = new Walker_Category_Checklist;
    } else {
        $walker = $r['walker'];
    }
...
        // Put checked cats on top
        $output .= call_user_func_array( array( $walker, 'walk' ), array( $checked_categories, 0, $args ) );
    }
    // Then the rest of them
    $output .= call_user_func_array( array( $walker, 'walk' ), array( $categories, 0, $args ) );

    if ( $r['echo'] ) {
        echo $output;
    }

    return $output;
}

WP Core \Walker::walk

Here you can see the definition of the walk method.   It expects $elements to be an array and $max_depth to be  an int.  No other parameters are supported.

$args is set within to array_slice( func_get_args() , 2).

func_get_args() in this implementation returns an array representing what was actually passed versus the documented parameter list.   The array[0] = the element list, array[1] = the max_depth (0) with the other elements being empty.

 /**
     * Display array of elements hierarchically.
     *
     * Does not assume any existing order of elements.
     *
     * $max_depth = -1 means flatly display every element.
     * $max_depth = 0 means display all levels.
     * $max_depth > 0 specifies the number of display levels.
     *
     * @since 2.1.0
     *
     * @param array $elements  An array of elements.
     * @param int   $max_depth The maximum hierarchical depth.
     * @return string The hierarchical item output.
     */
    public function walk( $elements, $max_depth ) {
        $args = array_slice(func_get_args(), 2);
        $output = '';

        //invalid parameter or nothing to walk
        if ( $max_depth < -1 || empty( $elements ) ) {
            return $output;
        }
...

         return $output;
    }

Findings and Resolution

This is happening because the custom Walker uses the interface as defined and documented in WordPress Core without accounting for the non-standard implementation triggered by the use of func_get_args() to resolve the extra parameters.

The problem with NOT doing that is when you write a derived class, as I did with SLP_Premier_Category_Walker_Checklist you now have to define the derived walk method to mirror the parent (as it should) but then have to CALL the parent with a non-compliant call that passes the rogue “third parameter”.   This now relies on the base Walker class to maintain the undocumented func_get_args() method to extract the rogue argument and then pass it down the chain to other methods that support/require it such as the start_el() method in our class.

Yes, we could write a COMPLETE class that is 100% custom and separate from the base Walker class.  What does that gain us?  A lot of duplicate code AND the requirement to rewrite our custom class if the underlying wp_terms_checklist() and/or support WP Core Walker classes change.

What is the proper solution?   It could be quite complex without breaking backwards compatibility.  I’ve not searched the WP Core code yet to see what breaks if you add the proper third argument as an optional argument to the defined WP Core Walker::walk class and drop func_get_args().   It could get ugly.

My Plugin Resolution

Somehow this just feels “dirty” –  repeating the extra overhead of the base Walker’s use of func_get_args() we do the same in our derived class.  Ick.   But it gets the job done and we don’t have to repeat ourselves (or WordPress Core actually) by copying an entire class just to tweak two minor output parameters.

 /**
     * Walk the walk...
     *
     * @param array $elements
     * @param int $max_depth
     *
     * @return string
     */
    public function walk( $elements, $max_depth ) {
        global $slplus;
        if ( $slplus->SmartOptions->hide_empty->is_true) {
            foreach ( $elements as $k => $wp_term ) {
                if ( ! property_exists( $wp_term , 'location_count' ) ){
                    return;
                }
                if ( empty( $wp_term->location_count ) ) {
                    unset( $elements[ $k ] );
                }
            }
        }
        $the_real_args = func_get_args();
        return parent::walk( $elements, $max_depth , $the_real_args[2]  );
    }

One thought on “Uncovering A WordPress Walker Class Inconsistency

Leave a Reply

This site uses Akismet to reduce spam. Learn how your comment data is processed.