Introduce new PHP cross-version compat function `is_iterable()`

[jrf]

Similar to and related to #43583

PHP 7.2 introduced a warning when count() is used on something non-countable.

PHP 7.1 introduced a new function called is_iterable() which basically checks:

if (is_array($foo) || $foo instanceof Traversable) {
    // $foo is traversable
}

See: ​http://php.net/manual/en/function.is-iterable.php

When count() is used to check if an array is traverable, i.e. usable in a foreach(){}, it would be better to replace the count() check with a call to is_iterable().

To that end, I would like to suggest adding this new function to the wp-includes/compat.php file, like so:

if ( ! function_exists( 'is_iterable' ) ) :
        function is_iterable( $var ) {
                return ( ( is_array( $var ) && ! empty( $var ) || $var instanceof Traversable );
        }
}
endif;

This seems like low hanging fruit. Both the instanceof operator as well as the Traversable interface were already available in PHP 5.2.4, so there should be no cross-version compatibility issues with introducing this function.

The function would provide an easy helper function to help fix the various issues where the warning is currently being reported - both for Core as well as for plugin/theme devs -.

Existing code like this:

if ( count( $var ) > 0 ) {
        foreach( $var as $key => $value) {
                // Do something.
        }
}

can then be replaced by:

if ( is_iterable( $var ) ) {
        foreach( $var as $key => $value) {
                // Do something.
        }
}

The function would also still allow for code refactoring from arrays to objects. /cc @schlessera

I would suggest introducing this function ASAP and not to wait for a major release.

Related issues (there are probably more, but a quick search yielded these):
#43583, #43374, #43368, #43312, #43216, #43201, #43216, #43201, #42860, #42814, #42147, #42498

[schlessera]

Related to this & #43583:
Considering the growing need to add PHP polyfills and other compat code to Core (due to the fact that PHP has a higher velocity than WordPress), we should look into using a more systematic approach to compatibility code come 5.0, like described in #42264

[jrf]

@schlessera I hadn't seen that ticket, thanks for pointing it out. I totally agree with the approach.
All the same, the need to make backfills load in a more modular manner is independent of the need for the functionality proposed here to be added.

[schlessera]

@jrf Yes, of course, the functionality proposed here can and should already go in with a next minor release. I just thought your proposed polyfills demonstrate the growing need for a more systematic approach, that's why I linked.

Using functions like the above makes the code more future-proof and less error-prone, so there's no reason not to add this small polyfill.

[schlessera]

Polyfill implementation with basic unit tests

[schlessera]

I added a patch with a polyfill implementation and some basic unit tests that are taken from the PHP source (except for the Generator test): ​https://github.com/php/php-src/blob/6053987bc27e8dede37f437193a5cad448f99bce/ext/standard/tests/general_functions/is_iterable.phpt

[desrosj]

43619.2.diff​ changes the @since version to 4.9.6 and adds @ticket tags to the unit tests. Everything looks good to go for this one.

[jrf]

I think we still need one more change here - mea culpa.

When I created the ticket I did something (not so) "clever". I added && ! empty( $var ) to the conditional in the function

<?php
return ( ( is_array( $var ) && ! empty( $var ) || $var instanceof Traversable );

However, the PHP native function does not do such a check, so as it stands, there will be situations where the WP back-fill does not give the same result as the PHP native function which is undesirable and will be the cause of bugs in the long run.

Sorry about that.

The latest patch contains the && ! empty( $var ), so it will need to be removed and if no unit tests are failing after that, we need to add a unit test for an empty array.

[desrosj]

Thanks for pointing that out, @jrf. I updated the patch and added a test for an empty array in 43619.3.diff​.

Did you have a plan in mind for updating Core to use this new function? There are several tickets referenced above, but wondering if making a singular ticket (or handling that on this ticket) is better. Same for #43583.

Since these produce warnings, I think these should be fixed in 4.9.6, if possible.

[SergeyBiryukov]

In 43036:

General: Introduce a polyfill for is_iterable() function added in PHP 7.1.

Props jrf, schlessera, desrosj.
See #43619.

[SergeyBiryukov]

In 43037:

General: Introduce a polyfill for is_iterable() function added in PHP 7.1.

Props jrf, schlessera, desrosj.
Merges [43036] to the 4.9 branch.
See #43619.

[SergeyBiryukov]

Let's create a new ticket for using is_iterable() throughout core where appropriate.

[desrosj]

This and #43583 would be great to mention in a dev note.

[johnjamesjacoby]

Motion to reopen for future discussion.

Should we backport those to earlier WordPress versions?

I can think of plugins that would like to be PHP7.3 ready without the need to bump their minimum WordPress version to 4.9.6. (I know earlier WP versions fixed a few issues, and I’d argue we should backport those also.)

Without backporting these in core, plugins need to implement their own back-fills until they’re ready to bump their minimum version, defeating the purpose of the core back-fills.

[desrosj]

Dev note: ​https://make.wordpress.org/core/2018/05/17/new-php-polyfills-in-4-9-6/.