improve RequireQueryBuilderOnRepositoryRule to report document manager and skip document repository (#222)

Author: TomasVotruba

src/Enum/DoctrineClass.php

@@ -35,4 +35,9 @@ final class DoctrineClass
      * @var string
      */
     public const CONNECTION = 'Doctrine\DBAL\Connection';
+
+    /**
+     * @var string
+     */
+    public const DOCUMENT_REPOSITORY = 'Doctrine\ODM\MongoDB\Repository\DocumentRepository';
 }

src/Rules/Doctrine/RequireQueryBuilderOnRepositoryRule.php

@@ -10,12 +10,15 @@
 use PHPStan\Rules\Rule;
 use PHPStan\Rules\RuleErrorBuilder;
 use PHPStan\Type\ObjectType;
+use PHPStan\Type\Type;
+use PHPStan\Type\UnionType;
 use Symplify\PHPStanRules\Enum\DoctrineClass;
 use Symplify\PHPStanRules\Enum\RuleIdentifier\DoctrineRuleIdentifier;
 use Symplify\PHPStanRules\Helper\NamingHelper;
 
 /**
  * @implements Rule<MethodCall>
+ * @see \Symplify\PHPStanRules\Tests\Rules\Doctrine\RequireQueryBuilderOnRepositoryRule\RequireQueryBuilderOnRepositoryRuleTest
  */
 final class RequireQueryBuilderOnRepositoryRule implements Rule
 {
@@ -39,16 +42,7 @@ public function processNode(Node $node, Scope $scope): array
         }
 
         $callerType = $scope->getType($node->var);
-        if (! $callerType instanceof ObjectType) {
-            return [];
-        }
-
-        // we safe as both select() + from() calls are made on the repository
-        if ($callerType->isInstanceOf(DoctrineClass::ENTITY_REPOSITORY)->yes()) {
-            return [];
-        }
-
-        if ($callerType->isInstanceOf(DoctrineClass::CONNECTION)->yes()) {
+        if ($this->isValidRepositoryObjectType($callerType)) {
             return [];
         }
 
@@ -58,4 +52,30 @@ public function processNode(Node $node, Scope $scope): array
 
         return [$identifierRuleError];
     }
+
+    private function isValidRepositoryObjectType(Type $type): bool
+    {
+        if ($type instanceof UnionType) {
+            foreach ($type->getTypes() as $unionType) {
+                if ($this->isValidRepositoryObjectType($unionType)) {
+                    return true;
+                }
+            }
+        }
+
+        if (! $type instanceof ObjectType) {
+            return true;
+        }
+
+        // we safe as both select() + from() calls are made on the repository
+        if ($type->isInstanceOf(DoctrineClass::ENTITY_REPOSITORY)->yes()) {
+            return true;
+        }
+
+        if ($type->isInstanceOf(DoctrineClass::DOCUMENT_REPOSITORY)->yes()) {
+            return true;
+        }
+
+        return $type->isInstanceOf(DoctrineClass::CONNECTION)->yes();
+    }
 }

stubs/Doctrine/DBAL/Connection.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace Doctrine\DBAL;
+
+class Connection
+{
+    public function createQueryBuilder()
+    {
+    }
+}

stubs/Doctrine/ODM/MongoDB/DocumentManager.php

@@ -2,12 +2,27 @@
 
 namespace Doctrine\ODM\MongoDB;
 
+use Doctrine\ODM\MongoDB\Repository\DocumentRepository;
+use Doctrine\ODM\MongoDB\Repository\GridFSRepository;
+use Doctrine\ODM\MongoDB\Repository\ViewRepository;
+
 if (class_exists('Doctrine\ODM\MongoDB\DocumentManager')) {
     return;
 }
 
 abstract class DocumentManager
 {
+    /**
+     * Gets the repository for a document class.
+     *
+     * @param string $className The name of the Document.
+     * @psalm-param class-string<T> $className
+     *
+     * @return DocumentRepository|GridFSRepository|ViewRepository  The repository.
+     * @psalm-return DocumentRepository<T>|GridFSRepository<T>|ViewRepository<T>
+     *
+     * @template T of object
+     */
     public function getRepository(string $class)
     {
     }

stubs/Doctrine/ODM/MongoDB/Repository/DocumentRepository.php

@@ -8,4 +8,7 @@
 
 abstract class DocumentRepository
 {
+    public function getQueryBuilder()
+    {
+    }
 }

stubs/Doctrine/ODM/MongoDB/Repository/GridFSRepository.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Doctrine\ODM\MongoDB\Repository;
+
+class GridFSRepository
+{
+
+}

stubs/Doctrine/ODM/MongoDB/Repository/ViewRepository.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Doctrine\ODM\MongoDB\Repository;
+
+class ViewRepository
+{
+
+}

tests/Rules/Doctrine/RequireQueryBuilderOnRepositoryRule/Fixture/ReportOnDocumentManager.php

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Doctrine\RequireQueryBuilderOnRepositoryRule\Fixture;
+
+use Doctrine\ODM\MongoDB\DocumentManager;
+
+final class ReportOnDocumentManager
+{
+    public function process(DocumentManager $documentManager)
+    {
+        $someRepository = $documentManager->createQueryBuilder();
+    }
+}

tests/Rules/Doctrine/RequireQueryBuilderOnRepositoryRule/Fixture/SkipConnection.php

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Doctrine\RequireQueryBuilderOnRepositoryRule\Fixture;
+
+use Doctrine\DBAL\Connection;
+
+final class SkipConnection
+{
+    public function process(Connection $connection)
+    {
+        $someQueryBuilder = $connection->createQueryBuilder();
+    }
+}

tests/Rules/Doctrine/RequireQueryBuilderOnRepositoryRule/Fixture/SkipDocumentRepository.php

@@ -0,0 +1,17 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Doctrine\RequireQueryBuilderOnRepositoryRule\Fixture;
+
+use Doctrine\ODM\MongoDB\DocumentManager;
+use Symplify\PHPStanRules\Tests\Rules\Doctrine\RequireQueryBuilderOnRepositoryRule\Source\RandomEntity;
+
+final class SkipDocumentRepository
+{
+    public function process(DocumentManager $documentManager)
+    {
+        $someRepository = $documentManager->getRepository(RandomEntity::class)
+            ->createQueryBuilder();
+    }
+}

tests/Rules/Doctrine/RequireQueryBuilderOnRepositoryRule/RequireQueryBuilderOnRepositoryRuleTest.php

@@ -21,10 +21,16 @@ public function testRule(string $filePath, array $expectedErrorsWithLines): void
     public static function provideData(): Iterator
     {
         yield [__DIR__ . '/Fixture/SkipCreateQueryBuilderOnRepository.php', []];
+        yield [__DIR__ . '/Fixture/SkipDocumentRepository.php', []];
+        yield [__DIR__ . '/Fixture/SkipConnection.php', []];
 
         yield [__DIR__ . '/Fixture/ReportOnEntityManager.php', [
             [RequireQueryBuilderOnRepositoryRule::ERROR_MESSAGE, 14],
         ]];
+
+        yield [__DIR__ . '/Fixture/ReportOnDocumentManager.php', [
+            [RequireQueryBuilderOnRepositoryRule::ERROR_MESSAGE, 13],
+        ]];
     }
 
     protected function getRule(): Rule