Skip to content

[PHPStan] improve FactoryCollection generics annotation with template-type #894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
edglev opened this issue May 5, 2025 · 8 comments
Open
Labels
enhancement New feature or request

Comments

@edglev
Copy link

edglev commented May 5, 2025

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
Copy link
Member

nikophil commented May 5, 2025

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
Copy link
Author

edglev commented May 5, 2025

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

@edglev edglev closed this as completed May 5, 2025
@edglev edglev reopened this May 5, 2025
@edglev
Copy link
Author

edglev commented May 5, 2025

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
Copy link
Member

nikophil commented May 5, 2025

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
Copy link
Author

edglev commented May 5, 2025

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
Copy link
Member

nikophil commented May 6, 2025

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

@edglev edglev closed this as completed May 6, 2025
@nikophil
Copy link
Member

nikophil commented May 6, 2025

@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)

@nikophil nikophil reopened this May 6, 2025
@nikophil nikophil added the enhancement New feature or request label May 6, 2025
@nikophil nikophil changed the title PhpStan complains about PersistentProxyObjectFactory subtypes [PHPStan] improve FactoryCollection generics annotation with template-type May 6, 2025
@edglev
Copy link
Author

edglev commented May 6, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

2 participants