QueryBuilder Result returns mixed when created in another method

[alexander-schranz]

I'm still running into the #266 issue, which I thought should be fixed with: 149cf71

But in my cases it ends still in mixed state.

I created a reproducer here:

git clone git@github.com:alexander-schranz/phpstan-query-builder-reproducer.git
cd phpstan-query-builder-reproducer/
composer install

vendor/bin/phpstan analyse

https://github.com/alexander-schranz/phpstan-query-builder-reproducer/blob/main/src/Repository/VisitorRepository.php

Output:

 ------ ----------------------------------------------------------------------------------------------------------
  Line   src/Repository/VisitorRepository.php
 ------ ----------------------------------------------------------------------------------------------------------
  46     Dumped type: mixed
  48     Method App\Repository\VisitorRepository::findBy() should return iterable<App\Entity\Visitor> but returns
         mixed.
  60     Dumped type: mixed
  62     Method App\Repository\VisitorRepository::findFlatBy() should return iterable<array{id: int, title:
         string|null}> but returns mixed.
  75     Dumped type: mixed
  77     Method App\Repository\VisitorRepository::findOneBy() should return App\Entity\Visitor|null but returns
         mixed.
  90     Dumped type: mixed
  92     Method App\Repository\VisitorRepository::getOneBy() should return App\Entity\Visitor but returns mixed.
 ------ ----------------------------------------------------------------------------------------------------------

Not sure what is different then to the test case 🤔

[sabbelasichon]

Have the same problem here, but cannot find the root cause. The QueryResultDynamicReturnTypeExtension always gives me MixedType as $queryResultType so i cannot infer the right type. But i cannot figure out why this is the case.

[sabbelasichon]

Should have read better the documentation. Added the option objectManagerLoader with a proper php configuration and now it works.

[ondrejmirtes]

Nice but apparently it's not the case for the reproduction repo: https://github.com/alexander-schranz/phpstan-query-builder-reproducer/blob/26c04ecb3495a187f75d8082e53a5749cc62b3ff/phpstan.neon#L18

[lenybernard]

Same probleme here. @sabbelasichon can you explain please what does mean Added the option objectManagerLoader with a proper php configuration

[alexander-schranz]

@lenybernard

can you explain please what does mean Added the option objectManagerLoader with a proper php configuration

See README about objectManagerLoader configuration:

• https://github.com/phpstan/phpstan-doctrine#configuration

Or here another example from our sulu/skeleton which is symfony based:

• https://github.com/sulu/skeleton/blob/581ad8dbcca60def7c5b3f66cc125c4cc3111296/phpstan.neon#L7-L8
• https://github.com/sulu/skeleton/blob/2.5/tests/phpstan/object-manager.php

Sadly it doesn't work for my case. But hope it works for you.

[cyrille-osyla]

@alexander-schranz same problem for me. The proposed solution does not work... 😢

[CodeCasterNL]

Can't get it to work either with the suggestions mentioned above. To ignore this error (supports subnamespaces like App\Entity\Foo\Bar and findXXX() methods returning a (nullable) entity or array of entities):

# phpstan.neon
parameters:
  ignoreErrors:
    - '#Method App\\Repository\\(\w+\\)*\w+Repository::(get|find)\w+\(\) should return ((array|iterable)<)?App\\Entity\\(\w+\\)*\w+(>)?(\|null)? but returns (object|mixed)\.#'

[cyrille-osyla]

Ignored error pattern #Method App\\Repository\\(\w+\\)*\w+Repository::find\w+\(\) should return (array<)?App\\Entity\\(\w+\\)*\w+(>)?(\|null)? but returns mixed\.# was not matched in reported errors.
@CodeCasterNL Any idea ?

[CodeCasterNL]

@cyrille-osyla which errors are reported for your repositories when you run PHPStan?

[cyrille-osyla]

@CodeCasterNL
Method App\Repository\Category\CategoryRepository::getOneCategoryById() should return App\Entity\Category\Category but returns mixed.

[ondrejmirtes]

There's a helpful tool on PHPStan's website to generate the ignoreErrors entry: https://phpstan.org/user-guide/ignoring-errors#generate-an-ignoreerrors-entry

[CodeCasterNL]

@cyrille-osyla I've updated the regex, your method started with "get" instead of "find".

[cyrille-osyla]

@CodeCasterNL
ah yes sorry...
I had changed the name of the method to fix the phpstan problem otherwise.
Now it works thank you

@ondrejmirtes good to know thank you !

[takeoto]

Hi! Is there a way to fix this without ignoring it?

[curry684]

I'm also running into this issue in a bleeding edge development environment, both with Symfony 6.2 and 6.3-beta.

Simple reproducer:

use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
use Doctrine\Persistence\ManagerRegistry;

/**
 * @extends ServiceEntityRepository<Unit>
 */
class UnitRepository extends ServiceEntityRepository
{
    public function __construct(ManagerRegistry $registry)
    {
        parent::__construct($registry, Unit::class);
    }

    /**
     * @return Unit[]
     */
    public function getUnits(): array
    {
        return $this->createQueryBuilder('u')->getQuery()->getResult();
    }
}

Gives:

  35     Method App\Repository\UnitRepository::getUnits() should return array<App\Entity\Unit> but returns mixed.

Notably this only occurs at level 9, it passes without issue on level 8. PHPStan documents level 9 as:

be strict about the mixed type - the only allowed operation you can do with it is to pass it to another mixed

Looking at https://github.com/phpstan/phpstan-doctrine/blob/1.3.x/stubs/ORM/QueryBuilder.stub#L17-L22 it would seem PHPStan is shooting its own foot - the stub is prescribing something that cannot pass level 9 by definition.

[curry684]

Does anyone have a clue (because I don't tbh) on how to implement a fundamental solution or are we stuck with an ignore entry forever?

Notably the 2.0 stub still uses mixed: https://github.com/phpstan/phpstan-doctrine/blob/2.0.x/stubs/ORM/QueryBuilder.stub

[ondrejmirtes]

What's in the stub here is irrelevant. There's a dynamic return type extension that overrides that: https://github.com/phpstan/phpstan-doctrine/blob/2.0.x/src/Type/Doctrine/QueryBuilder/QueryBuilderGetQueryDynamicReturnTypeExtension.php

I suspect that the problem with:

35 Method App\Repository\UnitRepository::getUnits() should return array<App\Entity\Unit> but returns mixed.

Might happen for different reasons in different projects. It usually works properly.

[curry684]

I have a project here with 12 of those rare exceptions 😆 Thanks for the pointer to that file, I see it changed a lot in 2.0 so I'll have a look at a fix when we've upgraded this project to 2.0 (which includes fixing 99 errors not reported before so not today).

[ondrejmirtes]

If you reproduce at least one, ideally here in the test suite, that would be great.

Here's a very similar test:

• https://github.com/phpstan/phpstan-doctrine/blob/2.0.x/tests/Type/Doctrine/QueryBuilder/QueryBuilderGetQueryDynamicReturnTypeExtensionTest.php
• https://github.com/phpstan/phpstan-doctrine/blob/2.0.x/tests/Type/Doctrine/data/QueryResult/queryBuilderGetQuery.php

It shows the happy paths work.

[curry684]

Here's the most basic one I have, integrally copied:

/** @extends ServiceEntityRepository<Payment> */
class PaymentRepository extends ServiceEntityRepository
{
    public function __construct(ManagerRegistry $registry)
    {
        parent::__construct($registry, Payment::class);
    }

    public function findByExternalTransactionId(string $id): ?Payment
    {
        return $this
            ->createQueryBuilder('p')
            ->where('p.externalTransactionId = :id')->setParameter('id', $id)
            ->getQuery()->getOneOrNullResult()
        ;
    }
}

Gives:

Method App\Repository\PaymentRepository::findByExternalTransactionId() should return App\Entity\Payment|null but returns mixed.  
         🪪  return.type       

Using EntityRepository instead of ServiceEntityRepository yields the same result.

[janedbal]

getOneOrNullResult needs to be called with Query::HYDRATE_OBJECT, see readme

[curry684]

Oh my that does indeed fix it, but the DX on this is horrible. I also have cases with the dynamic query builder scenario mentioned there, which are only realistically discoverable by enabling reportDynamicQueryBuilders.

We should definitely have more verbose errors here like an extra level 9 check on calling getOneOrNullResult without parameters as it can never pass that way.

[janedbal]

We are using this custom PHPStan rule to ensure only inferrable calls are used in our codebase.