[doctrine] Skip unconvertible builders in RequireQueryBuilderOnRepositoryRule

README.md

@@ -1583,13 +1583,47 @@ final class Product {}
 
 ### RequireQueryBuilderOnRepositoryRule
 
-Prevents using `$entityManager->createQueryBuilder('...')`,  use `$repository->createQueryBuilder()` as safer.
+Prevents using `$entityManager->createQueryBuilder()` inside a repository class, use `$repository->createQueryBuilder()` as safer.
+
+Builders that the repository shortcut cannot express are skipped: `update()`/`delete()` builders and `from()` on another entity than the repository's own one (e.g. cross-entity subqueries).
 
 ```yaml
 rules:
     - Symplify\PHPStanRules\Rules\Doctrine\RequireQueryBuilderOnRepositoryRule
 ```
 
+```php
+final class SomeRepository extends EntityRepository
+{
+    public function getSome(): array
+    {
+        return $this->getEntityManager()->createQueryBuilder()
+            ->select('s')
+            ->from(SomeEntity::class, 's')
+            ->getQuery()
+            ->getResult();
+    }
+}
+```
+
+:x:
+
+<br>
+
+```php
+final class SomeRepository extends EntityRepository
+{
+    public function getSome(): array
+    {
+        return $this->createQueryBuilder('s')
+            ->getQuery()
+            ->getResult();
+    }
+}
+```
+
+:+1:
+
 <br>
 
 ### NoGetRepositoryOutsideServiceRule

src/Rules/Doctrine/RequireQueryBuilderOnRepositoryRule.php

@@ -5,8 +5,16 @@
 namespace Symplify\PHPStanRules\Rules\Doctrine;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\ClassConstFetch;
 use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Name;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\NodeFinder;
 use PHPStan\Analyser\Scope;
+use PHPStan\Node\InClassMethodNode;
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Rules\Rule;
 use PHPStan\Rules\RuleErrorBuilder;
@@ -18,24 +26,38 @@
 use Symplify\PHPStanRules\Helper\NamingHelper;
 
 /**
- * @implements Rule<MethodCall>
+ * @implements Rule<InClassMethodNode>
  * @see \Symplify\PHPStanRules\Tests\Rules\Doctrine\RequireQueryBuilderOnRepositoryRule\RequireQueryBuilderOnRepositoryRuleTest
  */
-final class RequireQueryBuilderOnRepositoryRule implements Rule
+final readonly class RequireQueryBuilderOnRepositoryRule implements Rule
 {
-    public const string ERROR_MESSAGE = 'Avoid calling ->createQueryBuilder() directly on EntityManager as it requires select() + from() calls with specific values. Use $repository->createQueryBuilder() to be safe instead';
+    public const string ERROR_MESSAGE = 'Avoid calling ->createQueryBuilder() directly on EntityManager inside a repository class, as it requires select() + from() calls with specific values. Use $repository->createQueryBuilder() to be safe instead';
+
+    /**
+     * UPDATE/DELETE builders are not plain SELECTs, the repository shortcut cannot express them
+     * @var string[]
+     */
+    private const array NON_SELECT_BUILDER_METHODS = ['update', 'delete'];
+
+    private NodeFinder $nodeFinder;
+
+    public function __construct()
+    {
+        $this->nodeFinder = new NodeFinder();
+    }
 
     public function getNodeType(): string
     {
-        return MethodCall::class;
+        return InClassMethodNode::class;
     }
 
     /**
-     * @param MethodCall $node
+     * @param InClassMethodNode $node
      */
     public function processNode(Node $node, Scope $scope): array
     {
-        if (! NamingHelper::isName($node->name, 'createQueryBuilder')) {
+        $classMethod = $node->getOriginalNode();
+        if ($classMethod->stmts === null) {
             return [];
         }
 
@@ -44,16 +66,34 @@ public function processNode(Node $node, Scope $scope): array
             return [];
         }
 
-        $callerType = $scope->getType($node->var);
-        if ($this->isValidRepositoryObjectType($callerType)) {
-            return [];
-        }
+        /** @var MethodCall[] $methodCalls */
+        $methodCalls = $this->nodeFinder->findInstanceOf($classMethod->stmts, MethodCall::class);
+
+        $repositoryEntityClassNames = $this->resolveRepositoryEntityClassNames($scope);
 
-        $identifierRuleError = RuleErrorBuilder::message(self::ERROR_MESSAGE)
-            ->identifier(DoctrineRuleIdentifier::REQUIRE_QUERY_BUILDER_ON_REPOSITORY)
-            ->build();
+        $ruleErrors = [];
+        foreach ($methodCalls as $methodCall) {
+            if (! NamingHelper::isName($methodCall->name, 'createQueryBuilder')) {
+                continue;
+            }
+
+            $callerType = $scope->getType($methodCall->var);
+            if ($this->isValidRepositoryObjectType($callerType)) {
+                continue;
+            }
 
-        return [$identifierRuleError];
+            // the query builder cannot be swapped for $this->createQueryBuilder()
+            if ($this->isUnconvertibleBuilder($methodCall, $methodCalls, $classMethod, $repositoryEntityClassNames, $scope)) {
+                continue;
+            }
+
+            $ruleErrors[] = RuleErrorBuilder::message(self::ERROR_MESSAGE)
+                ->identifier(DoctrineRuleIdentifier::REQUIRE_QUERY_BUILDER_ON_REPOSITORY)
+                ->line($methodCall->getStartLine())
+                ->build();
+        }
+
+        return $ruleErrors;
     }
 
     private function isInsideRepositoryClass(Scope $scope): bool
@@ -95,4 +135,177 @@ private function isValidRepositoryObjectType(Type $type): bool
 
         return $type->isInstanceOf(DoctrineClass::CONNECTION)->yes();
     }
+
+    /**
+     * @param MethodCall[] $allMethodCalls
+     * @param string[] $repositoryEntityClassNames
+     */
+    private function isUnconvertibleBuilder(
+        MethodCall $methodCall,
+        array $allMethodCalls,
+        ClassMethod $classMethod,
+        array $repositoryEntityClassNames,
+        Scope $scope
+    ): bool {
+        $builderMethodCalls = $this->collectBuilderMethodCalls($methodCall, $allMethodCalls, $classMethod);
+
+        foreach ($builderMethodCalls as $builderMethodCall) {
+            if (! $builderMethodCall->name instanceof Identifier) {
+                continue;
+            }
+
+            $calledMethodName = $builderMethodCall->name->toString();
+
+            if (in_array($calledMethodName, self::NON_SELECT_BUILDER_METHODS, true)) {
+                return true;
+            }
+
+            // from() on another entity than the repository's own one cannot use the repository shortcut
+            if ($calledMethodName === 'from' && ! $this->isFromOnRepositoryEntity(
+                $builderMethodCall,
+                $repositoryEntityClassNames,
+                $scope
+            )) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Collects every method call made on the query builder that this createQueryBuilder() call produces,
+     * both as a fluent chain and via an intermediate variable.
+     *
+     * @param MethodCall[] $allMethodCalls
+     * @return MethodCall[]
+     */
+    private function collectBuilderMethodCalls(
+        MethodCall $createQueryBuilderCall,
+        array $allMethodCalls,
+        ClassMethod $classMethod
+    ): array {
+        $builderMethodCalls = [];
+
+        foreach ($allMethodCalls as $methodCall) {
+            if ($methodCall !== $createQueryBuilderCall && $this->chainPassesThrough($methodCall, $createQueryBuilderCall)) {
+                $builderMethodCalls[] = $methodCall;
+            }
+        }
+
+        $assignedVariableName = $this->resolveAssignedVariableName($createQueryBuilderCall, $classMethod);
+        if ($assignedVariableName !== null) {
+            foreach ($allMethodCalls as $allMethodCall) {
+                if ($this->resolveRootVariableName($allMethodCall) === $assignedVariableName) {
+                    $builderMethodCalls[] = $allMethodCall;
+                }
+            }
+        }
+
+        return $builderMethodCalls;
+    }
+
+    private function chainPassesThrough(MethodCall $methodCall, MethodCall $targetMethodCall): bool
+    {
+        $current = $methodCall->var;
+        while ($current instanceof MethodCall) {
+            if ($current === $targetMethodCall) {
+                return true;
+            }
+
+            $current = $current->var;
+        }
+
+        return false;
+    }
+
+    private function resolveRootVariableName(MethodCall $methodCall): ?string
+    {
+        $current = $methodCall->var;
+        while ($current instanceof MethodCall) {
+            $current = $current->var;
+        }
+
+        if ($current instanceof Variable && is_string($current->name)) {
+            return $current->name;
+        }
+
+        return null;
+    }
+
+    private function resolveAssignedVariableName(MethodCall $methodCall, ClassMethod $classMethod): ?string
+    {
+        /** @var Assign[] $assigns */
+        $assigns = $this->nodeFinder->findInstanceOf((array) $classMethod->stmts, Assign::class);
+
+        foreach ($assigns as $assign) {
+            $isBuilderAssign = $assign->expr === $methodCall
+                || ($assign->expr instanceof MethodCall && $this->chainPassesThrough($assign->expr, $methodCall));
+
+            if (! $isBuilderAssign) {
+                continue;
+            }
+
+            if ($assign->var instanceof Variable && is_string($assign->var->name)) {
+                return $assign->var->name;
+            }
+        }
+
+        return null;
+    }
+
+    /**
+     * @param string[] $repositoryEntityClassNames
+     */
+    private function isFromOnRepositoryEntity(MethodCall $fromMethodCall, array $repositoryEntityClassNames, Scope $scope): bool
+    {
+        $args = $fromMethodCall->getArgs();
+        if (! isset($args[0])) {
+            return false;
+        }
+
+        $firstArg = $args[0]->value;
+        if (! $firstArg instanceof ClassConstFetch) {
+            return false;
+        }
+
+        if (! $firstArg->class instanceof Name) {
+            return false;
+        }
+
+        if (! $firstArg->name instanceof Identifier || $firstArg->name->toString() !== 'class') {
+            return false;
+        }
+
+        $fromClassName = $scope->resolveName($firstArg->class);
+
+        return in_array($fromClassName, $repositoryEntityClassNames, true);
+    }
+
+    /**
+     * @return string[]
+     */
+    private function resolveRepositoryEntityClassNames(Scope $scope): array
+    {
+        $classReflection = $scope->getClassReflection();
+        if (! $classReflection instanceof ClassReflection) {
+            return [];
+        }
+
+        $entityClassNames = [];
+        foreach ([DoctrineClass::ENTITY_REPOSITORY, DoctrineClass::DOCUMENT_REPOSITORY] as $repositoryBaseClass) {
+            $ancestorClassReflection = $classReflection->getAncestorWithClassName($repositoryBaseClass);
+            if (! $ancestorClassReflection instanceof ClassReflection) {
+                continue;
+            }
+
+            foreach ($ancestorClassReflection->getActiveTemplateTypeMap()->getTypes() as $templateType) {
+                foreach ($templateType->getObjectClassNames() as $objectClassName) {
+                    $entityClassNames[] = $objectClassName;
+                }
+            }
+        }
+
+        return array_unique($entityClassNames);
+    }
 }

stubs/Doctrine/ORM/EntityManagerInterface.php

@@ -14,5 +14,5 @@ interface EntityManagerInterface
      */
     public function getRepository(string $class): object;
 
-    public function createQueryBuilder();
+    public function createQueryBuilder(): QueryBuilder;
 }

stubs/Doctrine/ORM/EntityRepository.php

@@ -6,9 +6,12 @@
     return;
 }
 
+/**
+ * @template T of object
+ */
 class EntityRepository
 {
-    public function createQueryBuilder()
+    public function createQueryBuilder(string $alias, ?string $indexBy = null): QueryBuilder
     {
     }
 }

stubs/Doctrine/ORM/QueryBuilder.php

@@ -0,0 +1,38 @@
+<?php
+
+namespace Doctrine\ORM;
+
+if (class_exists('Doctrine\ORM\QueryBuilder')) {
+    return;
+}
+
+class QueryBuilder
+{
+    public function select($select = null): self
+    {
+    }
+
+    public function from(string $from, string $alias, ?string $indexBy = null): self
+    {
+    }
+
+    public function update(?string $update = null, ?string $alias = null): self
+    {
+    }
+
+    public function delete(?string $delete = null, ?string $alias = null): self
+    {
+    }
+
+    public function where($predicates): self
+    {
+    }
+
+    public function leftJoin($join, string $alias): self
+    {
+    }
+
+    public function getDQL(): string
+    {
+    }
+}