doctrine · derrabus · May 8, 2023 · May 8, 2023
diff --git a/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php b/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php
@@ -92,7 +92,7 @@ public function storeCollectionCache(CollectionCacheKey $key, Collection|array $
         $targetHydrator = $targetPersister->getEntityHydrator();
 
         // Only preserve ordering if association configured it
-        if (! (isset($associationMapping['indexBy']) && $associationMapping['indexBy'])) {
+        if (! $associationMapping->isIndexed()) {
             // Elements may be an array or a Collection
             $elements = array_values($elements instanceof Collection ? $elements->getValues() : $elements);
         }

diff --git a/lib/Doctrine/ORM/Cache/Persister/Collection/NonStrictReadWriteCachedCollectionPersister.php b/lib/Doctrine/ORM/Cache/Persister/Collection/NonStrictReadWriteCachedCollectionPersister.php
@@ -56,7 +56,7 @@ public function update(PersistentCollection $collection): void
         $key     = new CollectionCacheKey($this->sourceEntity->rootEntityName, $this->association['fieldName'], $ownerId);
 
        // Invalidate non initialized collections OR ordered collection
-        if ($isDirty && ! $isInitialized || isset($this->association['orderBy'])) {
+        if ($isDirty && ! $isInitialized || $this->association->isOrdered()) {
             $this->persister->update($collection);
 
             $this->queuedCache['delete'][spl_object_id($collection)] = $key;

diff --git a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php
@@ -14,6 +14,7 @@
 
 use function array_fill_keys;
 use function array_keys;
+use function assert;
 use function count;
 use function is_array;
 use function key;
@@ -177,6 +178,7 @@ private function initRelatedCollection(
         }
 
         if (! $value instanceof PersistentCollection) {
+            assert($relation->isToMany());
             $value = new PersistentCollection(
                 $this->em,
                 $this->metadataCache[$relation['targetEntity']],

diff --git a/lib/Doctrine/ORM/Mapping/AssociationMapping.php b/lib/Doctrine/ORM/Mapping/AssociationMapping.php
@@ -205,6 +205,18 @@ final public function isManyToMany(): bool
         return $this instanceof ManyToManyAssociationMapping;
     }
 
+    /** @psalm-assert-if-true ToManyAssociationMapping $this */
+    final public function isOrdered(): bool
+    {
+        return $this->isToMany() && $this->orderBy() !== [];
+    }
+
+    /** @psalm-assert-if-true ToManyAssociationMapping $this */
+    public function isIndexed(): bool
+    {
+        return false;
+    }
+
     final public function type(): int
     {
         return match (true) {

diff --git a/lib/Doctrine/ORM/Mapping/ToManyAssociationMapping.php b/lib/Doctrine/ORM/Mapping/ToManyAssociationMapping.php
@@ -6,4 +6,11 @@
 
 interface ToManyAssociationMapping
 {
+    /** @psalm-assert-if-true string $this->indexBy() */
+    public function isIndexed(): bool;
+
+    public function indexBy(): string;
+
+    /** @return array<string, 'asc'|'desc'> */
+    public function orderBy(): array;
 }
diff --git a/lib/Doctrine/ORM/Mapping/ToManyAssociationMappingImplementation.php b/lib/Doctrine/ORM/Mapping/ToManyAssociationMappingImplementation.php
@@ -4,6 +4,10 @@
 
 namespace Doctrine\ORM\Mapping;
 
+use LogicException;
+
+use function sprintf;
+
 /** @internal */
 trait ToManyAssociationMappingImplementation
 {
@@ -18,19 +22,46 @@
     /**
      * A map of field names (of the target entity) to sorting directions
      *
-     * @var array<string, 'asc'|'desc'>|null
+     * @var array<string, 'asc'|'desc'>
      */
-    public array|null $orderBy = null;
+    public array $orderBy = [];
+
+    /** @return array<string, 'asc'|'desc'> */
+    final public function orderBy(): array
+    {
+        return $this->orderBy;
+    }
+
+    /** @psalm-assert-if-true !null $this->indexBy */
+    final public function isIndexed(): bool
+    {
+        return $this->indexBy !== null;
+    }
+
+    final public function indexBy(): string
+    {
+        if (! $this->isIndexed()) {
+            throw new LogicException(sprintf(
+                'This mapping is not indexed. Use %s::isIndexed() to check that before calling %s.',
+                self::class,
+                __METHOD__,
+            ));
+        }
+
+        return $this->indexBy;
+    }
 
     /** @return list<string> */
     public function __sleep(): array
     {
         $serialized = parent::__sleep();
 
-        foreach (['indexBy', 'orderBy'] as $stringOrArrayKey) {
-            if ($this->$stringOrArrayKey !== null) {
-                $serialized[] = $stringOrArrayKey;
-            }
+        if ($this->indexBy !== null) {
+            $serialized[] = 'indexBy';
+        }
+
+        if ($this->orderBy !== []) {
+            $serialized[] = 'orderBy';
         }
 
         return $serialized;

diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php
@@ -12,6 +12,7 @@
 use Doctrine\ORM\Mapping\AssociationMapping;
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Mapping\ManyToManyOwningSideMapping;
+use Doctrine\ORM\Mapping\ToManyAssociationMapping;
 use RuntimeException;
 use UnexpectedValueException;
 
@@ -56,6 +57,8 @@ final class PersistentCollection extends AbstractLazyCollection implements Selec
     /**
      * The association mapping the collection belongs to.
      * This is currently either a OneToManyMapping or a ManyToManyMapping.
+     *
+     * @var (AssociationMapping&ToManyAssociationMapping)|null
      */
     private AssociationMapping|null $association = null;
 
@@ -98,7 +101,7 @@ public function __construct(
      * Sets the collection's owning entity together with the AssociationMapping that
      * describes the association between the owner and the elements of the collection.
      */
-    public function setOwner(object $entity, AssociationMapping $assoc): void
+    public function setOwner(object $entity, AssociationMapping&ToManyAssociationMapping $assoc): void
     {
         $this->owner            = $entity;
         $this->association      = $assoc;
@@ -245,7 +248,7 @@ public function getInsertDiff(): array
     }
 
     /** INTERNAL: Gets the association mapping of the collection. */
-    public function getMapping(): AssociationMapping
+    public function getMapping(): AssociationMapping&ToManyAssociationMapping
     {
         if ($this->association === null) {
             throw new UnexpectedValueException('The underlying association mapping is null although it should not be');
@@ -600,7 +603,7 @@ public function matching(Criteria $criteria): Collection
 
         $criteria = clone $criteria;
         $criteria->where($expression);
-        $criteria->orderBy($criteria->getOrderings() ?: $association['orderBy'] ?? []);
+        $criteria->orderBy($criteria->getOrderings() ?: $association->orderBy());
 
         $persister = $this->getUnitOfWork()->getEntityPersister($association['targetEntity']);
 

diff --git a/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php
@@ -83,7 +83,7 @@ public function get(PersistentCollection $collection, mixed $index): object|null
     {
         $mapping = $collection->getMapping();
 
-        if (! isset($mapping->indexBy)) {
+        if (! $mapping->isIndexed()) {
             throw new BadMethodCallException('Selecting a collection by index is only supported on indexed collections.');
         }
 
@@ -95,7 +95,7 @@ public function get(PersistentCollection $collection, mixed $index): object|null
         assert($mappedKey !== null);
 
         return $persister->load(
-            [$mappedKey => $collection->getOwner(), $mapping->indexBy => $index],
+            [$mappedKey => $collection->getOwner(), $mapping->indexBy() => $index],
             null,
             $mapping,
             [],
@@ -177,7 +177,7 @@ public function containsKey(PersistentCollection $collection, mixed $key): bool
     {
         $mapping = $collection->getMapping();
 
-        if (! isset($mapping->indexBy)) {
+        if (! $mapping->isIndexed()) {
             throw new BadMethodCallException('Selecting a collection by index is only supported on indexed collections.');
         }
 
@@ -574,11 +574,10 @@ private function getJoinTableRestrictionsWithKey(
     ): array {
         $filterMapping = $collection->getMapping();
         $mapping       = $filterMapping;
-        assert(isset($mapping->indexBy));
-        $indexBy     = $mapping->indexBy;
-        $id          = $this->uow->getEntityIdentifier($collection->getOwner());
-        $sourceClass = $this->em->getClassMetadata($mapping->sourceEntity);
-        $targetClass = $this->em->getClassMetadata($mapping->targetEntity);
+        $indexBy       = $mapping->indexBy();
+        $id            = $this->uow->getEntityIdentifier($collection->getOwner());
+        $sourceClass   = $this->em->getClassMetadata($mapping->sourceEntity);
+        $targetClass   = $this->em->getClassMetadata($mapping->targetEntity);
 
         if (! $mapping->isOwningSide()) {
             assert($mapping instanceof InverseSideMapping);

diff --git a/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php
@@ -54,17 +54,18 @@ public function update(PersistentCollection $collection): void
     public function get(PersistentCollection $collection, mixed $index): object|null
     {
         $mapping = $collection->getMapping();
+        assert($mapping->isOneToMany());
 
-        if (! isset($mapping['indexBy'])) {
+        if (! $mapping->isIndexed()) {
             throw new BadMethodCallException('Selecting a collection by index is only supported on indexed collections.');
         }
 
         $persister = $this->uow->getEntityPersister($mapping['targetEntity']);
 
         return $persister->load(
             [
-                $mapping['mappedBy'] => $collection->getOwner(),
-                $mapping['indexBy']  => $index,
+                $mapping->mappedBy  => $collection->getOwner(),
+                $mapping->indexBy() => $index,
             ],
             null,
             $mapping,
@@ -102,7 +103,7 @@ public function containsKey(PersistentCollection $collection, mixed $key): bool
     {
         $mapping = $collection->getMapping();
 
-        if (! isset($mapping['indexBy'])) {
+        if (! $mapping->isIndexed()) {
             throw new BadMethodCallException('Selecting a collection by index is only supported on indexed collections.');
         }
 
@@ -113,8 +114,8 @@ public function containsKey(PersistentCollection $collection, mixed $key): bool
         // 'mappedBy' field.
         $criteria = new Criteria();
 
-        $criteria->andWhere(Criteria::expr()->eq($mapping['mappedBy'], $collection->getOwner()));
-        $criteria->andWhere(Criteria::expr()->eq($mapping['indexBy'], $key));
+        $criteria->andWhere(Criteria::expr()->eq($mapping->mappedBy, $collection->getOwner()));
+        $criteria->andWhere(Criteria::expr()->eq($mapping->indexBy(), $key));
 
         return (bool) $persister->count($criteria);
     }

diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
@@ -918,9 +918,9 @@ private function loadArrayFromResult(AssociationMapping $assoc, Result $stmt): a
         $rsm   = $this->currentPersisterContext->rsm;
         $hints = [UnitOfWork::HINT_DEFEREAGERLOAD => true];
 
-        if (isset($assoc->indexBy)) {
+        if ($assoc->isIndexed()) {
             $rsm = clone $this->currentPersisterContext->rsm; // this is necessary because the "default rsm" should be changed.
-            $rsm->addIndexBy('r', $assoc->indexBy);
+            $rsm->addIndexBy('r', $assoc->indexBy());
         }
 
         return $this->em->newHydrator(Query::HYDRATE_OBJECT)->hydrateAll($stmt, $rsm, $hints);
@@ -942,9 +942,9 @@ private function loadCollectionFromStatement(
             'collection' => $coll,
         ];
 
-        if (isset($assoc->indexBy)) {
+        if ($assoc->isIndexed()) {
             $rsm = clone $this->currentPersisterContext->rsm; // this is necessary because the "default rsm" should be changed.
-            $rsm->addIndexBy('r', $assoc->indexBy);
+            $rsm->addIndexBy('r', $assoc->indexBy());
         }
 
         return $this->em->newHydrator(Query::HYDRATE_OBJECT)->hydrateAll($stmt, $rsm, $hints);
@@ -1049,8 +1049,8 @@ public function getSelectSQL(
             $joinSql = $this->getSelectManyToManyJoinSQL($assoc);
         }
 
-        if ($assoc !== null && isset($assoc->orderBy)) {
-            $orderBy = $assoc->orderBy;
+        if ($assoc !== null && $assoc->isOrdered()) {
+            $orderBy = $assoc->orderBy();
         }
 
         if ($orderBy) {
@@ -1244,8 +1244,8 @@ protected function getSelectColumnsSQL(): string
             $association   = $assoc;
             $joinCondition = [];
 
-            if (isset($assoc->indexBy)) {
-                $this->currentPersisterContext->rsm->addIndexBy($assocAlias, $assoc->indexBy);
+            if ($assoc->isIndexed()) {
+                $this->currentPersisterContext->rsm->addIndexBy($assocAlias, $assoc->indexBy());
             }
 
             if (! $assoc->isOwningSide()) {

diff --git a/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php
@@ -271,8 +271,8 @@ public function getSelectSQL(
 
         $orderBySql = '';
 
-        if ($assoc !== null && isset($assoc['orderBy'])) {
-            $orderBy = $assoc['orderBy'];
+        if ($assoc !== null && $assoc->isOrdered()) {
+            $orderBy = $assoc->orderBy();
         }
 
         if ($orderBy) {

diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php
@@ -1027,8 +1027,8 @@ public function walkJoinAssociationDeclaration(
         if ($indexBy) {
             // For Many-To-One or One-To-One associations this obviously makes no sense, but is ignored silently.
             $this->walkIndexBy($indexBy);
-        } elseif (isset($relation['indexBy'])) {
-            $this->rsm->addIndexBy($joinedDqlAlias, $relation['indexBy']);
+        } elseif ($relation->isIndexed()) {
+            $this->rsm->addIndexBy($joinedDqlAlias, $relation->indexBy());
         }
 
         return $sql;

diff --git a/lib/Doctrine/ORM/Tools/SchemaValidator.php b/lib/Doctrine/ORM/Tools/SchemaValidator.php
@@ -214,8 +214,8 @@ public function validateClass(ClassMetadata $class): array
                 }
             }
 
-            if (isset($assoc['orderBy']) && $assoc['orderBy'] !== null) {
-                foreach ($assoc['orderBy'] as $orderField => $orientation) {
+            if ($assoc->isOrdered()) {
+                foreach ($assoc->orderBy() as $orderField => $orientation) {
                     if (! $targetMetadata->hasField($orderField) && ! $targetMetadata->hasAssociation($orderField)) {
                         $ce[] = 'The association ' . $class->name . '#' . $fieldName . ' is ordered by a foreign field ' .
                                 $orderField . ' that is not a field on the target entity ' . $targetMetadata->name . '.';

diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php
@@ -575,6 +575,7 @@ public function computeChangeSet(ClassMetadata $class, object $entity): void
                 }
 
                 $assoc = $class->associationMappings[$name];
+                assert($assoc->isToMany());
 
                 // Inject PersistentCollection
                 $value = new PersistentCollection(
@@ -671,6 +672,7 @@ public function computeChangeSet(ClassMetadata $class, object $entity): void
                 // created one. This can only mean it was cloned and replaced
                 // on another entity.
                 if ($actualValue instanceof PersistentCollection) {
+                    assert($assoc->isToMany());
                     $owner = $actualValue->getOwner();
                     if ($owner === null) { // cloned
                         $actualValue->setOwner($entity, $assoc);
@@ -2433,6 +2435,7 @@ public function createEntity(string $className, array $data, array &$hints = [])
                     break;
 
                 default:
+                    assert($assoc->isToMany());
                     // Ignore if its a cached collection
                     if (isset($hints[Query::HINT_CACHE_ENABLED]) && $class->getFieldValue($entity, $field) instanceof PersistentCollection) {
                         break;

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -125,6 +125,11 @@ parameters:
 			count: 1
 			path: lib/Doctrine/ORM/EntityRepository.php
 
+		-
+			message: "#^Parameter \\#2 \\$assoc of method Doctrine\\\\ORM\\\\PersistentCollection\\<\\(int\\|string\\),mixed\\>\\:\\:setOwner\\(\\) expects Doctrine\\\\ORM\\\\Mapping\\\\AssociationMapping&Doctrine\\\\ORM\\\\Mapping\\\\ToManyAssociationMapping, Doctrine\\\\ORM\\\\Mapping\\\\AssociationMapping given\\.$#"
+			count: 1
+			path: lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php
+
 		-
 			message: "#^Method Doctrine\\\\ORM\\\\Mapping\\\\ClassMetadata\\:\\:fullyQualifiedClassName\\(\\) should return class\\-string\\|null but returns string\\|null\\.$#"
 			count: 1
@@ -300,6 +305,16 @@ parameters:
 			count: 1
 			path: lib/Doctrine/ORM/Tools/SchemaTool.php
 
+		-
+			message: "#^Parameter \\#2 \\$assoc of method Doctrine\\\\ORM\\\\PersistentCollection\\<\\(int\\|string\\),mixed\\>\\:\\:setOwner\\(\\) expects Doctrine\\\\ORM\\\\Mapping\\\\AssociationMapping&Doctrine\\\\ORM\\\\Mapping\\\\ToManyAssociationMapping, Doctrine\\\\ORM\\\\Mapping\\\\AssociationMapping given\\.$#"
+			count: 4
+			path: lib/Doctrine/ORM/UnitOfWork.php
+
+		-
+			message: "#^Parameter \\#2 \\$assoc of method Doctrine\\\\ORM\\\\PersistentCollection\\<\\*NEVER\\*,\\*NEVER\\*\\>\\:\\:setOwner\\(\\) expects Doctrine\\\\ORM\\\\Mapping\\\\AssociationMapping&Doctrine\\\\ORM\\\\Mapping\\\\ToManyAssociationMapping, Doctrine\\\\ORM\\\\Mapping\\\\AssociationMapping given\\.$#"
+			count: 1
+			path: lib/Doctrine/ORM/UnitOfWork.php
+
 		-
 			message: "#^Parameter \\#3 \\$changeSet of class Doctrine\\\\ORM\\\\Event\\\\PreUpdateEventArgs constructor is passed by reference, so it expects variables only$#"
 			count: 1