search

sascha-egerer / phpstan-typo3

TYPO3 CMS class reflection extension for PHPStan & framework-specific rules
MIT License
42 stars 22 forks source link

What to do with the stubs? #133

Closed alexanderschnitzler closed 6 months ago

alexanderschnitzler commented 1 year ago

Hey, as of TYPO3 12.x, the Extbase domain layer has full support for generics. In contrary to the stubs, it uses T, not TEntity. It doesn't cause any problems unless you want to use the new "generic" find methods of the Repository findBy(), findOneBy and countBy. Those are not covered by the stubs and use T and therefore phpstan fails to gather their return types.

What's the plan with this package? Will it continue to support multiple TYPO3 versions? Do you want to adjust the stubs?

To be frank: I think we should focus on letting the core handle the phpstan implementation and remove everything from this package that isn't needed any more.

Is there an option to disable/enable stubs dynamically? I doubt it but it would be really good to tell this package what TYPO3 version is running and it selectively enables/disables it's features.

DanielSiepmann commented 1 year ago

I guess there is already a stub loader handling loading of stubs based on TYPO3 version, maybe use that for now: https://github.com/sascha-egerer/phpstan-typo3/blob/master/src/Stubs/StubFilesExtensionLoader.php

alexanderschnitzler commented 1 year ago

Thanks @DanielSiepmann! I will have a look and see what's doable with that.

sabbelasichon commented 8 months ago

I had a look into the issue and i think we could use almost all stubs from TYPO3 Core since TYPO3 12.4. IMHO there is only one piece missing or left.

The line https://github.com/TYPO3-CMS/extbase/blob/12.4/Classes/Persistence/Repository.php#L330 should be:

php @return class-string<static> Class name of the repository.

sabbelasichon commented 8 months ago

@oliverklee What do you think? Should we change this line in Core?

oliverklee commented 8 months ago

Should we change this line in Core?

Yes, that makes sense (in main, 13.0, 12.4, 11.5).

@sabbelasichon Would you be willing to create the change? Then I can review it.

sabbelasichon commented 8 months ago

See: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83056

sascha-egerer commented 6 months ago

Can we close this issue? It's possible to define stubs based on TYPO3 version so i don't think we have a TODO here right now, right?

sabbelasichon commented 6 months ago

@sascha-egerer Let me check.