Add native types to EntityRepository (#9515)

doctrine · Feb 17, 2022 · 6b33fa6 · 6b33fa6
1 parent 3a9cc3d
commit 6b33fa6
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 107 deletions.
diff --git a/lib/Doctrine/ORM/EntityRepository.php b/lib/Doctrine/ORM/EntityRepository.php
@@ -35,93 +35,65 @@
  */
 class EntityRepository implements ObjectRepository, Selectable
 {
-    /**
-     * @internal This property will be private in 3.0, call {@see getEntityName()} instead.
-     *
-     * @var string
-     */
-    protected $_entityName;
-
-    /**
-     * @internal This property will be private in 3.0, call {@see getEntityManager()} instead.
-     *
-     * @var EntityManagerInterface
-     */
-    protected $_em;
+    /** @psalm-var class-string<T> */
+    private string $entityName;
+    private static ?Inflector $inflector = null;
 
     /**
-     * @internal This property will be private in 3.0, call {@see getClassMetadata()} instead.
-     *
-     * @var ClassMetadata
+     * @psalm-param ClassMetadata<T> $class
      */
-    protected $_class;
-
-    /** @var Inflector|null */
-    private static $inflector;
-
-    public function __construct(EntityManagerInterface $em, ClassMetadata $class)
-    {
-        $this->_entityName = $class->name;
-        $this->_em         = $em;
-        $this->_class      = $class;
+    public function __construct(
+        private EntityManagerInterface $em,
+        private ClassMetadata $class
+    ) {
+        $this->entityName = $class->name;
     }
 
     /**
      * Creates a new QueryBuilder instance that is prepopulated for this entity name.
-     *
-     * @param string      $alias
-     * @param string|null $indexBy The index for the from.
-     *
-     * @return QueryBuilder
      */
-    public function createQueryBuilder($alias, $indexBy = null)
+    public function createQueryBuilder(string $alias, ?string $indexBy = null): QueryBuilder
     {
-        return $this->_em->createQueryBuilder()
+        return $this->em->createQueryBuilder()
             ->select($alias)
-            ->from($this->_entityName, $alias, $indexBy);
+            ->from($this->entityName, $alias, $indexBy);
     }
 
     /**
      * Creates a new result set mapping builder for this entity.
      *
      * The column naming strategy is "INCREMENT".
-     *
-     * @param string $alias
-     *
-     * @return ResultSetMappingBuilder
      */
-    public function createResultSetMappingBuilder($alias)
+    public function createResultSetMappingBuilder(string $alias): ResultSetMappingBuilder
     {
-        $rsm = new ResultSetMappingBuilder($this->_em, ResultSetMappingBuilder::COLUMN_RENAMING_INCREMENT);
-        $rsm->addRootEntityFromClassMetadata($this->_entityName, $alias);
+        $rsm = new ResultSetMappingBuilder($this->em, ResultSetMappingBuilder::COLUMN_RENAMING_INCREMENT);
+        $rsm->addRootEntityFromClassMetadata($this->entityName, $alias);
 
         return $rsm;
     }
 
     /**
      * Finds an entity by its primary key / identifier.
      *
-     * @param mixed    $id          The identifier.
-     * @param int|null $lockMode    One of the \Doctrine\DBAL\LockMode::* constants
-     *                              or NULL if no specific lock mode should be used
-     *                              during the search.
-     * @param int|null $lockVersion The lock version.
+     * @param int|null $lockMode One of the \Doctrine\DBAL\LockMode::* constants
+     *                           or NULL if no specific lock mode should be used
+     *                           during the search.
      * @psalm-param LockMode::*|null $lockMode
      *
      * @return object|null The entity instance or NULL if the entity can not be found.
      * @psalm-return ?T
      */
-    public function find($id, $lockMode = null, $lockVersion = null)
+    public function find(mixed $id, ?int $lockMode = null, ?int $lockVersion = null): ?object
     {
-        return $this->_em->find($this->_entityName, $id, $lockMode, $lockVersion);
+        return $this->em->find($this->entityName, $id, $lockMode, $lockVersion);
     }
 
     /**
      * Finds all entities in the repository.
      *
      * @psalm-return list<T> The entities.
      */
-    public function findAll()
+    public function findAll(): array
     {
         return $this->findBy([]);
     }
@@ -137,9 +109,9 @@ public function findAll()
      * @return object[] The objects.
      * @psalm-return list<T>
      */
-    public function findBy(array $criteria, ?array $orderBy = null, $limit = null, $offset = null)
+    public function findBy(array $criteria, ?array $orderBy = null, $limit = null, $offset = null): array
     {
-        $persister = $this->_em->getUnitOfWork()->getEntityPersister($this->_entityName);
+        $persister = $this->em->getUnitOfWork()->getEntityPersister($this->entityName);
 
         return $persister->loadAll($criteria, $orderBy, $limit, $offset);
     }
@@ -153,9 +125,9 @@ public function findBy(array $criteria, ?array $orderBy = null, $limit = null, $
      * @return object|null The entity instance or NULL if the entity can not be found.
      * @psalm-return ?T
      */
-    public function findOneBy(array $criteria, ?array $orderBy = null)
+    public function findOneBy(array $criteria, ?array $orderBy = null): ?object
     {
-        $persister = $this->_em->getUnitOfWork()->getEntityPersister($this->_entityName);
+        $persister = $this->em->getUnitOfWork()->getEntityPersister($this->entityName);
 
         return $persister->load($criteria, null, null, [], null, 1, $orderBy);
     }
@@ -169,23 +141,20 @@ public function findOneBy(array $criteria, ?array $orderBy = null)
      *
      * @todo Add this method to `ObjectRepository` interface in the next major release
      */
-    public function count(array $criteria = [])
+    public function count(array $criteria = []): int
     {
-        return $this->_em->getUnitOfWork()->getEntityPersister($this->_entityName)->count($criteria);
+        return $this->em->getUnitOfWork()->getEntityPersister($this->entityName)->count($criteria);
     }
 
     /**
      * Adds support for magic method calls.
      *
-     * @param string  $method
      * @param mixed[] $arguments
      * @psalm-param list<mixed> $arguments
      *
-     * @return mixed The returned value from the resolved method.
-     *
      * @throws BadMethodCallException If the method called is invalid.
      */
-    public function __call($method, $arguments)
+    public function __call(string $method, array $arguments): mixed
     {
         if (str_starts_with($method, 'findBy')) {
             return $this->resolveMagicCall('findBy', substr($method, 6), $arguments);
@@ -207,47 +176,37 @@ public function __call($method, $arguments)
     }
 
     /**
-     * @return string
+     * @psalm-return class-string<T>
      */
-    protected function getEntityName()
+    protected function getEntityName(): string
     {
-        return $this->_entityName;
+        return $this->entityName;
     }
 
-    /**
-     * @return string
-     */
-    public function getClassName()
+    public function getClassName(): string
     {
         return $this->getEntityName();
     }
 
-    /**
-     * @return EntityManagerInterface
-     */
-    protected function getEntityManager()
+    protected function getEntityManager(): EntityManagerInterface
     {
-        return $this->_em;
+        return $this->em;
     }
 
-    /**
-     * @return ClassMetadata
-     */
-    protected function getClassMetadata()
+    protected function getClassMetadata(): ClassMetadata
     {
-        return $this->_class;
+        return $this->class;
     }
 
     /**
      * Select all elements from a selectable that match the expression and
      * return a new collection containing these elements.
      *
-     * @return AbstractLazyCollection
      * @psalm-return AbstractLazyCollection<int, T>&Selectable<int, T>
      */
-    public function matching(Criteria $criteria)
+    public function matching(Criteria $criteria): AbstractLazyCollection
     {
-        $persister = $this->_em->getUnitOfWork()->getEntityPersister($this->_entityName);
+        $persister = $this->em->getUnitOfWork()->getEntityPersister($this->entityName);
 
         return new LazyCriteriaCollection($persister, $criteria);
     }
@@ -259,26 +218,22 @@ public function matching(Criteria $criteria)
      * @param string $by     The property name used as condition
      * @psalm-param list<mixed> $arguments The arguments to pass at method call
      *
-     * @return mixed
-     *
      * @throws InvalidMagicMethodCall If the method called is invalid or the
      *                                requested field/association does not exist.
      */
-    private function resolveMagicCall(string $method, string $by, array $arguments)
+    private function resolveMagicCall(string $method, string $by, array $arguments): mixed
     {
         if (! $arguments) {
             throw InvalidMagicMethodCall::onMissingParameter($method . $by);
         }
 
-        if (self::$inflector === null) {
-            self::$inflector = InflectorFactory::create()->build();
-        }
+        self::$inflector ??= InflectorFactory::create()->build();
 
         $fieldName = lcfirst(self::$inflector->classify($by));
 
-        if (! ($this->_class->hasField($fieldName) || $this->_class->hasAssociation($fieldName))) {
+        if (! ($this->class->hasField($fieldName) || $this->class->hasAssociation($fieldName))) {
             throw InvalidMagicMethodCall::becauseFieldNotFoundIn(
-                $this->_entityName,
+                $this->entityName,
                 $fieldName,
                 $method . $by
             );

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -1369,4 +1369,3 @@ parameters:
 			message: "#^Access to an undefined property Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\:\\:\\$subClasses\\.$#"
 			count: 1
 			path: lib/Doctrine/ORM/Utility/HierarchyDiscriminatorResolver.php
-
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -288,26 +288,14 @@
     </DeprecatedClass>
   </file>
   <file src="lib/Doctrine/ORM/EntityRepository.php">
-    <ArgumentTypeCoercion occurrences="5">
-      <code>$this-&gt;_entityName</code>
-      <code>$this-&gt;_entityName</code>
-      <code>$this-&gt;_entityName</code>
-      <code>$this-&gt;_entityName</code>
-      <code>$this-&gt;_entityName</code>
-    </ArgumentTypeCoercion>
-    <InvalidReturnStatement occurrences="3">
+    <InvalidReturnStatement occurrences="2">
       <code>$persister-&gt;load($criteria, null, null, [], null, 1, $orderBy)</code>
-      <code>$this-&gt;_em-&gt;find($this-&gt;_entityName, $id, $lockMode, $lockVersion)</code>
       <code>new LazyCriteriaCollection($persister, $criteria)</code>
     </InvalidReturnStatement>
-    <InvalidReturnType occurrences="3">
-      <code>?T</code>
+    <InvalidReturnType occurrences="2">
       <code>?T</code>
       <code>AbstractLazyCollection&lt;int, T&gt;&amp;Selectable&lt;int, T&gt;</code>
     </InvalidReturnType>
-    <LessSpecificImplementedReturnType occurrences="1">
-      <code>string</code>
-    </LessSpecificImplementedReturnType>
   </file>
   <file src="lib/Doctrine/ORM/Event/LifecycleEventArgs.php">
     <LessSpecificReturnStatement occurrences="1">

diff --git a/tests/Doctrine/Tests/ORM/Repository/DefaultRepositoryFactoryTest.php b/tests/Doctrine/Tests/ORM/Repository/DefaultRepositoryFactoryTest.php
@@ -10,6 +10,7 @@
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Repository\DefaultRepositoryFactory;
 use Doctrine\Tests\Models\DDC753\DDC753DefaultRepository;
+use Doctrine\Tests\Models\DDC753\DDC753EntityWithDefaultCustomRepository;
 use Doctrine\Tests\Models\DDC869\DDC869PaymentRepository;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
@@ -22,13 +23,12 @@
 class DefaultRepositoryFactoryTest extends TestCase
 {
     /** @var EntityManagerInterface&MockObject */
-    private $entityManager;
+    private EntityManagerInterface $entityManager;
 
     /** @var Configuration&MockObject */
-    private $configuration;
+    private Configuration $configuration;
 
-    /** @var DefaultRepositoryFactory */
-    private $repositoryFactory;
+    private DefaultRepositoryFactory $repositoryFactory;
 
     protected function setUp(): void
     {
@@ -70,7 +70,7 @@ public function testCreatedRepositoriesAreCached(): void
 
     public function testCreatesRepositoryFromCustomClassMetadata(): void
     {
-        $customMetadata                            = $this->buildClassMetadata(__DIR__);
+        $customMetadata                            = $this->buildClassMetadata(DDC753EntityWithDefaultCustomRepository::class);
         $customMetadata->customRepositoryClassName = DDC753DefaultRepository::class;
 
         $this->entityManager
@@ -107,12 +107,18 @@ public function testCachesDistinctRepositoriesPerDistinctEntityManager(): void
     }
 
     /**
+     * @psalm-param class-string<TEntity> $className
+     *
      * @return ClassMetadata&MockObject
+     * @psalm-return ClassMetadata<TEntity>&MockObject
+     *
+     * @template TEntity of object
      */
     private function buildClassMetadata(string $className): ClassMetadata
     {
         $metadata = $this->createMock(ClassMetadata::class);
         $metadata->expects(self::any())->method('getName')->will(self::returnValue($className));
+        $metadata->name = $className;
 
         $metadata->customRepositoryClassName = null;