Introduce convenience methods to narrow types

These methods assert the type of the mapping provided by the collection according to the name of the class they are in: the one to many persister only ever deals with one to many associations, and the many to many persister only ever deals with many to many associations.
derrabus · May 13, 2023 · c51c84f · c51c84f
1 parent 9f60fdc
commit c51c84f
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 23 deletions.
diff --git a/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php
@@ -12,6 +12,7 @@
 use Doctrine\ORM\Mapping\AssociationMapping;
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Mapping\InverseSideMapping;
+use Doctrine\ORM\Mapping\ManyToManyAssociationMapping;
 use Doctrine\ORM\PersistentCollection;
 use Doctrine\ORM\Persisters\SqlValueVisitor;
 use Doctrine\ORM\Query;
@@ -33,7 +34,7 @@ class ManyToManyPersister extends AbstractCollectionPersister
 {
     public function delete(PersistentCollection $collection): void
     {
-        $mapping = $collection->getMapping();
+        $mapping = $this->getMapping($collection);
 
         if (! $mapping->isOwningSide()) {
             return; // ignore inverse side
@@ -53,7 +54,7 @@ public function delete(PersistentCollection $collection): void
 
     public function update(PersistentCollection $collection): void
     {
-        $mapping = $collection->getMapping();
+        $mapping = $this->getMapping($collection);
 
         if (! $mapping->isOwningSide()) {
             return; // ignore inverse side
@@ -81,7 +82,7 @@ public function update(PersistentCollection $collection): void
 
     public function get(PersistentCollection $collection, mixed $index): object|null
     {
-        $mapping = $collection->getMapping();
+        $mapping = $this->getMapping($collection);
 
         if (! $mapping->isIndexed()) {
             throw new BadMethodCallException('Selecting a collection by index is only supported on indexed collections.');
@@ -109,7 +110,7 @@ public function count(PersistentCollection $collection): int
         $conditions  = [];
         $params      = [];
         $types       = [];
-        $mapping     = $collection->getMapping();
+        $mapping     = $this->getMapping($collection);
         $id          = $this->uow->getEntityIdentifier($collection->getOwner());
         $sourceClass = $this->em->getClassMetadata($mapping->sourceEntity);
         $association = $this->em->getMetadataFactory()->getOwningSide($mapping);
@@ -162,15 +163,15 @@ public function count(PersistentCollection $collection): int
      */
     public function slice(PersistentCollection $collection, int $offset, int|null $length = null): array
     {
-        $mapping   = $collection->getMapping();
+        $mapping   = $this->getMapping($collection);
         $persister = $this->uow->getEntityPersister($mapping['targetEntity']);
 
         return $persister->getManyToManyCollection($mapping, $collection->getOwner(), $offset, $length);
     }
 
     public function containsKey(PersistentCollection $collection, mixed $key): bool
     {
-        $mapping = $collection->getMapping();
+        $mapping = $this->getMapping($collection);
 
         if (! $mapping->isIndexed()) {
             throw new BadMethodCallException('Selecting a collection by index is only supported on indexed collections.');
@@ -209,7 +210,7 @@ public function contains(PersistentCollection $collection, object $element): boo
      */
     public function loadCriteria(PersistentCollection $collection, Criteria $criteria): array
     {
-        $mapping       = $collection->getMapping();
+        $mapping       = $this->getMapping($collection);
         $owner         = $collection->getOwner();
         $ownerMetadata = $this->em->getClassMetadata($owner::class);
         $id            = $this->uow->getEntityIdentifier($owner);
@@ -364,7 +365,7 @@ protected function getOnConditionSQL(AssociationMapping $mapping): array
     protected function getDeleteSQL(PersistentCollection $collection): string
     {
         $columns = [];
-        $mapping = $collection->getMapping();
+        $mapping = $this->getMapping($collection);
         assert($mapping->isManyToManyOwningSide());
         $class     = $this->em->getClassMetadata($collection->getOwner()::class);
         $joinTable = $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform);
@@ -384,7 +385,7 @@ protected function getDeleteSQL(PersistentCollection $collection): string
      */
     protected function getDeleteSQLParameters(PersistentCollection $collection): array
     {
-        $mapping = $collection->getMapping();
+        $mapping = $this->getMapping($collection);
         assert($mapping->isManyToManyOwningSide());
         $identifier = $this->uow->getEntityIdentifier($collection->getOwner());
 
@@ -415,7 +416,7 @@ protected function getDeleteSQLParameters(PersistentCollection $collection): arr
      */
     protected function getDeleteRowSQL(PersistentCollection $collection): array
     {
-        $mapping = $collection->getMapping();
+        $mapping = $this->getMapping($collection);
         assert($mapping->isManyToManyOwningSide());
         $class       = $this->em->getClassMetadata($mapping->sourceEntity);
         $targetClass = $this->em->getClassMetadata($mapping->targetEntity);
@@ -464,7 +465,7 @@ protected function getInsertRowSQL(PersistentCollection $collection): array
     {
         $columns = [];
         $types   = [];
-        $mapping = $collection->getMapping();
+        $mapping = $this->getMapping($collection);
         assert($mapping->isManyToManyOwningSide());
         $class       = $this->em->getClassMetadata($mapping->sourceEntity);
         $targetClass = $this->em->getClassMetadata($mapping->targetEntity);
@@ -514,7 +515,7 @@ private function collectJoinTableColumnParameters(
         object $element,
     ): array {
         $params  = [];
-        $mapping = $collection->getMapping();
+        $mapping = $this->getMapping($collection);
         assert($mapping->isManyToManyOwningSide());
         $isComposite = count($mapping->joinTableColumns) > 2;
 
@@ -563,7 +564,7 @@ private function getJoinTableRestrictionsWithKey(
         string $key,
         bool $addFilters,
     ): array {
-        $filterMapping = $collection->getMapping();
+        $filterMapping = $this->getMapping($collection);
         $mapping       = $filterMapping;
         $indexBy       = $mapping->indexBy();
         $id            = $this->uow->getEntityIdentifier($collection->getOwner());
@@ -653,7 +654,7 @@ private function getJoinTableRestrictions(
         object $element,
         bool $addFilters,
     ): array {
-        $filterMapping = $collection->getMapping();
+        $filterMapping = $this->getMapping($collection);
         $mapping       = $filterMapping;
 
         if (! $mapping->isOwningSide()) {
@@ -757,4 +758,13 @@ private function getLimitSql(Criteria $criteria): string
 
         return $this->platform->modifyLimitQuery('', $limit, $offset ?? 0);
     }
+
+    private function getMapping(PersistentCollection $collection): AssociationMapping&ManyToManyAssociationMapping
+    {
+        $mapping = $collection->getMapping();
+
+        assert($mapping instanceof ManyToManyAssociationMapping);
+
+        return $mapping;
+    }
 }
diff --git a/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php
@@ -8,6 +8,7 @@
 use Doctrine\Common\Collections\Criteria;
 use Doctrine\DBAL\Exception as DBALException;
 use Doctrine\DBAL\Types\Type;
+use Doctrine\ORM\Mapping\OneToManyAssociationMapping;
 use Doctrine\ORM\PersistentCollection;
 use Doctrine\ORM\Utility\PersisterHelper;
 
@@ -27,7 +28,7 @@ public function delete(PersistentCollection $collection): void
         // The only valid case here is when you have weak entities. In this
         // scenario, you have @OneToMany with orphanRemoval=true, and replacing
         // the entire collection with a new would trigger this operation.
-        $mapping = $collection->getMapping();
+        $mapping = $this->getMapping($collection);
 
         if (! $mapping['orphanRemoval']) {
             // Handling non-orphan removal should never happen, as @OneToMany
@@ -53,8 +54,7 @@ public function update(PersistentCollection $collection): void
 
     public function get(PersistentCollection $collection, mixed $index): object|null
     {
-        $mapping = $collection->getMapping();
-        assert($mapping->isOneToMany());
+        $mapping = $this->getMapping($collection);
 
         if (! $mapping->isIndexed()) {
             throw new BadMethodCallException('Selecting a collection by index is only supported on indexed collections.');
@@ -101,7 +101,7 @@ public function slice(PersistentCollection $collection, int $offset, int|null $l
 
     public function containsKey(PersistentCollection $collection, mixed $key): bool
     {
-        $mapping = $collection->getMapping();
+        $mapping = $this->getMapping($collection);
 
         if (! $mapping->isIndexed()) {
             throw new BadMethodCallException('Selecting a collection by index is only supported on indexed collections.');
@@ -148,7 +148,7 @@ public function loadCriteria(PersistentCollection $collection, Criteria $criteri
     /** @throws DBALException */
     private function deleteEntityCollection(PersistentCollection $collection): int
     {
-        $mapping     = $collection->getMapping();
+        $mapping     = $this->getMapping($collection);
         $identifier  = $this->uow->getEntityIdentifier($collection->getOwner());
         $sourceClass = $this->em->getClassMetadata($mapping['sourceEntity']);
         $targetClass = $this->em->getClassMetadata($mapping['targetEntity']);
@@ -230,4 +230,13 @@ private function deleteJoinedEntityCollection(PersistentCollection $collection):
 
         return $numDeleted;
     }
+
+    private function getMapping(PersistentCollection $collection): OneToManyAssociationMapping
+    {
+        $mapping = $collection->getMapping();
+
+        assert($mapping->isOneToMany());
+
+        return $mapping;
+    }
 }
diff --git a/phpstan.neon b/phpstan.neon
@@ -28,7 +28,6 @@ parameters:
         -
             message: "#^Access to an undefined property .*Mapping\\:\\:\\$(inversedBy|mappedBy|joinColumns|joinTableColumns|(relation|source|target)To(Target|Source)KeyColumns|joinTable|indexBy)\\.$#"
             paths:
-                - lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php
                 - lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php
                 - lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
                 - lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php
@@ -51,3 +50,7 @@ parameters:
             paths:
                 - lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
                 - lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php
+
+        -
+            message: '#^.*OneToManyPersister::getMapping\(\) should return.*#'
+            path: lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -725,9 +725,7 @@
       <code><![CDATA[$collection->getOwner()]]></code>
       <code><![CDATA[$collection->getOwner()]]></code>
     </PossiblyNullArgument>
-    <UndefinedPropertyFetch>
-      <code><![CDATA[$mapping->mappedBy]]></code>
-    </UndefinedPropertyFetch>
+    <UndefinedPropertyFetch/>
   </file>
   <file src="lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php">
     <DocblockTypeContradiction>