From d52c1dabb55c6dc417accdb52b55896d565d4298 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 30 Jun 2024 15:11:33 +0200 Subject: [PATCH 1/2] Infer getArrayResult --- .../HydrationModeReturnTypeResolver.php | 48 +++++++++++++ .../QueryResultDynamicReturnTypeExtension.php | 37 ++++++---- ...QueryResultTypeWalkerHydrationModeTest.php | 70 +++++++++++++++++++ .../Doctrine/data/QueryResult/queryResult.php | 35 ++++++++++ 4 files changed, 175 insertions(+), 15 deletions(-) diff --git a/src/Type/Doctrine/HydrationModeReturnTypeResolver.php b/src/Type/Doctrine/HydrationModeReturnTypeResolver.php index dabf4d9b..c37a0c96 100644 --- a/src/Type/Doctrine/HydrationModeReturnTypeResolver.php +++ b/src/Type/Doctrine/HydrationModeReturnTypeResolver.php @@ -10,10 +10,13 @@ use PHPStan\Type\IntegerRangeType; use PHPStan\Type\IntegerType; use PHPStan\Type\IterableType; +use PHPStan\Type\MixedType; use PHPStan\Type\ObjectWithoutClassType; use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; +use PHPStan\Type\TypeTraverser; use PHPStan\Type\TypeUtils; +use PHPStan\Type\TypeWithClassName; use PHPStan\Type\VoidType; class HydrationModeReturnTypeResolver @@ -44,6 +47,9 @@ public function getMethodReturnTypeForHydrationMode( switch ($hydrationMode) { case AbstractQuery::HYDRATE_OBJECT: break; + case AbstractQuery::HYDRATE_ARRAY: + $queryResultType = $this->getArrayHydratedReturnType($queryResultType, $objectManager); + break; case AbstractQuery::HYDRATE_SIMPLEOBJECT: $queryResultType = $this->getSimpleObjectHydratedReturnType($queryResultType); break; @@ -84,6 +90,48 @@ public function getMethodReturnTypeForHydrationMode( } } + /** + * When we're array-hydrating object, we're not sure of the shape of the array. + * We could return `new ArrayType(new MixedType(), new MixedType())` + * but the lack of precision in the array keys/values would give false positive. + * + * @see https://github.com/phpstan/phpstan-doctrine/pull/412#issuecomment-1497092934 + */ + private function getArrayHydratedReturnType(Type $queryResultType, ?ObjectManager $objectManager): ?Type + { + $mixedFound = false; + $queryResultType = TypeTraverser::map( + $queryResultType, + static function (Type $type, callable $traverse) use ($objectManager, &$mixedFound): Type { + $isObject = (new ObjectWithoutClassType())->isSuperTypeOf($type); + if ($isObject->no()) { + return $traverse($type); + } + if ( + $isObject->maybe() + || !$type instanceof TypeWithClassName + || $objectManager === null + ) { + $mixedFound = true; + + return new MixedType(); + } + + /** @var class-string $className */ + $className = $type->getClassName(); + if (!$objectManager->getMetadataFactory()->hasMetadataFor($className)) { + return $traverse($type); + } + + $mixedFound = true; + + return new MixedType(); + } + ); + + return $mixedFound ? null : $queryResultType; + } + private function getSimpleObjectHydratedReturnType(Type $queryResultType): ?Type { if ((new ObjectWithoutClassType())->isSuperTypeOf($queryResultType)->yes()) { diff --git a/src/Type/Doctrine/Query/QueryResultDynamicReturnTypeExtension.php b/src/Type/Doctrine/Query/QueryResultDynamicReturnTypeExtension.php index ef638cad..818c3869 100644 --- a/src/Type/Doctrine/Query/QueryResultDynamicReturnTypeExtension.php +++ b/src/Type/Doctrine/Query/QueryResultDynamicReturnTypeExtension.php @@ -28,6 +28,10 @@ final class QueryResultDynamicReturnTypeExtension implements DynamicMethodReturn 'getSingleResult' => 0, ]; + private const METHOD_HYDRATION_MODE = [ + 'getArrayResult' => AbstractQuery::HYDRATE_ARRAY, + ]; + /** @var ObjectMetadataResolver */ private $objectMetadataResolver; @@ -50,7 +54,8 @@ public function getClass(): string public function isMethodSupported(MethodReflection $methodReflection): bool { - return isset(self::METHOD_HYDRATION_MODE_ARG[$methodReflection->getName()]); + return isset(self::METHOD_HYDRATION_MODE_ARG[$methodReflection->getName()]) + || isset(self::METHOD_HYDRATION_MODE[$methodReflection->getName()]); } public function getTypeFromMethodCall( @@ -61,21 +66,23 @@ public function getTypeFromMethodCall( { $methodName = $methodReflection->getName(); - if (!isset(self::METHOD_HYDRATION_MODE_ARG[$methodName])) { - throw new ShouldNotHappenException(); - } - - $argIndex = self::METHOD_HYDRATION_MODE_ARG[$methodName]; - $args = $methodCall->getArgs(); - - if (isset($args[$argIndex])) { - $hydrationMode = $scope->getType($args[$argIndex]->value); + if (isset(self::METHOD_HYDRATION_MODE[$methodName])) { + $hydrationMode = new ConstantIntegerType(self::METHOD_HYDRATION_MODE[$methodName]); + } elseif (isset(self::METHOD_HYDRATION_MODE_ARG[$methodName])) { + $argIndex = self::METHOD_HYDRATION_MODE_ARG[$methodName]; + $args = $methodCall->getArgs(); + + if (isset($args[$argIndex])) { + $hydrationMode = $scope->getType($args[$argIndex]->value); + } else { + $parametersAcceptor = ParametersAcceptorSelector::selectSingle( + $methodReflection->getVariants() + ); + $parameter = $parametersAcceptor->getParameters()[$argIndex]; + $hydrationMode = $parameter->getDefaultValue() ?? new NullType(); + } } else { - $parametersAcceptor = ParametersAcceptorSelector::selectSingle( - $methodReflection->getVariants() - ); - $parameter = $parametersAcceptor->getParameters()[$argIndex]; - $hydrationMode = $parameter->getDefaultValue() ?? new NullType(); + throw new ShouldNotHappenException(); } $queryType = $scope->getType($methodCall->var); diff --git a/tests/Type/Doctrine/Query/QueryResultTypeWalkerHydrationModeTest.php b/tests/Type/Doctrine/Query/QueryResultTypeWalkerHydrationModeTest.php index f78b81a4..de3bf93b 100644 --- a/tests/Type/Doctrine/Query/QueryResultTypeWalkerHydrationModeTest.php +++ b/tests/Type/Doctrine/Query/QueryResultTypeWalkerHydrationModeTest.php @@ -138,6 +138,42 @@ public static function getTestData(): iterable Query::HYDRATE_SIMPLEOBJECT, ]; + yield 'getResult(array), full entity' => [ + new MixedType(), + ' + SELECT s + FROM QueryResult\Entities\Simple s + ', + 'getResult', + Query::HYDRATE_ARRAY, + ]; + + yield 'getResult(array), fields' => [ + self::list(self::constantArray([ + [new ConstantStringType('decimalColumn'), self::numericString()], + [new ConstantStringType('floatColumn'), new FloatType()], + ])), + ' + SELECT s.decimalColumn, s.floatColumn + FROM QueryResult\Entities\Simple s + ', + 'getResult', + Query::HYDRATE_ARRAY, + ]; + + yield 'getResult(array), expressions' => [ + self::list(self::constantArray([ + [new ConstantStringType('decimalColumn'), self::floatOrIntOrStringified()], + [new ConstantStringType('floatColumn'), self::floatOrStringified()], + ])), + ' + SELECT -s.decimalColumn as decimalColumn, -s.floatColumn as floatColumn + FROM QueryResult\Entities\Simple s + ', + 'getResult', + Query::HYDRATE_ARRAY, + ]; + yield 'getResult(object), fields' => [ self::list(self::constantArray([ [new ConstantStringType('decimalColumn'), self::numericString()], @@ -210,6 +246,16 @@ public static function getTestData(): iterable Query::HYDRATE_SIMPLEOBJECT, ]; + yield 'toIterable(array), full entity' => [ + new MixedType(), + ' + SELECT s + FROM QueryResult\Entities\Simple s + ', + 'toIterable', + Query::HYDRATE_ARRAY, + ]; + yield 'getArrayResult(), full entity' => [ new MixedType(), ' @@ -219,6 +265,30 @@ public static function getTestData(): iterable 'getArrayResult', ]; + yield 'getArrayResult(), fields' => [ + self::list(self::constantArray([ + [new ConstantStringType('decimalColumn'), self::numericString()], + [new ConstantStringType('floatColumn'), new FloatType()], + ])), + ' + SELECT s.decimalColumn, s.floatColumn + FROM QueryResult\Entities\Simple s + ', + 'getArrayResult', + ]; + + yield 'getArrayResult(), expressions' => [ + self::list(self::constantArray([ + [new ConstantStringType('decimalColumn'), self::floatOrIntOrStringified()], + [new ConstantStringType('floatColumn'), self::floatOrStringified()], + ])), + ' + SELECT -s.decimalColumn as decimalColumn, -s.floatColumn as floatColumn + FROM QueryResult\Entities\Simple s + ', + 'getArrayResult', + ]; + yield 'getResult(single_scalar), decimal field' => [ new MixedType(), ' diff --git a/tests/Type/Doctrine/data/QueryResult/queryResult.php b/tests/Type/Doctrine/data/QueryResult/queryResult.php index de4cc5bf..46734609 100644 --- a/tests/Type/Doctrine/data/QueryResult/queryResult.php +++ b/tests/Type/Doctrine/data/QueryResult/queryResult.php @@ -188,6 +188,41 @@ public function testReturnTypeOfQueryMethodsWithExplicitArrayHydrationMode(Entit 'mixed', $query->getOneOrNullResult(AbstractQuery::HYDRATE_ARRAY) ); + + + $query = $em->createQuery(' + SELECT m.intColumn, m.stringNullColumn, m.datetimeColumn + FROM QueryResult\Entities\Many m + '); + + assertType( + 'list', + $query->getResult(AbstractQuery::HYDRATE_ARRAY) + ); + assertType( + 'list', + $query->getArrayResult() + ); + assertType( + 'list', + $query->execute(null, AbstractQuery::HYDRATE_ARRAY) + ); + assertType( + 'list', + $query->executeIgnoreQueryCache(null, AbstractQuery::HYDRATE_ARRAY) + ); + assertType( + 'list', + $query->executeUsingQueryCache(null, AbstractQuery::HYDRATE_ARRAY) + ); + assertType( + 'array{intColumn: int, stringNullColumn: string|null, datetimeColumn: DateTime}', + $query->getSingleResult(AbstractQuery::HYDRATE_ARRAY) + ); + assertType( + 'array{intColumn: int, stringNullColumn: string|null, datetimeColumn: DateTime}|null', + $query->getOneOrNullResult(AbstractQuery::HYDRATE_ARRAY) + ); } From 8f84e0a837d99cba4788d7f2c35849c4629416d5 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 5 Apr 2023 17:01:29 +0200 Subject: [PATCH 2/2] Query type inference failures reproductions --- .../QueryBuilderReproductionsTest.php | 38 +++++++++++++++++++ .../data/QueryResult/Entities/OrderItem.php | 20 ++++++++++ .../data/QueryResult/Entities/Product.php | 17 +++++++++ .../data/QueryResult/entity-manager.php | 11 ++++++ .../data/queryBuilderReproductions.php | 31 +++++++++++++++ 5 files changed, 117 insertions(+) create mode 100644 tests/Type/Doctrine/QueryBuilderReproductionsTest.php create mode 100644 tests/Type/Doctrine/data/QueryResult/Entities/OrderItem.php create mode 100644 tests/Type/Doctrine/data/QueryResult/Entities/Product.php create mode 100644 tests/Type/Doctrine/data/queryBuilderReproductions.php diff --git a/tests/Type/Doctrine/QueryBuilderReproductionsTest.php b/tests/Type/Doctrine/QueryBuilderReproductionsTest.php new file mode 100644 index 00000000..80c00e65 --- /dev/null +++ b/tests/Type/Doctrine/QueryBuilderReproductionsTest.php @@ -0,0 +1,38 @@ + + */ + public function dataFileAsserts(): iterable + { + yield from $this->gatherAssertTypes(__DIR__ . '/data/queryBuilderReproductions.php'); + } + + /** + * @dataProvider dataFileAsserts + * @param mixed ...$args + */ + public function testFileAsserts( + string $assertType, + string $file, + ...$args + ): void + { + $this->assertFileAsserts($assertType, $file, ...$args); + } + + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/data/QueryResult/config.neon', + ]; + } + +} diff --git a/tests/Type/Doctrine/data/QueryResult/Entities/OrderItem.php b/tests/Type/Doctrine/data/QueryResult/Entities/OrderItem.php new file mode 100644 index 00000000..b9b570c1 --- /dev/null +++ b/tests/Type/Doctrine/data/QueryResult/Entities/OrderItem.php @@ -0,0 +1,20 @@ + true])] + private ?int $id = null; + +} diff --git a/tests/Type/Doctrine/data/QueryResult/entity-manager.php b/tests/Type/Doctrine/data/QueryResult/entity-manager.php index c9ba38af..2760d5f4 100644 --- a/tests/Type/Doctrine/data/QueryResult/entity-manager.php +++ b/tests/Type/Doctrine/data/QueryResult/entity-manager.php @@ -22,11 +22,22 @@ [__DIR__ . '/Entities'] ), 'QueryResult\Entities\\'); +if (PHP_VERSION_ID >= 80100) { + $metadataDriver->addDriver( + new AttributeDriver([__DIR__ . '/Entities']), + 'QueryResult\Entities\\' + ); +} + if (property_exists(Column::class, 'enumType') && PHP_VERSION_ID >= 80100) { $metadataDriver->addDriver(new AnnotationDriver( new AnnotationReader(), [__DIR__ . '/EntitiesEnum'] ), 'QueryResult\EntitiesEnum\\'); + $metadataDriver->addDriver( + new AttributeDriver([__DIR__ . '/EntitiesEnum']), + 'QueryResult\EntitiesEnum\\' + ); } $config->setMetadataDriverImpl($metadataDriver); diff --git a/tests/Type/Doctrine/data/queryBuilderReproductions.php b/tests/Type/Doctrine/data/queryBuilderReproductions.php new file mode 100644 index 00000000..e03479fc --- /dev/null +++ b/tests/Type/Doctrine/data/queryBuilderReproductions.php @@ -0,0 +1,31 @@ +entityManager = $entityManager; + } + + public function doFoo(): void + { + $result = $this->entityManager->createQueryBuilder() + ->select('DISTINCT IDENTITY(oi.product) AS id') + ->from(OrderItem::class, 'oi') + ->join('oi.product', 'p') + ->getQuery() + ->getArrayResult(); + assertType('list', $result); + } + +}