psalm icon indicating copy to clipboard operation
psalm copied to clipboard

Type hole when empty collection allowed to substitute for typed

Open muglug opened this issue 2 years ago • 4 comments

https://psalm.dev/r/20e884ad18

We encounter this issue because PHP does not allow the explicit specification of generic parameters e.g. new Collection<int>([])

The only way to detect this would be to create a type variable #1 for the type of $array.

$array = new Collection([]); // has type Collection<#1>

$this->ints = $array; // new constraint #1 ≤ int
$this->strings = $array; //  new constraint #1 ≤ string

At the end of the function body we would collect the constraints for #1 and where those constraints originated from, and realise that there's no way to satisfy those constraints, emitting an error to the user.

This analysis is a challenge to implement, and I don't really think it's worth it given how uncommon the problem area is. But it's definitely good to be aware of it all the same.

muglug avatar Sep 13 '22 17:09 muglug

I found these snippets:

https://psalm.dev/r/20e884ad18
<?php
/**
 * @template Tv
 */
class Collection {
  /** @param list<Tv> $arr */
  public function __construct(public array $arr) {}
}

class Foo {
    /** @var Collection<int> $ints */
    public Collection $ints;

    /** @var Collection<string> $strings */
    public Collection $strings;

    public function __construct() {
        $array = new Collection([]);
        /** @psalm-trace $array */
        $array;

        $this->ints = $array;
        /** @psalm-trace $this->ints */
        $this->ints;

        $this->strings = $array; // this should be prevented, but it is not
    }
}
Psalm output (using commit d957ff2):

INFO: Trace - 20:9 - $array: Collection<never>

INFO: Trace - 24:9 - $this->ints: Collection<int>

psalm-github-bot[bot] avatar Sep 13 '22 17:09 psalm-github-bot[bot]

Would this also be fixed by implementing #8066 (which basically allows for new Collection<int>([]) except you have to use a docblock instead) and then fixing the type comparison so that never isn't a special case for template comparison?

AndrolGenhald avatar Sep 14 '22 01:09 AndrolGenhald

Hmm — it wouldn't really be fixed for the Psalm users who rely on that never template comparison behaviour

muglug avatar Sep 14 '22 02:09 muglug

Hmm — it wouldn't really be fixed for the Psalm users who rely on that never template comparison behaviour

Well, yes, but isn't that never template comparison what leads to the type hole?

I do sort of like the idea of collecting and solving constraints, but it would indeed be a lot of work, and we would definitely want to have really good error messages explaining exactly where the conflicting constraints are coming from. You're probably right that it's not worth the trouble right now though.

AndrolGenhald avatar Sep 14 '22 14:09 AndrolGenhald