[PHPStan] improve `FactoryCollection` generics annotation with `template-type`

[edglev]

Looks like there is an issue with phpstan type definition, specifically &Proxy<T> part breaking extension chain resolving to Factory<T>.

I don't understand why T&Proxy<T> is a not a subtype of object.
Is this a problem in PhpStan itself?

Minimal reproducer https://phpstan.org/r/eece4718-641c-4e5b-bac9-e4f1430f8fda

Just as a test - removing &Proxy<T> part from PersistentProxyObjectFactory makes PhpStan happy.

https://phpstan.org/r/dc467615-ce04-4f6c-8370-b6be346ec8c0

[nikophil]

fixed 😄

https://phpstan.org/r/441da82d-53cc-40ee-8b4e-4ff82b7988b6

- @param FactoryCollection<Something, SomethingFactory> $somethingFactories
+ @param FactoryCollection<Something&Proxy<Something>, SomethingFactory> $somethingFactories

those FactoryCollection with proxies are kinda hard to deal with. We're open to any good idea to simplify this :)

By the way, the only use case to type hint with FactoryCollection is when dealing with data provider, but since Foundry 2.2 and PHPUnit 11.4, you can directly call YouFactory::createMany() in your data provider. you'll need to use the phpunit extension provided by Foundry. see https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html#phpunit-data-providers

[edglev]

Thank you ❤️ The use case for type hints is with data providers.

[edglev]

I've might have just found a way to simplify this - you can interpret the type from TFactory, example taken from here phpstan/phpstan#9053

https://phpstan.org/r/7a4e6c92-1d39-4470-860f-890d47aee32c

What do you think?

[nikophil]

this is very cool! I didn't even know template-type exists!

one thing I'd like, is that the current way of documenting factories still works. We've already introduced a BC break around this few months ago, and I would not want to introduce another one now...

do you think both fashions of documenting FactoryCollection could be compatible?

[edglev]

I don't think so, best I can come up right now is to just leave the signature as is and ignore the T https://phpstan.org/r/d7745a05-7be3-4425-b94f-6de8fb773d41

Tried a few ideas using Template default types from phpstan/phpstan#4801 but couldn't get any to work for both one and two template types specified. The hurdle here seems to be that the T to be removed is the first parameter. 😞

[nikophil]

ok then, I'm sorry, but we're not going to integrate this now in Foundry

BUT this could be integrated to Foundry 3, that might land at the end of the year (as for Symfony 8.0)

BTW, there's some chances that we offer a rector set to migrate to Foundry 3, the same way we did for migration from 1 to 2

[nikophil]

@edglev I'll keep this open 👍

if at some point, you're willing to provide a PR with the changes with template-type and make the CI green that would be very nice, and we'll merge it in a future 3.x branch (but nothing hurry, this is planned for next November I think)

[edglev]

Ah, sorry for closing this :D Sure, I'll make a PR sometime later

[nikophil]

we may release Foundry 3, along with Symfony 8 in november. This might be the good time to think a little bit more how we can improve Factory collection, as we may allow some breaking change at this time