diff --git a/extension.neon b/extension.neon index 2d7230bb..863d69ae 100644 --- a/extension.neon +++ b/extension.neon @@ -169,11 +169,6 @@ services: descendIntoOtherMethods: %doctrine.searchOtherMethodsForQueryBuilderBeginning% parser: @defaultAnalysisParser - - - class: PHPStan\Type\Doctrine\QueryBuilder\ReturnQueryBuilderExpressionTypeResolverExtension - tags: - - phpstan.broker.expressionTypeResolverExtension - - class: PHPStan\Stubs\Doctrine\StubFilesExtensionLoader tags: diff --git a/rules.neon b/rules.neon index 7545b956..07d41f2d 100644 --- a/rules.neon +++ b/rules.neon @@ -35,6 +35,7 @@ services: class: PHPStan\Rules\Doctrine\ORM\QueryBuilderDqlRule arguments: reportDynamicQueryBuilders: %doctrine.reportDynamicQueryBuilders% + searchOtherMethodsForQueryBuilderBeginning: %doctrine.searchOtherMethodsForQueryBuilderBeginning% tags: - phpstan.rules.rule - diff --git a/src/Rules/Doctrine/ORM/QueryBuilderDqlRule.php b/src/Rules/Doctrine/ORM/QueryBuilderDqlRule.php index 5e0030cd..a3186f60 100644 --- a/src/Rules/Doctrine/ORM/QueryBuilderDqlRule.php +++ b/src/Rules/Doctrine/ORM/QueryBuilderDqlRule.php @@ -12,6 +12,7 @@ use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\Doctrine\DoctrineTypeUtils; use PHPStan\Type\Doctrine\ObjectMetadataResolver; +use PHPStan\Type\Doctrine\QueryBuilder\OtherMethodQueryBuilderParser; use PHPStan\Type\ObjectType; use PHPStan\Type\TypeUtils; use Throwable; @@ -31,13 +32,23 @@ class QueryBuilderDqlRule implements Rule /** @var bool */ private $reportDynamicQueryBuilders; + /** @var OtherMethodQueryBuilderParser */ + private $otherMethodQueryBuilderParser; + + /** @var bool */ + private $searchOtherMethodsForQueryBuilderBeginning; + public function __construct( ObjectMetadataResolver $objectMetadataResolver, - bool $reportDynamicQueryBuilders + OtherMethodQueryBuilderParser $otherMethodQueryBuilderParser, + bool $reportDynamicQueryBuilders, + bool $searchOtherMethodsForQueryBuilderBeginning ) { $this->objectMetadataResolver = $objectMetadataResolver; + $this->otherMethodQueryBuilderParser = $otherMethodQueryBuilderParser; $this->reportDynamicQueryBuilders = $reportDynamicQueryBuilders; + $this->searchOtherMethodsForQueryBuilderBeginning = $searchOtherMethodsForQueryBuilderBeginning; } public function getNodeType(): string @@ -58,6 +69,14 @@ public function processNode(Node $node, Scope $scope): array $calledOnType = $scope->getType($node->var); $queryBuilderTypes = DoctrineTypeUtils::getQueryBuilderTypes($calledOnType); if (count($queryBuilderTypes) === 0) { + + if ($this->searchOtherMethodsForQueryBuilderBeginning) { + $queryBuilderTypes = $this->otherMethodQueryBuilderParser->getQueryBuilderTypes($scope, $node); + if (count($queryBuilderTypes) !== 0) { + return []; + } + } + if ( $this->reportDynamicQueryBuilders && (new ObjectType('Doctrine\ORM\QueryBuilder'))->isSuperTypeOf($calledOnType)->yes() diff --git a/src/Type/Doctrine/QueryBuilder/OtherMethodQueryBuilderParser.php b/src/Type/Doctrine/QueryBuilder/OtherMethodQueryBuilderParser.php index fccb8fdd..9e08de40 100644 --- a/src/Type/Doctrine/QueryBuilder/OtherMethodQueryBuilderParser.php +++ b/src/Type/Doctrine/QueryBuilder/OtherMethodQueryBuilderParser.php @@ -3,6 +3,8 @@ namespace PHPStan\Type\Doctrine\QueryBuilder; use PhpParser\Node; +use PhpParser\Node\Expr\MethodCall; +use PhpParser\Node\Identifier; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; @@ -15,12 +17,13 @@ use PHPStan\Analyser\ScopeFactory; use PHPStan\DependencyInjection\Container; use PHPStan\Parser\Parser; -use PHPStan\Reflection\MethodReflection; +use PHPStan\Reflection\ReflectionProvider; use PHPStan\Type\Generic\TemplateTypeMap; use PHPStan\Type\IntersectionType; use PHPStan\Type\Type; use PHPStan\Type\TypeTraverser; use PHPStan\Type\UnionType; +use function count; use function is_array; class OtherMethodQueryBuilderParser @@ -29,28 +32,61 @@ class OtherMethodQueryBuilderParser /** @var bool */ private $descendIntoOtherMethods; + /** @var ReflectionProvider */ + private $reflectionProvider; + /** @var Parser */ private $parser; /** @var Container */ private $container; - public function __construct(bool $descendIntoOtherMethods, Parser $parser, Container $container) + public function __construct(bool $descendIntoOtherMethods, ReflectionProvider $reflectionProvider, Parser $parser, Container $container) { $this->descendIntoOtherMethods = $descendIntoOtherMethods; + $this->reflectionProvider = $reflectionProvider; $this->parser = $parser; $this->container = $container; } /** - * @return list + * @return QueryBuilderType[] */ - public function findQueryBuilderTypesInCalledMethod(Scope $scope, MethodReflection $methodReflection): array + public function getQueryBuilderTypes(Scope $scope, MethodCall $methodCall): array { - if (!$this->descendIntoOtherMethods) { + if (!$this->descendIntoOtherMethods || !$methodCall->var instanceof MethodCall) { + return []; + } + + return $this->findQueryBuilderTypesInCalledMethod($scope, $methodCall->var); + } + /** + * @return QueryBuilderType[] + */ + private function findQueryBuilderTypesInCalledMethod(Scope $scope, MethodCall $methodCall): array + { + $methodCalledOnType = $scope->getType($methodCall->var); + if (!$methodCall->name instanceof Identifier) { + return []; + } + + $methodCalledOnTypeClassNames = $methodCalledOnType->getObjectClassNames(); + + if (count($methodCalledOnTypeClassNames) !== 1) { + return []; + } + + if (!$this->reflectionProvider->hasClass($methodCalledOnTypeClassNames[0])) { + return []; + } + + $classReflection = $this->reflectionProvider->getClass($methodCalledOnTypeClassNames[0]); + $methodName = $methodCall->name->toString(); + if (!$classReflection->hasNativeMethod($methodName)) { return []; } + $methodReflection = $classReflection->getNativeMethod($methodName); $fileName = $methodReflection->getDeclaringClass()->getFileName(); if ($fileName === null) { return []; diff --git a/src/Type/Doctrine/QueryBuilder/QueryBuilderGetQueryDynamicReturnTypeExtension.php b/src/Type/Doctrine/QueryBuilder/QueryBuilderGetQueryDynamicReturnTypeExtension.php index 1d0a1809..d2f8fd55 100644 --- a/src/Type/Doctrine/QueryBuilder/QueryBuilderGetQueryDynamicReturnTypeExtension.php +++ b/src/Type/Doctrine/QueryBuilder/QueryBuilderGetQueryDynamicReturnTypeExtension.php @@ -66,17 +66,22 @@ class QueryBuilderGetQueryDynamicReturnTypeExtension implements DynamicMethodRet /** @var DescriptorRegistry */ private $descriptorRegistry; + /** @var OtherMethodQueryBuilderParser */ + private $otherMethodQueryBuilderParser; + public function __construct( ObjectMetadataResolver $objectMetadataResolver, ArgumentsProcessor $argumentsProcessor, ?string $queryBuilderClass, - DescriptorRegistry $descriptorRegistry + DescriptorRegistry $descriptorRegistry, + OtherMethodQueryBuilderParser $otherMethodQueryBuilderParser ) { $this->objectMetadataResolver = $objectMetadataResolver; $this->argumentsProcessor = $argumentsProcessor; $this->queryBuilderClass = $queryBuilderClass; $this->descriptorRegistry = $descriptorRegistry; + $this->otherMethodQueryBuilderParser = $otherMethodQueryBuilderParser; } public function getClass(): string @@ -103,7 +108,10 @@ public function getTypeFromMethodCall( )->getReturnType(); $queryBuilderTypes = DoctrineTypeUtils::getQueryBuilderTypes($calledOnType); if (count($queryBuilderTypes) === 0) { - return $defaultReturnType; + $queryBuilderTypes = $this->otherMethodQueryBuilderParser->getQueryBuilderTypes($scope, $methodCall); + if (count($queryBuilderTypes) === 0) { + return $defaultReturnType; + } } $objectManager = $this->objectMetadataResolver->getObjectManager(); diff --git a/src/Type/Doctrine/QueryBuilder/QueryBuilderMethodDynamicReturnTypeExtension.php b/src/Type/Doctrine/QueryBuilder/QueryBuilderMethodDynamicReturnTypeExtension.php index cb76ba29..32b437fa 100644 --- a/src/Type/Doctrine/QueryBuilder/QueryBuilderMethodDynamicReturnTypeExtension.php +++ b/src/Type/Doctrine/QueryBuilder/QueryBuilderMethodDynamicReturnTypeExtension.php @@ -26,11 +26,16 @@ class QueryBuilderMethodDynamicReturnTypeExtension implements DynamicMethodRetur /** @var string|null */ private $queryBuilderClass; + /** @var OtherMethodQueryBuilderParser */ + private $otherMethodQueryBuilderParser; + public function __construct( - ?string $queryBuilderClass + ?string $queryBuilderClass, + OtherMethodQueryBuilderParser $otherMethodQueryBuilderParser ) { $this->queryBuilderClass = $queryBuilderClass; + $this->otherMethodQueryBuilderParser = $otherMethodQueryBuilderParser; } public function getClass(): string @@ -69,7 +74,10 @@ public function getTypeFromMethodCall( $queryBuilderTypes = DoctrineTypeUtils::getQueryBuilderTypes($calledOnType); if (count($queryBuilderTypes) === 0) { - return $calledOnType; + $queryBuilderTypes = $this->otherMethodQueryBuilderParser->getQueryBuilderTypes($scope, $methodCall); + if (count($queryBuilderTypes) === 0) { + return $calledOnType; + } } if (count($queryBuilderTypes) > self::MAX_COMBINATIONS) { diff --git a/src/Type/Doctrine/QueryBuilder/ReturnQueryBuilderExpressionTypeResolverExtension.php b/src/Type/Doctrine/QueryBuilder/ReturnQueryBuilderExpressionTypeResolverExtension.php deleted file mode 100644 index 2f780f22..00000000 --- a/src/Type/Doctrine/QueryBuilder/ReturnQueryBuilderExpressionTypeResolverExtension.php +++ /dev/null @@ -1,103 +0,0 @@ -otherMethodQueryBuilderParser = $otherMethodQueryBuilderParser; - } - - public function getType(Expr $expr, Scope $scope): ?Type - { - if (!$expr instanceof MethodCall && !$expr instanceof StaticCall) { - return null; - } - - if ($expr->isFirstClassCallable()) { - return null; - } - - $methodReflection = $this->getMethodReflection($expr, $scope); - - if ($methodReflection === null) { - return null; - } - - $returnType = ParametersAcceptorSelector::selectFromArgs($scope, $expr->getArgs(), $methodReflection->getVariants())->getReturnType(); - - $returnsQueryBuilder = (new ObjectType(QueryBuilder::class))->isSuperTypeOf($returnType)->yes(); - - if (!$returnsQueryBuilder) { - return null; - } - - $queryBuilderTypes = $this->otherMethodQueryBuilderParser->findQueryBuilderTypesInCalledMethod($scope, $methodReflection); - if (count($queryBuilderTypes) === 0) { - return null; - } - - return TypeCombinator::union(...$queryBuilderTypes); - } - - /** - * @param StaticCall|MethodCall $call - */ - private function getMethodReflection(CallLike $call, Scope $scope): ?MethodReflection - { - if (!$call->name instanceof Identifier) { - return null; - } - - if ($call instanceof MethodCall) { - $callerType = $scope->getType($call->var); - } else { - if (!$call->class instanceof Name) { - return null; - } - $callerType = $scope->resolveTypeByName($call->class); - } - - $methodName = $call->name->name; - - foreach ($callerType->getObjectClassReflections() as $callerClassReflection) { - if ($callerClassReflection->is(QueryBuilder::class)) { - return null; // covered by QueryBuilderMethodDynamicReturnTypeExtension - } - if ($callerClassReflection->is(EntityRepository::class) && $methodName === 'createQueryBuilder') { - return null; // covered by EntityRepositoryCreateQueryBuilderDynamicReturnTypeExtension - } - if ($callerClassReflection->is(EntityManagerInterface::class) && $methodName === 'createQueryBuilder') { - return null; // no need to dive there - } - } - - return $scope->getMethodReflection($callerType, $methodName); - } - -} diff --git a/tests/Rules/Doctrine/ORM/QueryBuilderDqlRuleSlowTest.php b/tests/Rules/Doctrine/ORM/QueryBuilderDqlRuleSlowTest.php index 7c4f4d40..655b89c7 100644 --- a/tests/Rules/Doctrine/ORM/QueryBuilderDqlRuleSlowTest.php +++ b/tests/Rules/Doctrine/ORM/QueryBuilderDqlRuleSlowTest.php @@ -5,6 +5,7 @@ use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; use PHPStan\Type\Doctrine\ObjectMetadataResolver; +use PHPStan\Type\Doctrine\QueryBuilder\OtherMethodQueryBuilderParser; /** * @extends RuleTestCase @@ -16,6 +17,8 @@ protected function getRule(): Rule { return new QueryBuilderDqlRule( new ObjectMetadataResolver(__DIR__ . '/entity-manager.php', __DIR__ . '/../../../../tmp'), + self::getContainer()->getByType(OtherMethodQueryBuilderParser::class), + true, true ); } diff --git a/tests/Rules/Doctrine/ORM/QueryBuilderDqlRuleTest.php b/tests/Rules/Doctrine/ORM/QueryBuilderDqlRuleTest.php index 37124635..67b94c8f 100644 --- a/tests/Rules/Doctrine/ORM/QueryBuilderDqlRuleTest.php +++ b/tests/Rules/Doctrine/ORM/QueryBuilderDqlRuleTest.php @@ -5,6 +5,7 @@ use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; use PHPStan\Type\Doctrine\ObjectMetadataResolver; +use PHPStan\Type\Doctrine\QueryBuilder\OtherMethodQueryBuilderParser; /** * @extends RuleTestCase @@ -16,6 +17,8 @@ protected function getRule(): Rule { return new QueryBuilderDqlRule( new ObjectMetadataResolver(__DIR__ . '/entity-manager.php', __DIR__ . '/../../../../tmp'), + self::getContainer()->getByType(OtherMethodQueryBuilderParser::class), + true, true ); } diff --git a/tests/Rules/Doctrine/ORM/data/query-builder-dql.php b/tests/Rules/Doctrine/ORM/data/query-builder-dql.php index 49aa95f2..2f6f5953 100644 --- a/tests/Rules/Doctrine/ORM/data/query-builder-dql.php +++ b/tests/Rules/Doctrine/ORM/data/query-builder-dql.php @@ -84,14 +84,14 @@ public function selectArray(): void ->getQuery(); } - public function analyseQueryBuilderOtherMethodBeginning(): void + public function analyseQueryBuilderUnknownBeginning(): void { $this->createQb()->getQuery(); } private function createQb(): \Doctrine\ORM\QueryBuilder { - return $this->entityManager->createQueryBuilder()->select('e')->from(MyEntity::class, 'e'); + return $this->entityManager->createQueryBuilder(); } public function analyseQueryBuilderDynamicArgs(string $entity): void diff --git a/tests/Type/Doctrine/QueryBuilder/ReturnQueryBuilderExpressionTypeResolverExtensionTest.php b/tests/Type/Doctrine/QueryBuilder/ReturnQueryBuilderExpressionTypeResolverExtensionTest.php deleted file mode 100644 index e62fd054..00000000 --- a/tests/Type/Doctrine/QueryBuilder/ReturnQueryBuilderExpressionTypeResolverExtensionTest.php +++ /dev/null @@ -1,35 +0,0 @@ - */ - public function dataFileAsserts(): iterable - { - yield from $this->gatherAssertTypes(__DIR__ . '/../data/QueryResult/queryBuilderExpressionTypeResolver.php'); - } - - /** - * @dataProvider dataFileAsserts - * @param mixed ...$args - */ - public function testFileAsserts( - string $assertType, - string $file, - ...$args - ): void - { - $this->assertFileAsserts($assertType, $file, ...$args); - } - - /** @return string[] */ - public static function getAdditionalConfigFiles(): array - { - return [__DIR__ . '/../data/QueryResult/config.neon']; - } - -} diff --git a/tests/Type/Doctrine/data/QueryResult/queryBuilderExpressionTypeResolver.php b/tests/Type/Doctrine/data/QueryResult/queryBuilderExpressionTypeResolver.php deleted file mode 100644 index 9fe7bcd1..00000000 --- a/tests/Type/Doctrine/data/QueryResult/queryBuilderExpressionTypeResolver.php +++ /dev/null @@ -1,110 +0,0 @@ -= 8.1 - -namespace QueryResult\CreateQuery; - -use Doctrine\Common\Collections\Criteria; -use Doctrine\ORM\AbstractQuery; -use Doctrine\ORM\EntityManagerInterface; -use Doctrine\ORM\EntityRepository; -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; - -class QueryBuilderExpressionTypeResolverTest -{ - - /** - * @var MyRepository - */ - private $myRepository; - - public function testQueryTypeIsInferredOnAcrossMethods(EntityManagerInterface $em): void - { - $query = $this->getQueryBuilder($em)->getQuery(); - $branchingQuery = $this->getBranchingQueryBuilder($em)->getQuery(); - - assertType('Doctrine\ORM\Query', $query); - assertType('Doctrine\ORM\Query', $branchingQuery); - } - - public function testQueryTypeIsInferredOnAcrossMethodsEvenWhenVariableAssignmentIsUsed(EntityManagerInterface $em): void - { - $queryBuilder = $this->getQueryBuilder($em); - - assertType('Doctrine\ORM\Query', $queryBuilder->getQuery()); - } - - public function testQueryBuilderPassedElsewhereNotTracked(EntityManagerInterface $em): void - { - $queryBuilder = $this->getQueryBuilder($em); - $queryBuilder->indexBy('m', 'm.stringColumn'); - - $this->adjustQueryBuilderToIndexByInt($queryBuilder); - - assertType('Doctrine\ORM\Query', $queryBuilder->getQuery()); - } - - public function testDiveIntoCustomEntityRepository(EntityManagerInterface $em): void - { - $queryBuilder = $this->myRepository->getCustomQueryBuilder($em); - - assertType('Doctrine\ORM\Query', $queryBuilder->getQuery()); - } - - - public function testStaticCallWorksToo(EntityManagerInterface $em): void - { - $queryBuilder = self::getStaticQueryBuilder($em); - - assertType('Doctrine\ORM\Query', $queryBuilder->getQuery()); - } - - public function testFirstClassCallableDoesNotFail(EntityManagerInterface $em): void - { - $this->getQueryBuilder(...); - } - - private function adjustQueryBuilderToIndexByInt(QueryBuilder $qb): void - { - $qb->indexBy('m', 'm.intColumn'); - } - - private function getQueryBuilder(EntityManagerInterface $em): QueryBuilder - { - return $em->createQueryBuilder() - ->select('m') - ->from(Many::class, 'm'); - } - - private static function getStaticQueryBuilder(EntityManagerInterface $em): QueryBuilder - { - return $em->createQueryBuilder() - ->select('m') - ->from(Many::class, 'm'); - } - - private function getBranchingQueryBuilder(EntityManagerInterface $em): QueryBuilder - { - $queryBuilder = $em->createQueryBuilder() - ->select('m') - ->from(Many::class, 'm'); - - if (random_int(0, 1) === 1) { - $queryBuilder->andWhere('m.intColumn = 1'); - } - - return $queryBuilder; - } -} - -class MyRepository extends EntityRepository { - - private function getCustomQueryBuilder(EntityManagerInterface $em): QueryBuilder - { - return $em->createQueryBuilder() - ->select('m') - ->from(Many::class, 'm'); - } -} diff --git a/tests/Type/Doctrine/data/QueryResult/queryBuilderGetQuery.php b/tests/Type/Doctrine/data/QueryResult/queryBuilderGetQuery.php index d4357def..e9c63efb 100644 --- a/tests/Type/Doctrine/data/QueryResult/queryBuilderGetQuery.php +++ b/tests/Type/Doctrine/data/QueryResult/queryBuilderGetQuery.php @@ -233,7 +233,6 @@ public function testDynamicMethodCall( assertType('mixed', $result); } - /** * @param class-string $many */ @@ -277,4 +276,33 @@ public function testNonEntityClassString(EntityManagerInterface $em, string $cla assertType('mixed', $result); } + public function testQueryTypeIsInferredOnAcrossMethods(EntityManagerInterface $em): void + { + $query = $this->getQueryBuilder($em)->getQuery(); + $branchingQuery = $this->getBranchingQueryBuilder($em)->getQuery(); + + assertType('Doctrine\ORM\Query', $query); + assertType('Doctrine\ORM\Query', $branchingQuery); + } + + private function getQueryBuilder(EntityManagerInterface $em): QueryBuilder + { + return $em->createQueryBuilder() + ->select('m') + ->from(Many::class, 'm'); + } + + private function getBranchingQueryBuilder(EntityManagerInterface $em): QueryBuilder + { + $queryBuilder = $em->createQueryBuilder() + ->select('m') + ->from(Many::class, 'm'); + + if (random_int(0, 1) === 1) { + $queryBuilder->andWhere('m.intColumn = 1'); + } + + return $queryBuilder; + } + }