Bug: return type resolving

[msvrtan]

Hi,

I'we just noticed that when using 'phpstan-doctrine' extension, there is a weird behaviour/bug with findOneBy methods: doesnt report error on calling non-existing method.

As a proof of concept (not able to share project I work on): https://github.com/msvrtan/phpstan-bug`

When calling a method findOneByUuid, it doesnt understand it's getting a Customer|null back but when calling findXOneByUuid it does: https://travis-ci.com/msvrtan/phpstan-bug/jobs/183766249

I'we also created a PR msvrtan/phpstan-bug#2 that fails https://travis-ci.com/msvrtan/phpstan-bug/builds/103870199 once I remove that Repository does extend EntityRepository...

On the other hand, if I do 'inject' repository as

/* @var \Doctrine\ORM\EntityRepository<\Customer\Customer> */
private $customerRepository;

it does report ...

I know this would be a quick 'fix' to go thru codebase and add those docblocks but it still feels like unexpected behaviour

[ondrejmirtes]

Hi,
thanks - I think the current behaviour is understandable and logical, although it could be improved.

When you typehint CustomerRepository, it knows that it's an object of CustomerRepository, but it doesn't know if the injected instance is supposed to be bound to a specific entity or not - you can reuse Doctrine repositories for multiple entities.

Because you define CustomerRepository::findXOneByUuid by hand, it knows the return type. It should also know the return type of findOneByUuid but that method clashes with the magic method naming convention, and since the $this->repository's entity type is unknown, it won't report anything - it basically returns mixed. But something like that is already reported in #52 (but please keep this open, I need more testcases like this :)).

If you typehint CustomerRepository<\Customer\CustomerEntity> (and provide object manager loader in the config), it will also know return type of findOneByUuid - but not thanks to your typehint in CustomerRepository, but because of Doctrine's magic method naming convention. If you declared your method as public function findOneByUuid($uuid): int, it would still think it's ?CustomerEntity. But that's already in #52 :)

The solution is to ask in a few places (like ObjectRepositoryDynamicReturnTypeExtension) to ask for the real method return typehint before inferring it's going to be ?Entity...

[ondrejmirtes]

One more side-effect of not typehinting CustomerRepository<\Customer\CustomerEntity> is that it will not tell you when you're calling findOneBy* method for an unknown field :)

[ondrejmirtes]

Maybe a fix is easier thank explanation :) f7ce984 Thanks!

[ondrejmirtes]

Released as 0.11.2 :) https://github.com/phpstan/phpstan-doctrine/releases/tag/0.11.2

[msvrtan]

Works on example repo! Will try it now on original project

[msvrtan]

OK, now it reports the problem properly! Thank you very much, I owe you some 🍽 or 🍻 next time we see each other

[ondrejmirtes]

Btw you should also include rules.neon in your project, not just extension.neon 😊

On Mon, 11 Mar 2019 at 13:53, Miro Svrtan ***@***.***> wrote: OK, now it reports the problem properly! Thank you very much, I owe you some 🍽 or 🍻 next time we see each other — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#57 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGZuBOSSd73Z__j-RsXRHo2ZaHTyGRCks5vVlHAgaJpZM4boMpf> .

-- Ondřej Mirtes

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.