Generics hole that can lead to runtime errors

[ondrejmirtes]

When type variable T is present multiple times in function parameters, the resulting T is the union of all passed parameters mapped to that type.

For example:

/**
 * @template T
 * @param T $a
 * @param T $b
 * @return T
 */
function foo($a, $b) { ... }

foo(new \InvalidArgumentException(), new \DomainException()); // T is DomainException|InvalidArgumentException

One exception to this behaviour is when T is in the position of a callable:

/**
 * @template T
 * @param callable(T) $a
 * @param T $b
 * @return T
 */
function foo(callable $a, $b) { ... }

foo(function (\InvalidArgumentException $e) {}, new \DomainException); // T is DomainException

I think I've found a similar situation that asks for the same behavior, but I want multiple sets of eyes before I change this in PHPStan (and ask for the same change in Psalm too).

Let's say you have Collection class with @template TValue. When you have Collection<Dog>, you don't want Cat to be added to that collection. But if you define a function like this, it can happen without any warning from SA:

/**
 * @template T of object
 * @param Collection<T> $c
 * @param T $d
 */
function foo(Collection $c, object $d): void
{
    $c->add($d);
}

Because you can call it like this:

/**
 * @param Collection<Dog> $c
 */
function bar(Collection $c): void
{
    foo($c, new Cat()); // T is resolved to Cat|Dog, no error is reported
}

Full example: https://psalm.dev/r/8b6d92aee7 (no issues reported)

With my suggestion to treat T in Collection<T> same way as callable(T) is treated, this wouldn't be an issue, because T in foo($c, new Cat()); would be resolved to Cat and Collection<Cat> doesn't accept Collection<Dog>.

Thank you for consideration.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/8b6d92aee7

<?php

/** @template T of object */
interface Collection
{
    /** @param T $item */
    public function add(object $item): void;
}

class Cat
{
}

class Dog
{
}

/**
 * @template T of object
 * @param Collection<T> $c
 * @param T $d
 */
function foo(Collection $c, object $d): void
{
    $c->add($d); // adds Cat to Collection<Dog>
}

/**
 * @param Collection<Dog> $c
 */
function bar(Collection $c): void
{
    foo($c, new Cat());
}

Psalm output (using commit f94f3b8):

No issues!

[ondrejmirtes]

Or maybe we don't have to change the type inference logic, I just noticed that PHPStan reports this on the referenced example:

Parameter #1 $c of function foo expects Collection<Cat|Dog>, Collection<Dog> given.

Since the T is invariant, Psalm should report the same.

[muglug]

Yeah, the call to foo should fail because upper and lower bounds cannot be both correct

[muglug]

Also broken: https://psalm.dev/r/8a00a541fa

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/8a00a541fa

<?php

/** @template T of object */
interface Collection1
{
    /** @param T $item */
    public function add(object $item): void;
}

/** @template T of object */
interface Collection2
{
    /** @param T $item */
    public function add(object $item): void;
        
    /** @return T */
    public function get(): object;
}

class Cat
{
}

class Dog
{
}

/**
 * @template T of object
 * @param Collection1<T> $c
 * @param Collection2<T> $d
 */
function foo(Collection1 $c, Collection2 $d): void
{
    $c->add($d->get()); // adds Cat to Collection<Dog>
}

/**
 * @param Collection1<Dog> $c
 * @param Collection2<Cat> $d
 */
function bar(Collection1 $c, Collection2 $d): void
{
    foo($c, $d);
}

Psalm output (using commit 5b36803):

No issues!