Skip to content

Commit

Permalink
Revert "Infer QueryBuilderType for any method returning QueryBuilder"
Browse files Browse the repository at this point in the history
This reverts commit a9d0aaf.
  • Loading branch information
ondrejmirtes committed Jan 16, 2024
1 parent 0edf5b0 commit ed96786
Show file tree
Hide file tree
Showing 13 changed files with 119 additions and 266 deletions.
5 changes: 0 additions & 5 deletions extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions rules.neon
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ services:
class: PHPStan\Rules\Doctrine\ORM\QueryBuilderDqlRule
arguments:
reportDynamicQueryBuilders: %doctrine.reportDynamicQueryBuilders%
searchOtherMethodsForQueryBuilderBeginning: %doctrine.searchOtherMethodsForQueryBuilderBeginning%
tags:
- phpstan.rules.rule
-
Expand Down
21 changes: 20 additions & 1 deletion src/Rules/Doctrine/ORM/QueryBuilderDqlRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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()
Expand Down
46 changes: 41 additions & 5 deletions src/Type/Doctrine/QueryBuilder/OtherMethodQueryBuilderParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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<QueryBuilderType>
* @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 [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down

This file was deleted.

3 changes: 3 additions & 0 deletions tests/Rules/Doctrine/ORM/QueryBuilderDqlRuleSlowTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<QueryBuilderDqlRule>
Expand All @@ -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
);
}
Expand Down
3 changes: 3 additions & 0 deletions tests/Rules/Doctrine/ORM/QueryBuilderDqlRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<QueryBuilderDqlRule>
Expand All @@ -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
);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/Rules/Doctrine/ORM/data/query-builder-dql.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit ed96786

Please sign in to comment.