Handle all hydration mode in QueryResultDynamicReturnTypeExtension

[VincentLanglet]

Since there was recent revert to release 1.3.8, this PR has:

• Handle all hydration mode in QueryResultDynamicReturnTypeExtension #412
• Avoid false positive about getArrayResult when selecting objects #444
• Use benevolent union for scalar in queries #447

[VincentLanglet]

Hi @ondrejmirtes, would you have time to take a new look on this and tell me if you encounter any issue on your personnal projects with this PR with all the new fixes provided. Thanks :)

[VincentLanglet]

Hi @ondrejmirtes, happy new year.

Any feedback about how I could help moving this PR forward ?
I tried the PR on my side and got:

• no false positive
• three real error found by phpstan

[ondrejmirtes]

I can merge this, test 4 real world projects, collect errors and then revert it again.

[VincentLanglet]

I can merge this, test 4 real world projects, collect errors and then revert it again.

Thanks, please ping me on any issue. I'll make myself available.

[ondrejmirtes]

Real world project number 1: 48 new errors, most are "Casting to int something that's already int.". A few real bugs found. But the PR is not currently usable because COUNT(pr.id) AS count as inferred as int<0, max>|numeric-string which will be solved by #506.

Real world project number 2: 21 new errors, some queries use getScalarResult and getResult(AbstractQuery::HYDRATE_SCALAR). In these cases the inferred type is mixed which should be improved. Example query (anonymized):

 $qb = $this->createQueryBuilder('a');
 $result = $qb
            ->select('a.one, a.two, a.three')
            ->andWhere($qb->expr()->in('a.four', $four))
            ->orderBy('a.date', 'ASC')
            ->getQuery()
            ->getResult(AbstractQuery::HYDRATE_SCALAR);

Can you please look into this bug? Reverting the change for now.

[VincentLanglet]

Real world project number 1: 48 new errors, most are "Casting to int something that's already int.". A few real bugs found. But the PR is not currently usable because COUNT(pr.id) AS count as inferred as int<0, max>|numeric-string which will be solved by #506.

Great.

Real world project number 2: 21 new errors, some queries use getScalarResult and getResult(AbstractQuery::HYDRATE_SCALAR). In these cases the inferred type is mixed which should be improved. Example query (anonymized):

 $qb = $this->createQueryBuilder('a');
 $result = $qb
            ->select('a.one, a.two, a.three')
            ->andWhere($qb->expr()->in('a.four', $four))
            ->orderBy('a.date', 'ASC')
            ->getQuery()
            ->getResult(AbstractQuery::HYDRATE_SCALAR);

Can you please look into this bug? Reverting the change for now.

Sure, what was the inferred type before mixed ?

[ondrejmirtes]

I'm sorry to be wrong here. The type before was mixed, the type with your PR is list<array>. But it should be more precise. Because right now it moves the error from level 9 to a lower level.

[VincentLanglet]

I'm sorry to be wrong here. The type before was mixed, the type with your PR is list<array>. But it should be more precise. Because right now it moves the error from level 9 to a lower level.

Ok I see.

I had a test for

$query = $em->createQuery('
			SELECT		m.intColumn, m.stringNullColumn
			FROM		QueryResult\Entities\Many m
		');

		assertType(
			'list<array{intColumn: int, stringNullColumn: string|null}>',
			$query->getResult(AbstractQuery::HYDRATE_SCALAR)
		);

So I assume that in your example, 'a.one, a.two, a.three' are not all scalar columns. (Maybe a datetime)

It's not easy in those case to infer the correct type.
I prefer to follow the same strategy than #444
If I cannot exactly infer the type, at least I return the same than before to avoid changing error levels.

opened #520 as draft since we need #506 first