Inconsistency of is_iterable() and isIterable Reflection class method with what foreach actually expects

[stalkonr]

Description

What I did?

Basically I have json_decode object, which contains key value configuration (which comes from API), like this

  $var = [
	  'key1' => 'value1',
	  'key2' => 'value2',
	  'key3' => 'value3'
  ];
  
  $object = json_decode(json_encode($var));

and I want to iterate over those key values with foreach:

foreach ($object as $key => $value) {
	echo $key;
}

which works fine. However, I want to guard this foreach from iterating on improper values. That's why I used is_iterable() (https://www.php.net/manual/en/function.is-iterable.php), In PHP RFC: Iterable (https://wiki.php.net/rfc/iterable) we can read:

It is common for a function to accept or return either an array or an object implementing Traversable to be used with foreach. However, because array is a primitive type and Traversable is an interface, there currently is no way to use a type declaration on a parameter or return type to indicate that the value is iterable.

What I wanted to happen?

I wanted that is_iterable would guard my foreach from every wrong value, like false, null and would allow object like above. See example:

  $var = [
	  'key1' => 'value1',
	  'key2' => 'value2',
	  'key3' => 'value3'
  ];
  
  $object = json_decode(json_encode($var));
  
  if (is_iterable($object) {
    foreach ($object as $key => $value) {
	echo $key;
    }
  }

What actually happened?

It turned that above object passed to is_iterable() returns false, which suprise me because it can be perfectlly fined passed to foreach, which has this warning if you pass something expected:

foreach() argument must be of type array|object, %s given

Summary

My PR propose to fix this inconsistency (or at least in my opinion it's inconsistency).

Please take a look and let me know what do you think, thanks!

Best regards,
stalkonr

PHP Version

8.1

Operating System

Ubuntu 20.04

[DeveloperRob]

The reason for this behaviour is explained in the RFC you linked to: https://wiki.php.net/rfc/iterable#object_iteration - while foreach will accept any object, for an object to be considered iterable it must implement Traversable. The StdClass object returned by json_decode doesn't implement any interfaces (https://www.php.net/manual/en/class.stdclass.php).

For your use-case, you can use $object instanceof \StdClass which will perform the check you need that the json_decode returned an object (https://3v4l.org/FeVuA)

[Girgias]

Any object is iterable via foreach (c.f. https://www.php.net/manual/en/language.oop5.iterations.php) and it will loop through all visible properties in the current scope: e.g. https://3v4l.org/X5E2l

This is in some sense a "legacy" behaviour of considering objects as a different way of having a set of key-value pairs instead of arrays.

What you are proposing is a massive semantic change and would need to be discussed on the internals mailing list and need an RFC.

[bwoebi]

I found it funny that the Traversable docs state bluntly:

Interface to detect if a class is traversable using foreach.

Like it's implying ... given that every class can be traversed using for foreach ... every class implements Traversable? Which isn't the case at least :-D

[mvorisek]

IMHO foreach ($object as $key => $value) { should be disallowed when not iterable (not an array nor implements Traversable).

[stalkonr]

Thank you all for taking your time on this thread (@DeveloperRob @Girgias @bwoebi @mvorisek ). That's my first attempt to try to contribute something to PHP core.

---

@DeveloperRob I know that it can be workaround, however isn't it a little bit confusing? In Traversable definition we have:

Interface to detect if a class is traversable using foreach.

[source: https://www.php.net/manual/en/class.traversable.php]

stdClass is traversable using foreach, however doesn't implement this interface. What is more confusing for me that is_iterable() checks implementation of traversable interface. In that case, why it isn't called is_traversable()?

---

@Girgias You are right, now when I'm thinking about my PR I see that it will break some code using is_iterable() (or change behaviour)

What you are proposing is a massive semantic change and would need to be discussed on the internals mailing list and need an RFC.

I will research how to do it, and try to start discussion there.

---

Like it's implying ... given that every class can be traversed using for foreach ... every class implements Traversable? Which isn't the case at least :-D

@bwoebi Exactly, it's confusing IMHO

---

IMHO foreach ($object as $key => $value) { should be disallowed when not iterable (not implements Traversable).

@mvorisek that would be consistent and is something that I would expect. Or at least to improve documentation?

---

Now I'm think that maybe it will be safer to have seperate function, like is_foreachable()? (I know, awful name, however it is simple to know what intention of this funciton is). In this function we would do the same checks as currently during iteration:

EXPECTED(Z_TYPE_P(array_ptr) == IS_ARRAY)
|| EXPECTED(Z_TYPE_P(array_ptr) == IS_OBJECT)

[source: https://raw.githubusercontent.com/php/php-src/master/Zend/zend_vm_execute.h:6653]

So that would give consistent behavior? What do you think @mvorisek @bwoebi @Girgias @DeveloperRob ?

[Girgias]

The problem is that one can uses objects in multiple different ways. Which is part of PHP's inherent problem, they can be used as a properly named Key:Value set (i.e. a Value Object) or more complex behaviour that carries state around.

In other languages, one would have a proper distinction between these two usages (e.g. structs and classes in C#), however PHP does not. Traversable is there to indicate that a complex object which carries state around can be iterated.

In your use case (decoding stuff from JSON), the objects are always going to be Value Objects and thus are a proper Key:Value set where iterating through the visible properties make sense.

I don't think adding a is_foreachable() is at all useful, as it's basically (is_array($v) || is_object($v)) and removes the required thought about "is iterating over this arbitrary object sensible".

Do the current semantics suck? Yes, but that's more an issue of objects being overused for completely different semantics because this is the only thing PHP provides.

[iluuu1994]

@stalkonr

That's my first attempt to try to contribute something to PHP core.

Welcome 😊

You are right, now when I'm thinking about my PR I see that it will break some code using is_iterable() (or change behaviour)

That's exactly right. The terminology is indeed confusing here. I think any change here would require discussion in the internals list at the very least and probably an RFC. The documentation can absolutely improved, so I'm moving this issue.