fix: fix doctrine collection with union/intersection type

[mdeboer]

This PR fixes an issue introduced in v2.3.2 where hydrating doctrine collections with a union or intersection type failed. These types were not recognized as doctrine collections and therefore they were assigned as array instead of ArrayCollection.

[nikophil]

Hi @mdeboer

thanks for this fix!

could you please add a non regression test, please? Don't hesitate if you need some hints about the testsuite

[mdeboer]

Hi @mdeboer

thanks for this fix!

could you please add a non regression test, please? Don't hesitate if you need some hints about the testsuite

@nikophil I am fairly new with the project itself and I see that the Hydrator class is not covered by any tests at all (as far as I can tell).

I don't have that much time to spend unfortunately, but I can add a few basic unit tests to that cover my changes?

[nikophil]

could you please provide an example of failing code here please? so I can understand the problem better, I'll write the test myself

[mdeboer]

Yeah I'm writing some basic unit tests for the Hydrator as the moment, they will clarify I think.

But for instance, hydrating this would work in v2.3.1 but fails in v2.3.2+:

<?php

use Doctrine\Common\Collections\Collection;
use Doctrine\Common\Collections\Selectable;

class Foo {
    public Collection&Selectable $foo;

    __construct() {
        $this->foo = new ArrayCollection();
    }
}

This is because Foo::$foo is a union type here. In v2.3.2+ the hydrator fails to see Foo::$foo as a doctrine collection as it only checks if the type of that property is a \ReflectionNamedType, which is not true, because it is a \ReflectionUnionType.

[nikophil]

ok thanks! If you have some time to write a unit test for the hydrator, that's super nice!

I'll create an integration test anyway.

it only checks if the type of that property is a \ReflectionNamedType, which is not true, because it is a \ReflectionUnionType.

yeah that's totally true. I wish I could use component type info for this, this would have prevented this I guess

[mdeboer]

@nikophil I have added some unit tests for the Hydrator that should prevent regressions. Hopefully they help explain what the problem was.

[nikophil]

Pretty nice! could you rebase on 2.3.x please? so I can release this quickly

Also, I might make some changes on your PR before merging

[mdeboer]

@nikophil I already based it on 2.3.x, from what I see it is up to date 😅 Feel free to make any changes you like!

[nikophil]

hi @mdeboer

your fix is released https://github.com/zenstruck/foundry/releases/tag/v2.3.8

[mdeboer]

Thank you so much! Appreciate the quick response 😁 Keep up the good work!