From 3313ae0d2ec631e8ac9f8f03ed30bc7ad6ea980b Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Fri, 17 Nov 2023 10:13:26 +0100 Subject: [PATCH 1/2] QueryBuilder: allow dynamic args in methods not affecting result type --- ...lderGetQueryDynamicReturnTypeExtension.php | 18 ++++ .../Doctrine/ORM/QueryBuilderDqlRuleTest.php | 13 +++ .../ORM/data/query-builder-dynamic.php | 78 +++++++++++++++ .../data/QueryResult/queryBuilderGetQuery.php | 95 +++++++++++++++++++ 4 files changed, 204 insertions(+) create mode 100644 tests/Rules/Doctrine/ORM/data/query-builder-dynamic.php diff --git a/src/Type/Doctrine/QueryBuilder/QueryBuilderGetQueryDynamicReturnTypeExtension.php b/src/Type/Doctrine/QueryBuilder/QueryBuilderGetQueryDynamicReturnTypeExtension.php index 5760f21f..09571b5a 100644 --- a/src/Type/Doctrine/QueryBuilder/QueryBuilderGetQueryDynamicReturnTypeExtension.php +++ b/src/Type/Doctrine/QueryBuilder/QueryBuilderGetQueryDynamicReturnTypeExtension.php @@ -32,6 +32,21 @@ class QueryBuilderGetQueryDynamicReturnTypeExtension implements DynamicMethodReturnTypeExtension { + /** + * Those are critical methods where we need to understand arguments passed to them, the rest is allowed to be more dynamic + * - this list reflects what is implemented in QueryResultTypeWalker + */ + private const METHODS_AFFECTING_RESULT_TYPE = [ + 'add', + 'select', + 'addselect', + 'from', + 'join', + 'innerjoin', + 'leftjoin', + 'indexby', + ]; + /** @var ObjectMetadataResolver */ private $objectMetadataResolver; @@ -139,6 +154,9 @@ public function getTypeFromMethodCall( try { $args = $this->argumentsProcessor->processArgs($scope, $methodName, $calledMethodCall->getArgs()); } catch (DynamicQueryBuilderArgumentException $e) { + if (!in_array($lowerMethodName, self::METHODS_AFFECTING_RESULT_TYPE, true)) { + continue; + } return $defaultReturnType; } diff --git a/tests/Rules/Doctrine/ORM/QueryBuilderDqlRuleTest.php b/tests/Rules/Doctrine/ORM/QueryBuilderDqlRuleTest.php index b0e076b6..bbb6e1ca 100644 --- a/tests/Rules/Doctrine/ORM/QueryBuilderDqlRuleTest.php +++ b/tests/Rules/Doctrine/ORM/QueryBuilderDqlRuleTest.php @@ -134,6 +134,19 @@ public function testBranchingPerformance(): void ]); } + public function testDynamicWhere(): void + { + $this->analyse([__DIR__ . '/data/query-builder-dynamic.php'], [ + ['Could not analyse QueryBuilder with dynamic arguments.', 40], + ['Could not analyse QueryBuilder with dynamic arguments.', 45], + ['Could not analyse QueryBuilder with dynamic arguments.', 51], + ['Could not analyse QueryBuilder with dynamic arguments.', 56], + ['Could not analyse QueryBuilder with dynamic arguments.', 61], + ['Could not analyse QueryBuilder with dynamic arguments.', 66], + ['Could not analyse QueryBuilder with dynamic arguments.', 71], + ]); + } + public static function getAdditionalConfigFiles(): array { return [ diff --git a/tests/Rules/Doctrine/ORM/data/query-builder-dynamic.php b/tests/Rules/Doctrine/ORM/data/query-builder-dynamic.php new file mode 100644 index 00000000..c4418eee --- /dev/null +++ b/tests/Rules/Doctrine/ORM/data/query-builder-dynamic.php @@ -0,0 +1,78 @@ +createQueryBuilder() + ->select('m') + ->from(MyEntity::class, 'm') + ->andWhere($and) + ->setParameter($string, $string) + ->setParameters([$string]) + ->orWhere($string) + ->addOrderBy($string) + ->addGroupBy($string) + ->addCriteria($criteria) + ->getQuery(); + + $em->createQueryBuilder() + ->select('m') + ->add('from', new From(MyEntity::class, 'm', null), true) + ->where($string) + ->orderBy($string) + ->groupBy($string) + ->getQuery(); + + // all below are disallowed dynamic + $em->createQueryBuilder() + ->select('m') + ->from($string, 'm') + ->getQuery(); + + $em->createQueryBuilder() + ->select('m') + ->from(MyEntity::class, 'm') + ->indexBy($string, $string) + ->getQuery(); + + $em->createQueryBuilder() + ->select('m') + ->from(MyEntity::class, 'm', $string) + ->getQuery(); + + $em->createQueryBuilder() + ->select([$string]) + ->from(MyEntity::class, 'm') + ->getQuery(); + + $em->createQueryBuilder() + ->select(['m']) + ->from(MyEntity::class, $string) + ->getQuery(); + + $em->createQueryBuilder() + ->addSelect($string) + ->from(MyEntity::class, 'm') + ->getQuery(); + + $em->createQueryBuilder() + ->addSelect('m') + ->from(MyEntity::class, 'm') + ->join($string, $string) + ->getQuery(); + } + +} diff --git a/tests/Type/Doctrine/data/QueryResult/queryBuilderGetQuery.php b/tests/Type/Doctrine/data/QueryResult/queryBuilderGetQuery.php index a913262b..0fc6ae02 100644 --- a/tests/Type/Doctrine/data/QueryResult/queryBuilderGetQuery.php +++ b/tests/Type/Doctrine/data/QueryResult/queryBuilderGetQuery.php @@ -2,8 +2,11 @@ namespace QueryResult\CreateQuery; +use Doctrine\Common\Collections\Criteria; use Doctrine\ORM\AbstractQuery; use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\Query\Expr\Andx; +use Doctrine\ORM\Query\Expr\From; use Doctrine\ORM\QueryBuilder; use QueryResult\Entities\Many; use function PHPStan\Testing\assertType; @@ -135,7 +138,99 @@ public function testQueryResultTypeIsVoidWithDeleteOrUpdate(EntityManagerInterfa ->getQuery(); assertType('Doctrine\ORM\Query', $query); + } + + + public function testDynamicMethodCall( + EntityManagerInterface $em, + Andx $and, + Criteria $criteria, + string $string + ): void + { + $result = $em->createQueryBuilder() + ->select('m') + ->from(Many::class, 'm') + ->andWhere($and) + ->setParameter($string, $string) + ->setParameters([$string]) + ->orWhere($string) + ->addOrderBy($string) + ->addGroupBy($string) + ->addCriteria($criteria) + ->getQuery() + ->getResult(); + + assertType('list', $result); + + $result = $em->createQueryBuilder() + ->select(['m.stringNullColumn']) + ->add('from', new From(Many::class, 'm', null), true) + ->where($string) + ->orderBy($string) + ->groupBy($string) + ->getQuery() + ->getResult(); + + assertType('list', $result); + + $result = $em->createQueryBuilder() + ->select(['m.intColumn', 'm.stringNullColumn']) + ->from($string, 'm') + ->getQuery() + ->getResult(); + + assertType('mixed', $result); + + $result = $em->createQueryBuilder() + ->select(['m.intColumn', 'm.stringNullColumn']) + ->from(Many::class, 'm') + ->indexBy($string, $string) + ->getQuery() + ->getResult(); + + assertType('mixed', $result); + + $result = $em->createQueryBuilder() + ->select('m') + ->from(Many::class, 'm', $string) + ->getQuery() + ->getResult(); + + assertType('mixed', $result); + + $result = $em->createQueryBuilder() + ->select([$string, 'm.stringNullColumn']) + ->from(Many::class, 'm') + ->getQuery() + ->getResult(); + + assertType('mixed', $result); + + $result = $em->createQueryBuilder() + ->select(['m.stringNullColumn']) + ->from(Many::class, $string) + ->getQuery() + ->getResult(); + + assertType('mixed', $result); + + $result = $em->createQueryBuilder() + ->addSelect($string) + ->from(Many::class, 'm') + ->getQuery() + ->getResult(); + + assertType('mixed', $result); + + $result = $em->createQueryBuilder() + ->addSelect('m') + ->from(Many::class, 'm') + ->join($string, $string) + ->getQuery() + ->getResult(); + assertType('mixed', $result); } public function testQueryTypeIsInferredOnAcrossMethods(EntityManagerInterface $em): void From 0aca96d0cba696b5d99541c7adc88a154a6024a9 Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Thu, 16 Nov 2023 17:12:34 +0100 Subject: [PATCH 2/2] Add descriptor for AsciiStringType --- extension.neon | 3 ++ .../Doctrine/Descriptors/AsciiStringType.php | 31 +++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 src/Type/Doctrine/Descriptors/AsciiStringType.php diff --git a/extension.neon b/extension.neon index ce08b076..6412fe36 100644 --- a/extension.neon +++ b/extension.neon @@ -278,6 +278,9 @@ services: - class: PHPStan\Type\Doctrine\Descriptors\ArrayType tags: [phpstan.doctrine.typeDescriptor] + - + class: PHPStan\Type\Doctrine\Descriptors\AsciiStringType + tags: [phpstan.doctrine.typeDescriptor] - class: PHPStan\Type\Doctrine\Descriptors\BigIntType tags: [phpstan.doctrine.typeDescriptor] diff --git a/src/Type/Doctrine/Descriptors/AsciiStringType.php b/src/Type/Doctrine/Descriptors/AsciiStringType.php new file mode 100644 index 00000000..fbee4d12 --- /dev/null +++ b/src/Type/Doctrine/Descriptors/AsciiStringType.php @@ -0,0 +1,31 @@ +