Phpstan is messing with isEmpty and first/last value.

[VincentLanglet]

In
#172
#173
TypeSpecifyingExtension were introduced to narrow the return type of the Collection::first and Collection::last methods.

With the following tests

$entityOrFalse = $collection->first();
$entityOrFalse;

if ($collection->isEmpty()) {
	$false = $collection->first();
	$false;
}

if (!$collection->isEmpty()) {
	$entity = $collection->first();
	$entity;
}

I discovered that the following code it introduce some issues if you modify the $collection AFTER the isEmpty check and BEFORE the first or last call. Of course, I couldn't call this a good practice, so I don't know if it's a big deal.

For instance

if (!$collection->isEmpty()) {
        $collection->remove($foo);
	$entity = $collection->first();
	$entity; // This now might be false if the collection had only one element, `$foo`. But phpstan consider it to be an entity.
}

Or

if ($collection->isEmpty()) {
        $collection->add($bar);
	$false = $collection->first();
	$false; // This now should return `$bar` but phpstan consider it to be `false`.
}

Signature of Collection method can be found here: https://github.com/doctrine/collections/blob/1.6.x/lib/Doctrine/Common/Collections/Collection.php

• add return TRUE.
• remove return the removed element or NULL
• removeElement return TRUE
• clear return void
• set return void
• Since the interface extends ArrayAccess, I'm not sure about the behavior of offsetSet.

If I understand correctly the comment #172 (comment)
it require to add @phpstan-impure to every of those methods witch return a value (add, remove, removeElement and offsetSet maybe (?)). And to add tests too.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.