From c51c84fc4721501f3fd4bd131730dd87bc0c89ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Thu, 11 May 2023 21:14:37 +0200 Subject: [PATCH] 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. --- .../Collection/ManyToManyPersister.php | 38 ++++++++++++------- .../Collection/OneToManyPersister.php | 19 +++++++--- phpstan.neon | 5 ++- psalm-baseline.xml | 4 +- 4 files changed, 43 insertions(+), 23 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php index 64c8accadea..6934c2e215d 100644 --- 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,7 +163,7 @@ 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); @@ -170,7 +171,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.'); @@ -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 index 2391b24617e..8f24bafa959 100644 --- 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 index d63c4b8b0fc..846d43d211f 100644 --- 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 index 78135a9cbb4..05d1ad7e066 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -725,9 +725,7 @@ getOwner()]]> getOwner()]]> - - mappedBy]]> - +