phpstan-doctrine icon indicating copy to clipboard operation
phpstan-doctrine copied to clipboard

Collection->contains() returns always-false for freshly created ArrayCollection without defined template type

Open janedbal opened this issue 2 years ago • 7 comments

Following error appeared after upgrading from 1.3.14 to 1.3.15. Easily fixable by using array and creating collection later, so I assume this is minor issue.

/**
 * @return Collection<int, Account>
 */
public function getAccounts(): Collection
{
    $accounts = new ArrayCollection();

    foreach ($this->users as $user) {
        $account = $user->getAccount();

        if (!$accounts->contains($account)) { // error: Negated boolean expression is always true. 
            $accounts->add($account);
        }
    }

    return $accounts;
}

janedbal avatar Sep 30 '22 13:09 janedbal

@janedbal \Doctrine\Common\Collections\ReadableCollection::contains has conditional return statement using template. Returns false if the contains parameter type does not match type of collection items. You need to specify type for $accounts collection.

/** @var ArrayCollection<Account> $accounts */
$accounts = new ArrayCollection();

vitkutny avatar Oct 12 '22 08:10 vitkutny

Thanks for explanation, that works. Although it is a bit questionable behaviour.

    /**
     * @psalm-param TMaybeContained $element
     * @psalm-return (TMaybeContained is T ? bool : false)
     *
     * @template TMaybeContained
     */
    public function contains($element);

So it means Account is MixedType is evaluated as false. I'd expect evaluation to Maybe... which is hard to decide which return type to use :)

janedbal avatar Oct 12 '22 10:10 janedbal

Wondering if phpstan-doctrine should not raise an error if Collection is created without known template type (no constructor argument). That should prevent those problems.

$accounts = new ArrayCollection(); // error
$accounts = new ArrayCollection($knownList); // ok

janedbal avatar Oct 12 '22 11:10 janedbal

I think this needs https://github.com/phpstan/phpstan/issues/6732#issuecomment-1062029088 to be solved...

ondrejmirtes avatar Oct 26 '22 08:10 ondrejmirtes

Isn't this fixed thanks to https://github.com/phpstan/phpstan-doctrine/pull/401 ?

ondrejmirtes avatar Jan 10 '23 08:01 ondrejmirtes

No, just tested on 1.3.29

janedbal avatar Jan 10 '23 09:01 janedbal

        // __construct - Works
        $collection = new ArrayCollection(['foo']);
        if ($collection->contains('foo')) {}

        // add - False positive
        $collection = new ArrayCollection();
        $collection->add('foo');
        if ($collection->contains('foo')) {} # ERROR: If condition is always false.PHPStan

        // Workaround - Works
        /** @var ArrayCollection<int, string> $collection */
        $collection = new ArrayCollection();
        $collection->add('foo');
        if ($collection->contains('foo')) {}

ili101 avatar Mar 13 '24 16:03 ili101