Added more ReturnTypeWillChange attributes

[thirsch]

During my work on making the coverage working again, I stumbled upon these. Will try to clean the codebase in small steps to make it compatible before activating the coverage within our ci builds.

[thePanz]

LGTM, but the Iterator interface (from PHP) should be respected/enforced here... what about adding the right return type to those methds?
It could be seen as a BC break, but it would be a PHP break if we return something different from the interface.

WDYT?

[thirsch]

LGTM, but the Iterator interface (from PHP) should be respected/enforced here... what about adding the right return type to those methds? It could be seen as a BC break, but it would be a PHP break if we return something different from the interface.

As long as we are supporting php 7.4, this might not be possible, as the php interfaces use mixed as a return type, which is only supported >= 8.0.

[thePanz]

As long as we are supporting php 7.4, this might not be possible, as the php interfaces use mixed as a return type

That would be only as a docblock of the interface, right? As PHP 7.4 does not support mixed as a native typehint IIRC.
I would be in for adding the right typehint here... do you think it will be too much a BC break? How could we add them when well support v8.x only?

[thirsch]

I would be in for adding the right typehint here... do you think it will be too much a BC break? How could we add them when well support v8.x only?

You mean changing e. g. @return Doctrine_Record to @return mixed? IMO this would only remove more precise information, as the interface allows mixed and we are just more specific with Doctrine_Record here, it is not wrong. With >= 8.0 we could add the correct return type function current(): mixed, keep the docblock comment @return Doctrine_Record and avoid the #[\ReturnTypeWillChange] attributes.

[thirsch]

At the moment, I wouldn't change these occurrences as we have introduced the attribute on other places as well. If dropping 7.4 one time, maybe we will come up with a true change of the return types? But we would complete the >= 8.1 support on one hand and we are able to go forward towards reactivating the existing coverage mechanism with this pr.

[connorhu]

Btw anyone know of a tool that updates the entire codebase with static analysis along a given sign/attribute (method will change notice, return type will change etc)? This is what I miss in php that we have in swift, that there is an automatic migration process in the code base when the programming language updates.

[thirsch]

Btw anyone know of a tool that updates the entire codebase with static analysis along a given sign/attribute (method will change notice, return type will change etc)? This is what I miss in php that we have in swift, that there is an automatic migration process in the code base when the programming language updates.

Does not really belong to this pr, but I'll answer here: Have you tried Rector?

[thirsch]

Thanks for merging it :)