[doctrine] fix repository crash when variable classname is used (#280)

psalm · Oct 22, 2022 · 5dca178 · 5dca178
1 parent b9511a3
commit 5dca178
Show file tree

Hide file tree

Showing 6 changed files with 97 additions and 19 deletions.
diff --git a/src/Handler/DoctrineRepositoryHandler.php b/src/Handler/DoctrineRepositoryHandler.php
@@ -17,6 +17,8 @@
 use Psalm\SymfonyPsalmPlugin\Issue\RepositoryStringShortcut;
 use Psalm\Type\Atomic\TNamedObject;
 use Psalm\Type\Union;
+use ReflectionClass;
+use ReflectionException;
 
 class DoctrineRepositoryHandler implements AfterMethodCallAnalysisInterface, AfterClassLikeVisitInterface
 {
@@ -41,13 +43,17 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
                     $statements_source->getSuppressedIssues()
                 );
             } elseif ($entityName instanceof Expr\ClassConstFetch) {
-                /** @psalm-var class-string $className */
+                /** @psalm-var class-string|null $className */
                 $className = $entityName->class->getAttribute('resolvedName');
 
+                if (null === $className) {
+                    return;
+                }
+
                 try {
-                    $reflectionClass = new \ReflectionClass($className);
+                    $reflectionClass = new ReflectionClass($className);
 
-                    if (method_exists(\ReflectionClass::class, 'getAttributes')) {
+                    if (\PHP_VERSION_ID >= 80000 && method_exists(ReflectionClass::class, 'getAttributes')) {
                         $entityAttributes = $reflectionClass->getAttributes(EntityAnnotation::class);
 
                         foreach ($entityAttributes as $entityAttribute) {
@@ -70,7 +76,7 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
                             $event->setReturnTypeCandidate(new Union([new TNamedObject($entityAnnotation->repositoryClass)]));
                         }
                     }
-                } catch (\ReflectionException $e) {
+                } catch (ReflectionException $e) {
                 }
             }
         }

diff --git a/...e/acceptance/DoctrineQueryBuilder.feature → ...nce/doctrine/DoctrineQueryBuilder.feature b/...e/acceptance/DoctrineQueryBuilder.feature → ...nce/doctrine/DoctrineQueryBuilder.feature
diff --git a/tests/acceptance/acceptance/doctrine/RepositoryClass.feature b/tests/acceptance/acceptance/doctrine/RepositoryClass.feature
@@ -0,0 +1,53 @@
+@symfony-common
+Feature: RepositoryClass
+
+  Background:
+    Given I have issue handlers "UndefinedClass,UnusedVariable" suppressed
+    And I have Symfony plugin enabled
+    And I have the following code preamble
+      """
+      <?php
+      namespace RepositoryClass;
+
+      use Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine\Foo;
+      use Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine\FooRepository;
+      use Doctrine\ORM\EntityManagerInterface;
+      """
+
+  Scenario: The plugin can find correct repository class from entity
+    Given I have the following code
+      """
+      class SomeService
+      {
+        public function __construct(EntityManagerInterface $entityManager)
+        {
+          /** @psalm-trace $repository */
+          $repository = $entityManager->getRepository(Foo::class);
+        }
+      }
+      """
+    When I run Psalm
+    Then I see these errors
+      | Type   | Message                                                                     |
+      | Trace  | $repository: Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine\FooRepository  |
+    And I see no other errors
+
+  Scenario: Passing variable class does not crash the plugin
+    Given I have the following code
+      """
+      class SomeService
+      {
+        public function __construct(EntityManagerInterface $entityManager)
+        {
+          $entity = 'Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine\Foo';
+          /** @psalm-trace $repository */
+          $repository = $entityManager->getRepository($entity::class);
+        }
+      }
+      """
+    When I run Psalm
+    Then I see these errors
+      | Type          | Message                                                                                                  |
+      | Trace         | $repository: Doctrine\ORM\EntityRepository<object>                                                       |
+      | MixedArgument | Argument 1 of Doctrine\ORM\EntityManagerInterface::getRepository cannot be mixed, expecting class-string |
+    And I see no other errors
diff --git a/...ceptance/RepositoryStringShortcut.feature → ...doctrine/RepositoryStringShortcut.feature b/...ceptance/RepositoryStringShortcut.feature → ...doctrine/RepositoryStringShortcut.feature
@@ -5,27 +5,21 @@ Feature: RepositoryStringShortcut
   I need Psalm to check preferred repository syntax
 
   Background:
-    Given I have issue handlers "UndefinedClass,UnusedVariable" suppressed
+    Given I have issue handlers "ArgumentTypeCoercion,MixedArgument,UndefinedClass" suppressed
     And I have Symfony plugin enabled
     And I have the following code preamble
       """
       <?php
-      namespace Doctrine\ORM;
-      interface EntityManagerInterface
-      {
-        /**
-         * @param string $className
-         *
-         * @return void
-         */
-        public function getRepository($className);
-      }
+
+      namespace RepositoryStringShortcut;
+
+      use Doctrine\ORM\EntityManagerInterface;
+      use Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine\Foo;
       """
 
   Scenario: Asserting using 'AppBundle:Entity' syntax raises issue
     Given I have the following code
       """
-      use Doctrine\ORM\EntityManagerInterface;
       class SomeService
       {
         public function __construct(EntityManagerInterface $entityManager)
@@ -36,7 +30,7 @@ Feature: RepositoryStringShortcut
       """
     When I run Psalm
     Then I see these errors
-      | Type                       | Message                            |
+      | Type                      | Message                          |
       | RepositoryStringShortcut  | Use Entity::class syntax instead |
     And I see no other errors
 
@@ -47,7 +41,7 @@ Feature: RepositoryStringShortcut
       {
         public function __construct(EntityManagerInterface $entityManager)
         {
-          $entityManager->getRepository(Entity::class);
+          $entityManager->getRepository(Foo::class);
         }
       }
       """
@@ -61,7 +55,7 @@ Feature: RepositoryStringShortcut
       {
         public function __construct(EntityManagerInterface $entityManager)
         {
-          $className = 'App\Entity\EntityA';
+          $className = Foo::class;
           $entityManager->getRepository($className);
         }
       }

diff --git a/tests/fixture/Doctrine/Foo.php b/tests/fixture/Doctrine/Foo.php
@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine;
+
+use Doctrine\ORM\Mapping\Entity;
+
+/**
+ * @Entity(repositoryClass=FooRepository::class)
+ */
+class Foo
+{
+}
diff --git a/tests/fixture/Doctrine/FooRepository.php b/tests/fixture/Doctrine/FooRepository.php
@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine;
+
+use Doctrine\ORM\EntityRepository;
+
+class FooRepository extends EntityRepository
+{
+}