fix: IterableFieldTrait:current() should be compatible with Iterator:…

[Niellles]

…:current()

As of PHP 8.1 methods return types should be compatible with their parent/interface. I.e. deprecration as of 8.1 enforced as of 9.0. IterableFieldTrait::current() should therefore return mixed, the same as Iterator::current(). However this is not possible to do without dropping support for PHP < 8.0, as mixed was introduced then.

We fix this by creating different versions of IterableFieldTrait based on the PHP version; defining current() with(out) a return type based on the (major) version of PHP and/or the availability of mixed.

phpstan: flip logic so that phpstan loads correct trait.

---

This fix is not a nice fix, nor is it elegant. However, it is a fix.. and that's something.

It aims to fix the issue first raised in #3395 by @enrise-mbraam. TLDR; Anything implementing the Iterator interface should have mixed as the return type of current(). However, adding that return type, would break the code for PHP < 8, as this is when mixed was first introduced.

Unfortunately this PR is the only viable solution that I see, short of dropping support for PHP < 8.

This work around should be removed when minimum PHP version is bumped to 8.0. Which should/will be done in bolt/bolt#7969 (couldn't find the moved issue).

Marking this as WIP, as we should discuss/explore more elegant solutions.

[bobdenotter]

Hi @Niellles, Thanks for looking into this. I agree that the current proposed fix is less than optimal.

Anything implementing the Iterator interface should have mixed as the return type of current(). However, adding that return type, would break the code for PHP < 8, as this is when mixed was first introduced.

Well, and this is why people think PHP is a mess. ;-)

I'm not certain if we can/should fix this. We'd fix it to appease phpstan, but the "offending" code will still be there. Is it really fixed, then?
If we simply add a line for PHPStan to ignore it, the solution will be just as hackish, but it will make more sense if somebody looks at the code in 6 months or a year, when it's time to drop PHP7 (when we move to SF6). 🤔

[Niellles]

@bobdenotter, I'd have to disagree on:

I'm not certain if we can/should fix this. We'd fix it to appease phpstan, but the "offending" code will still be there. Is it really fixed, then?

We'd also prevent anyone/any environment that hasn't disabled deprecation logging from going crazy because of logs being flooded with notices.

---

Anyway, after giving it some more thought I came op with the correct solution that's ready to be merged in #3405. It's the obvious solution, that I somehow ignored when I first started to look into this; I assumed that using #[\ReturnTypeWillChange] wouldn't work, because attributes were only added in 8.0. However, as we all know (and I somehow seem to have temporarily forgotten), these lines would be just handled as comments prior to the addition of attributes.

This also seems to be working in PHP 8.0 where this specific attribute doesn't exist. Probably due to how attributes work internally and the fact that this methods attributes are never actually used/parsed.