phpstan / phpstan-doctrine

Doctrine extensions for PHPStan
MIT License
595 stars 97 forks source link

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

Open janedbal opened 2 years ago

janedbal commented 2 years ago

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;
}
vitkutny commented 2 years ago

@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();
janedbal commented 2 years ago

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 commented 2 years ago

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
ondrejmirtes commented 2 years ago

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

ondrejmirtes commented 1 year ago

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

janedbal commented 1 year ago

No, just tested on 1.3.29

ili101 commented 7 months ago
        // __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')) {}