wp_list_filter should filter all iterable objects

[jarednova]

#16499 introduced an issue which requires that data sent to wp_list_filter is an array. What it really means is the data sent to wp_list_filter should be iterable _like_ an array. By permitting both arrays and objects that are Transverable we can allow for anything that decends from the \ArrayObject class.

While there is an is_iterable method we _could_ use, it's only availabile in PHP 7.1 so the attached is friendly to PHP v4.2 and above

[jarednova]

Proposed patch

[jarednova]

Test for patch

[jarednova]

@swissspidy I added a test in 43025.test.diff — please let me know if you need anything else to make your review as easy as possible!

[swissspidy]

@flixos90 Since you worked on WP_List_Util, would you mind having a look?

I think we can add such a check directly to the class instead of the helper function.

Also, I'd use instanceof instead of is_a(). We rarely use the latter in core and it's better to be consistent.

[flixos90]

Some thoughts from my end:

• I agree with @swissspidy that the is_array() clauses in the beginning of wp_list_filter() (and wp_list_sort() and wp_filter_object_list() as well!) should be removed from there and become part of WP_List_Util. The easiest way would be to check in the constructor whether $input is an array of traversable, and if not, simply set it to an empty array.
• I agree that instanceof should be used instead of is_a(). Also recommended since it's a language construct and not a function.
• Not sure whether that's necessary, but maybe we should adjust all related docblocks to say "iterable" instead of array.
• And now it gets a little more complex: WP_List_Util::filter() and WP_List_Util::sort() create a new array with the data from the input. Not sure whether we should just keep it that way or try to return the same object that was passed (that would probably make sense). If yes, how could we achieve this reasonably? Add a private utility method that transforms the output array back to the same type of traversable object if the input is one? Should we use clone and then populate the cloned object with the new values?
• WP_List_Util::sort() uses usort() and uasort() both of which don't work with a traversable object. Maybe we really need to transform the traversable into an array immediately and transform it back after the operations.

IMO this is a little more complex than initially thought.

[jarednova]

Reaction to feedback on patch

[jarednova]

@flixos90 I resolved some of the points you raised in 43025.2.diff...

I agree with @swissspidy that the is_array() clauses in the beginning of wp_list_filter() (and wp_list_sort() and wp_filter_object_list() as well!) should be removed from there and become part of WP_List_Util. The easiest way would be to check in the constructor whether $input is an array of traversable, and if not, simply set it to an empty array.

I created a function called WP_List_Util::maybe_list() to take an input and determine whether it's a valid iterable object. If it's not, it just sends back an empty array. I took a stab at the naming here — would love your thoughts on the name and whether this matches with what you were looking for.

---

I agree that instanceof should be used instead of is_a(). Also recommended since it's a language construct and not a function.

Done!

---

Not sure whether that's necessary, but maybe we should adjust all related docblocks to say "iterable" instead of array.
And now it gets a little more complex: WP_List_Util::filter() and WP_List_Util::sort() create a new array with the data from the input. Not sure whether we should just keep it that way or try to return the same object that was passed (that would probably make sense). If yes, how could we achieve this reasonably? Add a private utility method that transforms the output array back to the same type of traversable object if the input is one? Should we use clone and then populate the cloned object with the new values?
WP_List_Util::sort() uses usort() and uasort() both of which don't work with a traversable object. Maybe we really need to transform the traversable into an array immediately and transform it back after the operations.

Like you said, this "more complex" stuff I left for now