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

Infer getArrayResult

Open ondrejmirtes opened this issue 1 year ago • 3 comments

ondrejmirtes avatar Jun 30 '24 13:06 ondrejmirtes

Hi, maybe it's better to continue the discussion https://github.com/phpstan/phpstan-doctrine/pull/592#issue-2382244642 here (cc @janedbal)

There is a lot of failure on this draft but if I understand correctly the one exposed by your reproducer is

70) PHPStan\Type\Doctrine\QueryBuilderReproductionsTest::testFileAsserts with data set "/home/runner/work/phpstan-doctrine/phpstan-doctrine/tests/Type/Doctrine/data/queryBuilderReproductions.php:28" ('type', '/home/runner/work/phpstan-doc...ns.php', 'list<array{id: int}>', 'list<array{id: int|null}>', 28)
Expected type list<array{id: int}>, got type list<array{id: int|null}> in /home/runner/work/phpstan-doctrine/phpstan-doctrine/tests/Type/Doctrine/data/queryBuilderReproductions.php on line 28.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'list<array{id: int}>'
+'list<array{id: int|null}>'

which is similar to the sentence

I'd love if we could make phpstan-doctrine understand that where/andWhere can make a selected field non-nullable for example.

I'm sure you've already noticed that HYDRATE_OBJECT already infer as list<array{id: int|null}>.

For the where situation I had last time a "solution" to use a BenevolentUnion https://github.com/phpstan/phpstan-doctrine/pull/447 ; I dunno if the PR handled the IDENTITY/join issues but it may be possible to do it. Maybe this could be a good first step ?

This would allow HYDRATE_OBJECT to be inferred as list<array{id: __benevolent<int|null>}>, and avoid some false positive. WDYT ? Or do you (@ondrejmirtes or @janedbal) see a way to understand when a nullable field is not null ?

VincentLanglet avatar Jun 30 '24 22:06 VincentLanglet

My point of view:

  • It should be possible to implement proper deduction of "this field is not null due to this WHERE" by traversing the DQL AST (big effort tho)
    1. benevolent union is smart hack to avoid some issues but (ii)
    2. phpstan-doctrine will never have 100% accurate types for all codebases, thus will always need inline vardocs or other methods to adjust those types
      • (e.g. when you pass QB to another method, you have no idea what happens there)

janedbal avatar Jul 01 '24 11:07 janedbal

If I update the "failing test" you added @ondrejmirtes by changing getArrayResult to getResult

$result = $this->entityManager->createQueryBuilder()
			->select('DISTINCT IDENTITY(oi.product) AS id')
			->from(OrderItem::class, 'oi')
			->join('oi.product', 'p')
			->getQuery()
			->getResult(); // or getArrayResult

-> with getResult it's inferred as list<array{id: int|null}>. -> with getArrayResult is inferred as array.

If the fact that getArrayResult is not inferred as list<array{id: int}>, shouldn't it have been a blocker for getResult too ?

Currently I solve my issues with changing every call to getArrayResult in my code base to getResult in order to have PHPStan inferring the code ; but I would expect to have at least the same behavior than getResult.

VincentLanglet avatar Aug 25 '24 13:08 VincentLanglet