Document properties as possibly null

__sleep() does not list these properties as suposed to be serialized. This means we have to assert that it is not in the many scenarios where we resort to these properties. Introducing a private method allows us to centralize the assertions.
doctrine · Oct 2, 2022 · f139165 · f139165
1 parent b05fb96
commit f139165
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 28 deletions.
diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php
@@ -65,7 +65,7 @@ final class PersistentCollection extends AbstractLazyCollection implements Selec
  /**
  * The EntityManager that manages the persistence of the collection.
  *
- * @var EntityManagerInterface
+ * @var EntityManagerInterface|null
  */
  private $em;
 
@@ -80,7 +80,7 @@ final class PersistentCollection extends AbstractLazyCollection implements Selec
  /**
  * The class descriptor of the collection's entity type.
  *
- * @var ClassMetadata
+ * @var ClassMetadata|null
  */
  private $typeClass;
 
@@ -136,9 +136,18 @@ public function getOwner()
  /** @return Mapping\ClassMetadata */
  public function getTypeClass(): Mapping\ClassMetadataInfo
  {
+ assert($this->typeClass !== null);
+
  return $this->typeClass;
  }
 
+ private function getUnitOfWork(): UnitOfWork
+ {
+ assert($this->em !== null);
+
+ return $this->em->getUnitOfWork();
+ }
+
  /**
  * INTERNAL:
  * Adds an element to a collection during hydration. This will automatically
@@ -153,13 +162,14 @@ public function hydrateAdd($element): void
  // If _backRefFieldName is set and its a one-to-many association,
  // we need to set the back reference.
  if ($this->backRefFieldName && $this->association['type'] === ClassMetadata::ONE_TO_MANY) {
+ assert($this->typeClass !== null);
  // Set back reference to owner
  $this->typeClass->reflFields[$this->backRefFieldName]->setValue(
  $element,
  $this->owner
  );
 
- $this->em->getUnitOfWork()->setOriginalEntityProperty(
+ $this->getUnitOfWork()->setOriginalEntityProperty(
  spl_object_id($element),
  $this->backRefFieldName,
  $this->owner
@@ -181,6 +191,7 @@ public function hydrateSet($key, $element): void
  // If _backRefFieldName is set, then the association is bidirectional
  // and we need to set the back reference.
  if ($this->backRefFieldName && $this->association['type'] === ClassMetadata::ONE_TO_MANY) {
+ assert($this->typeClass !== null);
  // Set back reference to owner
  $this->typeClass->reflFields[$this->backRefFieldName]->setValue(
  $element,
@@ -283,9 +294,10 @@ private function changed(): void
  $this->association['isOwningSide'] &&
  $this->association['type'] === ClassMetadata::MANY_TO_MANY &&
  $this->owner &&
+ assert($this->em !== null) &&
  $this->em->getClassMetadata(get_class($this->owner))->isChangeTrackingNotify()
  ) {
- $this->em->getUnitOfWork()->scheduleForDirtyCheck($this->owner);
+ $this->getUnitOfWork()->scheduleForDirtyCheck($this->owner);
  }
  }
 
@@ -343,7 +355,7 @@ public function remove($key)
  $this->owner &&
  $this->association['orphanRemoval']
  ) {
- $this->em->getUnitOfWork()->scheduleOrphanRemoval($removed);
+ $this->getUnitOfWork()->scheduleOrphanRemoval($removed);
  }
 
  return $removed;
@@ -368,7 +380,7 @@ public function removeElement($element): bool
  $this->owner &&
  $this->association['orphanRemoval']
  ) {
- $this->em->getUnitOfWork()->scheduleOrphanRemoval($element);
+ $this->getUnitOfWork()->scheduleOrphanRemoval($element);
  }
 
  return $removed;
@@ -383,7 +395,7 @@ public function containsKey($key): bool
  ! $this->initialized && $this->association['fetch'] === ClassMetadata::FETCH_EXTRA_LAZY
  && isset($this->association['indexBy'])
  ) {
- $persister = $this->em->getUnitOfWork()->getCollectionPersister($this->association);
+ $persister = $this->getUnitOfWork()->getCollectionPersister($this->association);
 
  return $this->unwrap()->containsKey($key) || $persister->containsKey($this, $key);
  }
@@ -399,7 +411,7 @@ public function containsKey($key): bool
  public function contains($element): bool
  {
  if (! $this->initialized && $this->association['fetch'] === ClassMetadata::FETCH_EXTRA_LAZY) {
- $persister = $this->em->getUnitOfWork()->getCollectionPersister($this->association);
+ $persister = $this->getUnitOfWork()->getCollectionPersister($this->association);
 
  return $this->unwrap()->contains($element) || $persister->contains($this, $element);
  }
@@ -417,11 +429,13 @@ public function get($key)
  && $this->association['fetch'] === ClassMetadata::FETCH_EXTRA_LAZY
  && isset($this->association['indexBy'])
  ) {
+ assert($this->em !== null);
+ assert($this->typeClass !== null);
  if (! $this->typeClass->isIdentifierComposite && $this->typeClass->isIdentifier($this->association['indexBy'])) {
  return $this->em->find($this->typeClass->name, $key);
  }
 
- return $this->em->getUnitOfWork()->getCollectionPersister($this->association)->get($this, $key);
+ return $this->getUnitOfWork()->getCollectionPersister($this->association)->get($this, $key);
  }
 
  return parent::get($key);
@@ -430,7 +444,7 @@ public function get($key)
  public function count(): int
  {
  if (! $this->initialized && $this->association !== null && $this->association['fetch'] === ClassMetadata::FETCH_EXTRA_LAZY) {
- $persister = $this->em->getUnitOfWork()->getCollectionPersister($this->association);
+ $persister = $this->getUnitOfWork()->getCollectionPersister($this->association);
 
  return $persister->count($this) + ($this->isDirty ? $this->unwrap()->count() : 0);
  }
@@ -448,7 +462,7 @@ public function set($key, $value): void
  $this->changed();
 
  if (is_object($value) && $this->em) {
- $this->em->getUnitOfWork()->cancelOrphanRemoval($value);
+ $this->getUnitOfWork()->cancelOrphanRemoval($value);
  }
  }
 
@@ -462,7 +476,7 @@ public function add($value): bool
  $this->changed();
 
  if (is_object($value) && $this->em) {
- $this->em->getUnitOfWork()->cancelOrphanRemoval($value);
+ $this->getUnitOfWork()->cancelOrphanRemoval($value);
  }
 
  return true;
@@ -525,7 +539,7 @@ public function clear(): void
  return;
  }
 
- $uow = $this->em->getUnitOfWork();
+ $uow = $this->getUnitOfWork();
 
  if (
  $this->association['type'] & ClassMetadata::TO_MANY &&
@@ -585,7 +599,7 @@ public function __sleep(): array
  public function slice($offset, $length = null): array
  {
  if (! $this->initialized && ! $this->isDirty && $this->association['fetch'] === ClassMetadata::FETCH_EXTRA_LAZY) {
- $persister = $this->em->getUnitOfWork()->getCollectionPersister($this->association);
+ $persister = $this->getUnitOfWork()->getCollectionPersister($this->association);
 
  return $persister->slice($this, $offset, $length);
  }
@@ -637,7 +651,7 @@ public function matching(Criteria $criteria): Collection
  }
 
  if ($this->association['type'] === ClassMetadata::MANY_TO_MANY) {
- $persister = $this->em->getUnitOfWork()->getCollectionPersister($this->association);
+ $persister = $this->getUnitOfWork()->getCollectionPersister($this->association);
 
  return new ArrayCollection($persister->loadCriteria($this, $criteria));
  }
@@ -651,7 +665,7 @@ public function matching(Criteria $criteria): Collection
  $criteria->where($expression);
  $criteria->orderBy($criteria->getOrderings() ?: $this->association['orderBy'] ?? []);
 
- $persister = $this->em->getUnitOfWork()->getEntityPersister($this->association['targetEntity']);
+ $persister = $this->getUnitOfWork()->getEntityPersister($this->association['targetEntity']);
 
  return $this->association['fetch'] === ClassMetadata::FETCH_EXTRA_LAZY
  ? new LazyCriteriaCollection($persister, $criteria)
@@ -680,7 +694,7 @@ protected function doInitialize(): void
  }
 
  $this->unwrap()->clear();
- $this->em->getUnitOfWork()->loadCollection($this);
+ $this->getUnitOfWork()->loadCollection($this);
  $this->takeSnapshot();
 
  if ($newlyAddedDirtyObjects) {

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -250,11 +250,6 @@ parameters:
  count: 1
  path: lib/Doctrine/ORM/PersistentCollection.php
 
- -
- message: "#^Right side of && is always true\\.$#"
- count: 2
- path: lib/Doctrine/ORM/PersistentCollection.php
-
  -
  message: "#^Method Doctrine\\\\ORM\\\\Persisters\\\\Collection\\\\OneToManyPersister\\:\\:delete\\(\\) should return int\\|null but empty return statement found\\.$#"
  count: 1

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1105,12 +1105,6 @@
  <code>setValue</code>
  <code>setValue</code>
  </PossiblyNullReference>
- <RedundantConditionGivenDocblockType occurrences="4">
- <code>$this-&gt;em</code>
- <code>$this-&gt;em</code>
- <code>is_object($value) &amp;&amp; $this-&gt;em</code>
- <code>is_object($value) &amp;&amp; $this-&gt;em</code>
- </RedundantConditionGivenDocblockType>
  <TooManyArguments occurrences="1">
  <code>andX</code>
  </TooManyArguments>