-
Notifications
You must be signed in to change notification settings - Fork 112
Description
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
addreturn TRUE.removereturn the removed element or NULLremoveElementreturn TRUEclearreturn voidsetreturn void- Since the interface extends
ArrayAccess, I'm not sure about the behavior ofoffsetSet.
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.