phpstan-typo3 icon indicating copy to clipboard operation
phpstan-typo3 copied to clipboard

Having a class called …Repository should not mandate it being an Extbase repository

Open andreaswolf opened this issue 1 year ago • 6 comments

The repository pattern existed before Extbase and is useful w/o Extbase, so having a check that lets PhpStan fail if a class called SomethingRepository does not (directly or via a parent class) implement RepositoryInterface seems unnecessarily strict to me.

andreaswolf avatar May 07 '23 11:05 andreaswolf

@sascha-egerer What do you think? Should we loosen this check here? I would vote +1 for it.

sabbelasichon avatar Feb 21 '24 11:02 sabbelasichon

Makes sense

sascha-egerer avatar Mar 04 '24 08:03 sascha-egerer

@andreaswolf Could you create a PR for that?

sabbelasichon avatar Apr 05 '24 08:04 sabbelasichon

I have the relevant changes ready, I'm just not sure enough they don't break anything… I tried writing a test for a plain repository, but the only type I get for any findBy* method is "*ERROR*".

Help greatly appreciated :)

andreaswolf avatar Apr 26 '24 10:04 andreaswolf

@andreaswolf Could you provide more details?

sascha-egerer avatar May 13 '24 15:05 sascha-egerer

@sascha-egerer see the WIP commits on the 124-… branch in my fork. The two latest fixups are failed attempts to fix the issue.

andreaswolf avatar May 13 '24 16:05 andreaswolf

I think it may be better to just remove the \PHPStan\ShouldNotHappenException in translateRepositoryNameToModelName instead, if the current class is not a RepositoryInterface::class. I've run into several issues, where a repository extended an abstract repository of another extension which implemented TYPO3\CMS\Extbase\Persistence\Repository, but phpstan kept throwing the exception, that the repository does not implement RepositoryInterface.

derhansen avatar Oct 07 '24 14:10 derhansen

I'll try to check that soon.

sascha-egerer avatar Oct 09 '24 07:10 sascha-egerer