"iterable" alias "array|Traversable" breaks PHP 8.1 code

[cziegenberg]

Description

I just recognized a problem in a project when testing it with PHP 8.2RC2 (API Platform 3.0).

There is an interface that contains a method with the return type definition "object|iterable|null". In PHP 8.2 "iterable" becomes "array|Traversable" and so the return type definition results in "object|array|Traversable|null", which now contains the redundant "Traversable" type (although "object" is already contained) and leads to a fatal error.

Eample...

The following code:

<?php
class Foo
{
    public function bar(): object|iterable|null
    {
        return null;
    }
}

echo 'baz';

Resulted in this output:

Fatal error: Type Traversable|object|array|null contains both object and a class type, which is redundant in [...

But I expected this output instead:

baz

As a workaround one could use "object|array" instead of "object|iterable", but best would be if the change in PHP 8.2 handles this special case, to avoid this break... Otherwise this should be listed in "Backward Incompatible Changes" instead of "Other Changes".

PHP Version

PHP 8.2.0RC2

Operating System

No response

[cmb69]

Not sure if this has been considered. @Girgias?

[Girgias]

Huh, I will see if I can do an exception for the compile time redundancy check but not sure that this is possible.
Pedantically this feels like a bug in the union type implementation which probably should have made object|iterable impossible similar to how array|iterable is considered redundant (see: https://3v4l.org/liB3K)

[Girgias]

So I've had a look, and the problem is that the redundancy check is one of the last ones and doesn't have the knowledge of what the class name is, just that there is at least one class name within the type union.

In the current state ttrying to allow object|iterable would necessarily imply allowing a redundant type such as object|iterable|T. And as said in my first reply, the initial implementation of union types might have had a bug in not detecting that object|iterable is in some sense redundant when it maybe should have.

I'll think a bit more, but by of the looks of it, it seems unlikely I can make this case an exception.

[cziegenberg]

I did some tests and it seems to be safe to replace object|iterable with object|array for PHP 8.2. The modification of the return type definition in existing interfaces etc. doesn't seem to break existing code (i.e. a modified interface return type object|array for PHP 8.2 can still be implemented with a return type object|iterable in PHP 8.1 etc.: https://3v4l.org/ShnOT#v8.1.10).

So it seems to be easy to fix when mentioned in the "Backward Incompatible Changes".

[Girgias]

I did some tests and it seems to be safe to replace object|iterable with object|array for PHP 8.2. The modification of the return type definition in existing interfaces etc. doesn't seem to break existing code (i.e. a modified interface return type object|array for PHP 8.2 can still be implemented with a return type object|iterable in PHP 8.1 etc.: https://3v4l.org/ShnOT#v8.1.10).

So it seems to be easy to fix when mentioned in the "Backward Incompatible Changes".

FYI object|array is identical to object|iterable even in PHP 8.0 and 8.1. :)

[cziegenberg]

Yes, that's what I expected, but I wanted to test it to be sure. :)

[guilliamxavier]

To be "sure", you should test not only return types (covariant) but also parameter types (contravariant): https://3v4l.org/NQLV5 confirms the equivalence between object|iterable and object|array 🙂
Same with typed properties (invariant): https://3v4l.org/mKQdm

[Girgias]

To be "sure", you should test not only return types (covariant) but also parameter types (contravariant): https://3v4l.org/NQLV5 confirms the equivalence between object|iterable and object|array slightly_smiling_face Same with typed properties (invariant): https://3v4l.org/mKQdm

Contravariance is done by doing a covariant check just with the types reversed, so testing just one is fine, added an invariant test in #9558