Fix more types on EntityRepository and FilterCollection (doctrine#9525)

derrabus · Feb 20, 2022 · 08eaba4 · 08eaba4
1 parent 05c35c3
commit 08eaba4
Show file tree

Hide file tree

Showing 6 changed files with 52 additions and 40 deletions.
diff --git a/lib/Doctrine/ORM/Configuration.php b/lib/Doctrine/ORM/Configuration.php
@@ -34,6 +34,7 @@
 use Doctrine\ORM\Mapping\EntityListenerResolver;
 use Doctrine\ORM\Mapping\NamingStrategy;
 use Doctrine\ORM\Mapping\QuoteStrategy;
+use Doctrine\ORM\Query\Filter\SQLFilter;
 use Doctrine\ORM\Query\ResultSetMapping;
 use Doctrine\ORM\Repository\DefaultRepositoryFactory;
 use Doctrine\ORM\Repository\RepositoryFactory;
@@ -804,6 +805,7 @@ public function getClassMetadataFactoryName()
      *
      * @param string $name      The name of the filter.
      * @param string $className The class name of the filter.
+     * @psalm-param class-string<SQLFilter> $className
      *
      * @return void
      */
@@ -819,7 +821,7 @@ public function addFilter($name, $className)
      *
      * @return string|null The class name of the filter, or null if it is not
      *  defined.
-     * @psalm-return ?class-string
+     * @psalm-return class-string<SQLFilter>|null
      */
     public function getFilterClassName($name)
     {

diff --git a/lib/Doctrine/ORM/EntityRepository.php b/lib/Doctrine/ORM/EntityRepository.php
@@ -40,6 +40,7 @@ class EntityRepository implements ObjectRepository, Selectable
      * @internal This property will be private in 3.0, call {@see getEntityName()} instead.
      *
      * @var string
+     * @psalm-var class-string<T>
      */
     protected $_entityName;
 
@@ -54,12 +55,16 @@ class EntityRepository implements ObjectRepository, Selectable
      * @internal This property will be private in 3.0, call {@see getClassMetadata()} instead.
      *
      * @var ClassMetadata
+     * @psalm-var ClassMetadata<T>
      */
     protected $_class;
 
     /** @var Inflector|null */
     private static $inflector;
 
+    /**
+     * @psalm-param ClassMetadata<T> $class
+     */
     public function __construct(EntityManagerInterface $em, ClassMetadata $class)
     {
         $this->_entityName = $class->name;
@@ -276,14 +281,15 @@ public function __call($method, $arguments)
 
     /**
      * @return string
+     * @psalm-return class-string<T>
      */
     protected function getEntityName()
     {
         return $this->_entityName;
     }
 
     /**
-     * @return string
+     * {@inheritdoc}
      */
     public function getClassName()
     {
@@ -300,6 +306,7 @@ protected function getEntityManager()
 
     /**
      * @return ClassMetadata
+     * @psalm-return ClassMetadata<T>
      */
     protected function getClassMetadata()
     {

diff --git a/lib/Doctrine/ORM/Query/FilterCollection.php b/lib/Doctrine/ORM/Query/FilterCollection.php
@@ -47,13 +47,23 @@ class FilterCollection
      * Instances of enabled filters.
      *
      * @var SQLFilter[]
+     * @psalm-var array<string, SQLFilter>
      */
     private $enabledFilters = [];
 
-    /** @var string The filter hash from the last time the query was parsed. */
-    private $filterHash;
+    /**
+     * The filter hash from the last time the query was parsed.
+     *
+     * @var string
+     */
+    private $filterHash = '';
 
-    /** @var int The current state of this filter. */
+    /**
+     * The current state of this filter.
+     *
+     * @var int
+     * @psalm-var self::FILTERS_STATE_*
+     */
     private $filtersState = self::FILTERS_STATE_CLEAN;
 
     public function __construct(EntityManagerInterface $em)
@@ -66,6 +76,7 @@ public function __construct(EntityManagerInterface $em)
      * Gets all the enabled filters.
      *
      * @return SQLFilter[] The enabled filters.
+     * @psalm-return array<string, SQLFilter>
      */
     public function getEnabledFilters()
     {
@@ -167,7 +178,9 @@ public function isEnabled($name)
     }
 
     /**
-     * @return bool True, if the filter collection is clean.
+     * Checks if the filter collection is clean.
+     *
+     * @return bool
      */
     public function isClean()
     {

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -353,26 +353,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>
     <TooManyArguments occurrences="1">
       <code>find</code>
     </TooManyArguments>
@@ -2163,20 +2151,6 @@
       <code>$this-&gt;parameters</code>
     </PropertyTypeCoercion>
   </file>
-  <file src="lib/Doctrine/ORM/Query/FilterCollection.php">
-    <LessSpecificReturnStatement occurrences="1">
-      <code>$this-&gt;enabledFilters[$name]</code>
-    </LessSpecificReturnStatement>
-    <MoreSpecificReturnType occurrences="1">
-      <code>SQLFilter</code>
-    </MoreSpecificReturnType>
-    <PropertyNotSetInConstructor occurrences="1">
-      <code>$filterHash</code>
-    </PropertyNotSetInConstructor>
-    <PropertyTypeCoercion occurrences="1">
-      <code>$this-&gt;enabledFilters</code>
-    </PropertyTypeCoercion>
-  </file>
   <file src="lib/Doctrine/ORM/Query/Parser.php">
     <ArgumentTypeCoercion occurrences="1">
       <code>$stringPattern</code>

diff --git a/tests/Doctrine/Tests/ORM/Query/FilterCollectionTest.php b/tests/Doctrine/Tests/ORM/Query/FilterCollectionTest.php
@@ -85,6 +85,12 @@ public function testHashing(): void
         self::assertTrue($filterCollection->isClean());
 
         $oldHash = $filterCollection->getHash();
+        $filterCollection->setFiltersStateDirty();
+
+        self::assertFalse($filterCollection->isClean());
+        self::assertSame($oldHash, $filterCollection->getHash());
+        self::assertTrue($filterCollection->isClean());
+
         $filterCollection->enable('testFilter');
 
         self::assertFalse($filterCollection->isClean());
@@ -94,6 +100,12 @@ public function testHashing(): void
         self::assertNotSame($oldHash, $hash);
         self::assertTrue($filterCollection->isClean());
         self::assertSame($hash, $filterCollection->getHash());
+
+        $filterCollection->disable('testFilter');
+
+        self::assertFalse($filterCollection->isClean());
+        self::assertSame($oldHash, $filterCollection->getHash());
+        self::assertTrue($filterCollection->isClean());
     }
 }
 

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;
@@ -70,7 +71,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 +108,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->method('getName')->willReturn($className);
+        $metadata->name = $className;
 
         $metadata->customRepositoryClassName = null;
 
@@ -125,10 +132,7 @@ private function buildClassMetadata(string $className): ClassMetadata
     private function createEntityManager(): EntityManagerInterface
     {
         $entityManager = $this->createMock(EntityManagerInterface::class);
-
-        $entityManager->expects(self::any())
-            ->method('getConfiguration')
-            ->will(self::returnValue($this->configuration));
+        $entityManager->method('getConfiguration')->willReturn($this->configuration);
 
         return $entityManager;
     }