Repository navigation
Bidirectional type narrowing for generic types (would close an obscure hole in type checking!) #6732
Description
Feature request
Currently an assignment of the form$this->attribute = $value narrows the type of $this->attribute to the same type as $value for the remainder of the method. In most cases, this makes sense.
For example, the following code is statically sound. Iterables don't always have a forAll method, but there's no possible way $this->stuff can be of a type other than ArrayCollection when we attempt to call the forAll method.
/** @var iterable<int, Thing> */
private iterable $stuff;
public function __construct() {
$this->stuff = new ArrayCollection();
$this->stuff->forAll($doAThing);
}However, a problem arises if the type of $value is in some way wider than the type of $this->attribute. This is only possible when generic types are used: $this->attribute might have narrower type parameters than $value does, although $value might have a more specific class type. Since the assignment only affects the inferred type of $this->attribute, it actually widens those type parameters.
I demonstrated an example of this problem a few minutes ago, which I'll reproduce here with dumpType information included:
/** @var Collection<int, int> $ints */
public iterable $ints;
/** @var Collection<int, string> $strings */
public iterable $strings;
public function __construct() {
dumpType($this->ints); // Doctrine\Common\Collections\Collection<int, int>
dumpType($this->strings); // Doctrine\Common\Collections\Collection<int, string>
$array = new ArrayCollection();
dumpType($array); // Doctrine\Common\Collections\ArrayCollection<*NEVER*, *NEVER*>
$this->ints = $array;
dumpType($this->ints); // Doctrine\Common\Collections\ArrayCollection<*NEVER*, *NEVER*>
$this->strings = $array;
dumpType($this->strings); // Doctrine\Common\Collections\ArrayCollection<*NEVER*, *NEVER*>
}As you can see, PHPStan "forgets" that $this->ints and $this->strings have different type parameters. This leads to an admittedly-minor hole in the type system, since this mechanism may be used to statically cast from any type to any other, without a corresponding runtime conversion occurring.
In a situation like this, I believe that PHPStan should effectively unify the types of $this->attribute and $value, in the logic programming sense. The inferred types of both variables ought to be narrowed by the assignment expression, preserving type compatibility for both the outer class type and its type parameters. In other words, this ought to happen:
dumpType($this->ints); // Doctrine\Common\Collections\Collection<int, int>
$array = new ArrayCollection();
dumpType($array); // Doctrine\Common\Collections\ArrayCollection<*NEVER*, *NEVER*>
$this->ints = $array;
dumpType($this->ints); // Doctrine\Common\Collections\ArrayCollection<int, int>
dumpType($array); // Doctrine\Common\Collections\ArrayCollection<int, int>This narrowing step would cause the subsequent assignment, $this->strings = $array, to fail type-checking: a Collection<int, int> cannot be assigned to a variable of type Collection<int, string>. The hole in the type system is sealed.
I do think this is an unlikely situation, since one would typically write $this->ints = new ArrayCollection() in the first place rather than using a temporary variable. Additionally, Psalm has the exact same type system hole at time of writing. Still, this is an issue worth being aware of. Also, narrowing bidirectionally would allow the constructor to go ahead and populate $this->ints in a properly typesafe way, which currently doesn't occur.
Did PHPStan help you today? Did it make you happy in any way?
Yes! I've been experimenting with API Platform at work, since we're hoping to adopt it for newly planned projects, and API Platform's very structured design has turned out to mesh wonderfully with PHPStan's type checking. As a Haskell fan I generally love powerful type systems, and PHPStan has been giving me those warm fuzzy TypeScript feels despite working in PHP. 🐱
Activity
arnaud-lb commentedon Mar 5, 2022
Here is an other example: https://phpstan.org/r/064e69da-8b94-461b-872c-a2322d69e2a6
arnaud-lb commentedon Mar 8, 2022
Agreed.
So we would have these rules:
(Rules 1-3 are the current behavior)
*NEVER*type could denote that, or an other specific typeFoo<int>acceptsFoo<unresolved>)Rules 4 and 5 prevent aliasing the same value from different variables of different types, which closes the hole. I think that its important that all type parameters are resolved in 4 and 5, otherwise it's still possible to create aliases with different types.
Examples for rule 4 and 5:
marcospassos commentedon Apr 22, 2022
In my experience, most use cases fall into the argument narrowing use case.
As discussed with @ondrejmirtes here, most programming languages with generic support can proper infer cases like the following:
Unfortunately, casting is the only workaround available, but it introduces many unnecessary variable assignments and, consequently, a cognitive burden.
marcospassos commentedon Apr 27, 2022
This example shows how painful this limitation is currently:
To make it work today, you need this:
As expected, due to the same issue, this also doesn't work:
michelv commentedon Sep 9, 2022
Hi! Could this issue be the cause of the 2 errors here? https://phpstan.org/r/814c4824-d65a-4108-8547-59058c1c901f
The errors:
muglug commentedon Sep 13, 2022
I opened a duplicate issue in Psalm's repo: vimeo/psalm#8482
Hack solves this with the solution I give there, but I'm hesitant to copy that implementation because the type hole is a pretty small edge-case.
marcospassos commentedon Sep 13, 2022
We've several cases like the one I've shared in a single application and dozens throughout the codebase. I don't think it's uncommon at all.
muglug commentedon Sep 13, 2022
The edge-case I was talking about is the false-negative I mention in the linked issue. This issue also mentions a number of false-positives in PHPStan (some of which also appear in Psalm), but I wasn't talking about those. In a tool that has few of either, I see false-negatives as inherently more dangerous than false-positives.
ondrejmirtes commentedon Sep 21, 2022
I consider the implementation suggestion above with
Collection<unresolved>etc. to be a huge deal, because once implemented, it will allow us to get rid of generic types generalization (where1becomes anint). More and more issues are being reported connected to that problem.Closing a small type hole is just a small part of that...
ondrejmirtes commentedon Dec 15, 2022
After this we'll also be able to do phpstan/phpstan-src#2110.
phpstan-bot commentedon Sep 4, 2024
@marcospassos After the latest push in 2.0.x, PHPStan now reports different result with your code snippet:
Full report
PHP 8.0 – 8.1 (1 error)
Parameter #1 $collection of method Example<T>::addAll() expects Collection<T>, Collection<mixed> given.PHP 7.1 – 7.4 (3 errors)
PHPDoc tag @param for parameter $value with type T is not subtype of native type mixed.Parameter $value of method Collection::add() has invalid type mixed.Parameter #1 $collection of method Example<T>::addAll() expects Collection<T>, Collection<mixed> given.phpstan-bot commentedon Sep 16, 2024
@michelv After the latest push in 2.0.x, PHPStan now reports different result with your code snippet:
Full report
Method MyTest::provideSets1() should return Ds\Set<mixed> but returns Ds\Set<'a'>.Method MyTest::provideSets2() should return Ds\Set<string> but returns Ds\Set<'a'>.phpstan-bot commentedon Sep 22, 2024
@arnaud-lb After the latest push in 2.0.x, PHPStan now reports different result with your code snippet:
Full report
Parameter #1 $c of function takeCollection expects Collection<int>, Collection<mixed> given.Dumped type: Collection<mixed>Function makeCollection() should return Collection<string> but returns Collection<mixed>.github-actions commentedon Oct 9, 2026
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.