Solve some PHPStan baseline errors

[VincentLanglet]

This is some things I saw in the baseline which could be easily solve I think.

[VincentLanglet]

In the matching method, there is the code

public function matching(Criteria $criteria): Collection
    {
        if ($this->isDirty) {
            $this->initialize();
        }

        if ($this->initialized) {
            return $this->unwrap()->matching($criteria);
        }

So, unwrap need to return a Selectable if we want to call matching on it.

[VincentLanglet]

Since unwrap need to return a Selectable (cf next comment), we need to only accepts selectable.

[VincentLanglet]

This is not possible since collectionDeletions is always containing PersistentCollection.

• when looking at the added value they are always persistent collection
• it it was possible to go in this if, we would have called $collectionToDelete->getMapping() but getMapping only exists for PersistentCollection.

[greg0ire]

This code was deliberately added in #7861, which is a bit worrying, but also, the test that was added in that PR still passes, so I guess it's fine?

[VincentLanglet]

Yes. I would say that the developer wanted to be BC

• If not PersistentCollection do like before
• If PersistentCollection add the bugfix

But it's not possible to have something else than PersistentCollection ^^
(You can look at the place where collectionDeletions is updated)

[VincentLanglet]

We only add PersistentCollection to this array

if ($orgValue instanceof PersistentCollection) {
                    // A PersistentCollection was de-referenced, so delete it.
                    $coid = spl_object_id($orgValue);

                    if (isset($this->collectionDeletions[$coid])) {
                        continue;
                    }

                    $this->collectionDeletions[$coid] = $orgValue;
                    $changeSet[$propName]             = $orgValue; // Signal changeset, to-many assocs will be ignored.

                    continue;
                }

[VincentLanglet]

We only add PersistentCollection to this array

if ($value instanceof PersistentCollection && $value->isDirty()) {
            $coid = spl_object_id($value);

            $this->collectionUpdates[$coid]  = $value;
            $this->visitedCollections[$coid] = $value;
        }

[VincentLanglet]

We only add PersistentCollection to this array

if ($value instanceof PersistentCollection && $value->isDirty()) {
            $coid = spl_object_id($value);

            $this->collectionUpdates[$coid]  = $value;
            $this->visitedCollections[$coid] = $value;
        }

[VincentLanglet]

I just have one error to solve for phpstan https://github.com/doctrine/orm/actions/runs/3509913825/jobs/5879333229
What do you recommend @greg0ire ?

It's related to the code

foreach ($class->reflFields as $name => $refProp) {
            $value = $refProp->getValue($entity);

            if ($class->isCollectionValuedAssociation($name) && $value !== null) {
                if ($value instanceof PersistentCollection) {
                    if ($value->getOwner() === $entity) {
                        continue;
                    }

                    $value = new ArrayCollection($value->getValues());
                }

                // If $value is not a Collection then use an ArrayCollection.
                if (! $value instanceof Collection) {
                    $value = new ArrayCollection($value);
                }

                $assoc = $class->associationMappings[$name];

                // Inject PersistentCollection
                $value = new PersistentCollection(
                    $this->em,
                    $this->em->getClassMetadata($assoc['targetEntity']),
                    $value
                );

we're only checking value is an instanceof Collection before passing the value to the PersistentCollection, but as explained in the comment of the PR, Persistent require we pass a Selectable&Collection or the matching method won't work.

Should I just add this error to the baseline ? (So it would be 5 less, for 1 more in the baseline)

[greg0ire]

Should I just add this error to the baseline ? (So it would be 5 less, for 1 more in the baseline)

Yes, it sounds like a fair tradeoff 👍

[greg0ire]

Thanks @VincentLanglet !