Typing a handrolled collection when it's empty? #6731

00dani · 2022-03-04T04:11:52Z

00dani
Mar 4, 2022

Hi! I'm currently experimenting with how PHPStan treats iterable<int, T>, because I'm thinking about writing a collection library that separates lists from dictionaries - so you'd have Sequence<T> implements iterable<int, T> and Dictionary<T> implements iterable<string, T>. However, I've noticed PHPStan handles compatibility between iterable and specific collection types a little strangely.

In particular, if i write this, PHPStan 1.4.7 approves and it passes type checking:

/** @var iterable<int, Friend> **/
private iterable $friends;

public function __construct() {
  $friends = new \ArrayObject();
  \PHPStan\dumpType($friends);
  $this->friends = $friends;
}

The dumped type is ArrayObject<(int|string), mixed>.

But if I write this, then I get a type error:

/** @var iterable<int, Friend> **/
private iterable $friends;

public function __construct() {
  $friends = new \Doctrine\Common\Collections\ArrayCollection();
  \PHPStan\dumpType($friends);
  $this->friends = $friends;
}

The dumped type is now Doctrine\Common\Collections\ArrayCollection<(int|string), mixed>, and the error tells me that Property Test::$friends (iterable<int, Friend>) does not accept Doctrine\Common\Collections\ArrayCollection<(int|string), mixed>..

Both classes are iterable by virtue of implementing IteratorAggregate, and when they're both empty and unreferenced it's definitely typesafe to narrow the generic parameters from <(int|string), mixed> to <int, Friend>. I do have a local variable $friends in both cases, but it doesn't actually make a difference: identical behaviour occurs if I simply write $this->friends = new ClassBeingTested() instead.

So, why does PHPStan know that an brand-new empty ArrayObject can safely be narrowed to an iterable<int, Friend>, but doesn't know that a brand new empty Doctrine ArrayCollection can be narrowed in exactly the same way? I've examined the PHPDoc types for both classes carefully, as well as the function map and other stubs for ArrayObject, and I couldn't find anything that gives an ArrayObject this special behaviour.

Ideally, I'd like my own sequence type to behave the way ArrayObject does, so that you can write simple things like $this->friends = Sequence::empty() rather than /**@var Sequence<Friend> */ $friends = Sequence::empty(); $this->friends = $friends;, so I need to know how that behaviour is achieved. Is it a hardcoded special case, or is there a type annotation I can use that I haven't managed to find?

Answered by ondrejmirtes

Mar 4, 2022

Coincidentally, I made a change last night and you won't get an error on dev-master, feel free to try it out :) composer require --dev phpstan/phpstan:dev-master

The change is that new \Doctrine\Common\Collections\ArrayCollection(); leads to ArrayCollection<*NEVER*, *NEVER*> which then gets accepted into the property.

View full answer

ondrejmirtes · 2022-03-04T09:23:11Z

ondrejmirtes
Mar 4, 2022
Maintainer

Coincidentally, I made a change last night and you won't get an error on dev-master, feel free to try it out :) composer require --dev phpstan/phpstan:dev-master

The change is that new \Doctrine\Common\Collections\ArrayCollection(); leads to ArrayCollection<*NEVER*, *NEVER*> which then gets accepted into the property.

4 replies

00dani Mar 4, 2022
Author

Oh, that makes sense! I experimented with running Psalm over similar code and noticed it inferred an ArrayCollection<empty> type, which I believe is treated similarly to PHPStan's never.

Should I expect a new release soon with this extra functionality? 🐱

ondrejmirtes Mar 4, 2022
Maintainer

Yeah, most likely later today :)

00dani Mar 4, 2022
Author

Just tried this out, and my examples pass as expected on 445c882. Awesome.

This change does however open up a hole in the type system:

/** @var Collection<int, int> $ints */
public iterable $ints;

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

public function __construct() {
	$array = new ArrayCollection();
	$this->ints = $array;
	$this->strings = $array;
}

PHPStan has no complaints about this code. However both $ints and $strings point to the same object, so you can store both integers and strings in the underlying collection. This allows any type to be statically cast to any other type, without actually converting the runtime value.

I suspect this hole isn't a major concern? One would naturally write code like $this->ints = new ArrayCollection() in the constructor anyway, which cannot lead to this issue. Additionally, I found in testing out Psalm that it has exactly the same hole in its type system. Nonetheless, it's there and probably worth mentioning.

If possible, I would suggest that the assignment $this->ints = $array should narrow the statically inferred types of both $this->ints and $array. It appears that currently the static type of $this->ints is basically "overwritten" with the static type of $array for the rest of the method, which does make sense in most cases but leads to odd behaviour when generics are involved.

ondrejmirtes Mar 4, 2022
Maintainer

If possible, I would suggest that the assignment $this->ints = $array should narrow the statically inferred types of both $this->ints and $array. It appears

Yes, makes sense! :) Please open a feature request about that.

arnaud-lb · 2022-03-05T16:54:02Z

arnaud-lb
Mar 5, 2022

This is an interesting problem. One workaround I've been using is to make the empty constructor aware of the types I want:

Sequence::of(Friend::class);

Example: https://phpstan.org/r/49321c40-21e6-47c6-99b3-ad5de947f4a7

This allows to fully resolve the type parameters immediately, even on an empty container.

Typescript appears to infer the type parameters from the destination that receives the instantiation, but will refuse to narrow the types after that: https://www.typescriptlang.org/play?#code/FDDGBsEMGdoAgMIB4AqA+OBvYddwKbj4C20AXHCgNoC6OeoA9gHbQAuATgK6huMcAKAJRZ6ePGwAWAS2gA6QiXgBeOLTFwAvhoAOXaJIH4KKEdnHipshUVJy9Bo0I3btYKLDgARUeOZdiACN8DnJEJH8gkLQNdg5pZgBzMOQ4hMSY8SZWTh4+QTMNLJZ2OFA4VWZ8AHdEYQBuIokZeUjg0IqyptwreTSklS7xVxAIGHgAUV88BLYwtpD1cX7kihWlhhLc3n5haYsyrbLOjYPeuVnB0G64c5Wrl2A3Mc8AMX24BdCKZC-MvHuPyQK3+uGycTyu0KBzBR1AQL+nSqtQQwhu5y+DxhtxacnuSJqdREAHpieF7qCtE8QAAzLjMXjSFhwGkCeHkzjpNBCCgAN0Y0gAJqJqcBWciiSSyalOUkYkA

As a result, this works only when passing the instantiation result to something with a known type. This seems fine, because the syntax allows to express the type of a variable: const c: C<number> = new C();.

In PHPStan, one solution could be to delay type inference until we are confronted to actual types:

Make unresolved type parameters explicit in the engine, such that new Sequence() is a Sequence<unresolved>
Resolve the unresolved type parameters the first time they are passed to something

This would work for assignments to properties:

$c = new Collection();
dumpType($c);           // Collection<unresolved>
$this->strings = $c;    // $c is now a Collection<string>
$this->ints = $c;       // error: Collection<int> does not accept Collection<string>

For arguments:

$c = new Collection();
initStrings($c); // $c is now a Collection<string>

Maybe for method calls as well, although in this case the type inference may be partial:

$c = new Collection();  // Collection<unresolved>
$c->push("hello");      // $c is now a Collection<string>

For variables, although in this case the type parameters should be inferred to mixed:

$c = new Collection();
$d = $c; // both $d and $c are now a Collection<mixed>

Same when passing the object to a generic method:

function gen<T>(Collection<T> $c);
gen($c); // $c is now a Collection<mixed>

References to $this could be a problem unless method calls resolve all type parameters.

Otherwise this seems safe, since the instant the variable is referenced, type parameters get resolved. So there is no way that it gets aliased with a different type.

It's possible that I'm missing some case that will not work, though.

WDYT ?

Are you aware of type systems that resolved this problem ? Would be interesting to see other approaches.

2 replies

00dani Mar 8, 2022
Author

Hmm. I do like the workaround you proposed right at the top, where you write Sequence::of(Friend::class) in the first place and therefore have a well-defined type for an empty collection. The only problem there is that, because PHP doesn't natively have type parameters, you can't define empty sequences with types like Sequence<Sequence<int>>.

I believe the *NEVER* type that @ondrejmirtes recently introduced is functionally fairly similar to the unresolved type you propose, as well as being analogous to the empty type in Psalm. It works, but in its current implementation in both PHPStan and Psalm it opens a hole in the type system - I have proposed a solution for this.

TypeScript doesn't have these issues, and also doesn't have the type system hole I describe above. In TypeScript, this code works and is well-typed:

class Sequence<T> {
  storage: T[] = [];
}

class Test {
  ints: Sequence<number>;
  strings: Sequence<number>;
  constructor() {
    this.ints = new Sequence();
    this.strings = new Sequence();
  }
}

While this code correctly produces static type errors:

class Test {
  ints: Sequence<number>;
  strings: Sequence<number>;
  constructor() {
    const seq = new Sequence(); // seq has type Sequence<unknown>
    this.ints = seq; // Type 'Sequence<unknown>' is not assignable to type 'Sequence<number>'
    this.strings = seq; // Type 'Sequence<unknown>' is not assignable to type 'Sequence<string>'
  }
}

TypeScript however does not let you do this, which is also well-typed and is allowed by PHPStan and Psalm. The assignment can safely narrow the type of this.ints, since there's no possible way the property could be changed until the constructor finishes.

class MyCoolSequence<T> extends Sequence<T> {
  foo() {}
}
class Test {
  ints: Sequence<number>;
  constructor() {
    this.ints = new MyCoolSequence();
    this.ints.foo(); // Property 'foo' does not exist on type 'Sequence<number>'
  }
}

arnaud-lb Mar 8, 2022

Indeed, the solution I proposed is totally inspired by both of your solutions. After reading your issue carefully I believe that what I'm proposing is exactly what you are proposing.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Typing a handrolled collection when it's empty? #6731

Uh oh!

{{title}}

Uh oh!

Replies: 2 comments 6 replies

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{editor}}'s edit

{{editor}}'s edit

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Select a reply

Uh oh!

Uh oh!

Typing a handrolled collection when it's empty? #6731

Uh oh!

00dani Mar 4, 2022

Replies: 2 comments · 6 replies

Uh oh!

ondrejmirtes Mar 4, 2022 Maintainer

Uh oh!

00dani Mar 4, 2022 Author

Uh oh!

ondrejmirtes Mar 4, 2022 Maintainer

Uh oh!

00dani Mar 4, 2022 Author

Uh oh!

ondrejmirtes Mar 4, 2022 Maintainer

Uh oh!

Uh oh!

arnaud-lb Mar 5, 2022

Uh oh!

00dani Mar 8, 2022 Author

Uh oh!

arnaud-lb Mar 8, 2022

00dani
Mar 4, 2022

Replies: 2 comments 6 replies

ondrejmirtes
Mar 4, 2022
Maintainer

00dani Mar 4, 2022
Author

ondrejmirtes Mar 4, 2022
Maintainer

00dani Mar 4, 2022
Author

ondrejmirtes Mar 4, 2022
Maintainer

arnaud-lb
Mar 5, 2022

00dani Mar 8, 2022
Author