This isn't gonna work if you have union of constant arrays. Please test that scenario + fix it with TypeUtils::getOldConstantArrays().

I wasn't sure how you wanted that I use getOldConstantArrays.

I handled UnionType in a different way and added a test. Tell me if this is ok this way.

It's not OK, please do not use any instanceof *Type. Use TypeUtils::getOldConstantArrays() and iterate over the array of types.

I tried a solution with getOldConstantArrays now.
But this seems more complicated to me.

I suspect this if/else is no longer necessary.

I just tried, and if I only keep the if part, tests crash, and if I only keep the else part, test fails.

Can you explain how you want the code changed @ondrejmirtes ? Because I didn't succeed having my test green as soon as I touch this if/else.

Any code that does instanceof *Type is suspicious to me. Just try deleting this piece of code and see what tests fail. And try fixing them in another way.

Recently we've been adding specific methods on Type. It also means that if you rebase on top of 1.9.x, some methods you're currently using will show up as deprecated.

Do i need to retarget to 1.9.x ?

Sorry, this isn't how I imagine and manage quality software development. The fix needs to make sense and is easier to do once the knowledge of the specific area is already in your headspace.

I'll be more than happy to find another way to write this code but I'll need more help then. Because I didn't understood everything during this discussion. You said "Recently we've been adding specific methods on Type" and "we need a specific method for min/max" so do you want me to add a new method to types ? What should it does ?

@herndlm can help answering that but usually you check recent merged PR history and also the above discussion to get the idea.

I saw Pr like #1869 which introduced a new method.

My main concern was about the method to introduce.
I thought about introducing a getTypes method to all the Type, in order to write:

foreach ($iterableValueType->getTypes() as $innerType) {
    $argumentTypes[] = $innerType;
}

But i'm not sure you will like this solution.

You said "we need a specific method for min/max" but at first sight that would mean reproducing/duplicating part of the logic from MinMaxFunctionReturnTypeExtension::processType/compareTypes in every types.

sorry, I didn't fully dive into this yet, but it sounds like you can add a Type::max(Types ...$types): Type and min methods. moving it to type often could mean slight duplications, but the core logic for arrays should then be split into ArrayType and ConstantArrayType and we automatically add support for unions and e.g. don't need Type::getConstantArrays() & co anymore. The other types are mostly covered via the array traits and we need to think explicitly how the array accessory types like HasOffsetType should behave. it is possible then to simply add code for them as well via the new methods.

I didn't think through yet how you can solve the iterableValueType problem that seems to be here. because it sounds like it would just be moved somewhere else. but you have then max and min methods on type, so maybe that helps somehow. since max and min don't only handle arrays, it means most likely that other types need to implement too somehow, I have no clue yet how, so I'm not sure if that helps much here. Maybe you also need minArray() and maxArray() methods on type and want to keep the rest here. again, I didn't dive into this, sorry

how you can solve the iterableValueType problem

The previous logic was

• If it's an Array
  • if the IterableValueType is an UnionType, iterate over the types.
  • else take the iterableValueType like this.

The logic I neeeded to implements in the ReturnTypeExtension is

• If it's a ConstantArray
  • if the IterableValueType is an UnionType, iterate over the types.
  • else take the iterableValueType like this.
• If it's just an Array
  • take the iterableValueType like this.

I just discovered in case it's a ConstantArray, it could just be replaced by the getValueTypes method !

Finally! This is what I meant that you don't need UnionType here anymore. I'm happy to merge this if the build is green.