Support sealed/unsealed array shapes

[jrmajor]

Feature request

Psalm allows array{foo: int, bar: int, ...} notation to specify that this array may also contain other keys. Array shapes without the ... must not contain any other keys.

This is a feature introduced in Psalm 5. It's described in depth in the announcement post. Here are the examples from the post in PHPStan playground: https://phpstan.org/r/40e730d8-e941-4102-9e76-a07f1c0a2e46.

[stof]

The basic support would be to support parsing the ... syntax while still treating all shapes as unsealed (i.e. the current behavior).
Fully supporting sealed shapes would require a new major version (and so only be available in bleeding-edge)

[jrmajor]

while still treating all shapes as unsealed (i.e. the current behavior)

I think the way PHPStan treats array shapes currently is something in-between sealed and unsealed.

Fully supporting sealed shapes would require a new major version (and so only be available in bleeding-edge)

Yes, but given that currently the unsealed syntax is an error in PHPStan, treating array shapes with ... differently would not be a BC break.

This means we can change sealed shapes semantics only in bleeding edge, but use the new semantics for unsealed shapes right away.

[stof]

If the current behavior is indeed in-between, the unsealed array shapes might indeed be implemented fully already.

[jrmajor]

Here is an example of PHPStan treating array shape as sealed. If that was fully the case though, there would be an error reported in the first function in my original example.

[VincentLanglet]

Not sure if PHPStan should follow something imposed by Psalm 20 days before the major release without discussion when another syntax strict-array was preferred for months and would have avoid BC breaks.

array{key: string}&array<int, int> is another syntax supported by Psalm which is still not supported by PHPStan, I think by choice.

[stof]

The issue is having the same syntax having a different meaning in both tools, as it means a library writing types with one tool in mind is incompatible with the other tool (and projects using the library may choose to use the other tool)

[jrmajor]

@VincentLanglet Hi, thanks for bringing that up. I wasn't aware of the discussions in vimeo/psalm#4700 and vimeo/psalm#8395.

@ondrejmirtes Regarding your comments in these discussions — I would just like to point out that it's not entirely true that array shapes are currently treated as unsealed. I've provided an example in my previous comment, but here is a more entertaining one. The error reported in this example shows that PHPStan treats this shape as a sealed one, but no error reported for the return type of the function implies that it's unsealed. This inconsistency can be fixed by making array shapes consistently fully sealed or fully unsealed. Both solutions are BC breaking, but one of them makes PHPStan compatible with Psalm, and the other one makes it even more incompatible.

Edit: I've made a mistake in that example, which @VincentLanglet pointed out. Here is a corrected one.

[VincentLanglet]

I'm sad the strategy change wasn't more discussed.

When a method is doThings(Foo $foo): Foo nobody is shocked if we pass SuperFoo or if the method returns SuperFoo.
So with doThings(array $foo): array i would have consider natural to accepts bigger arrays or returning bigger arrays.

strict-array or sealed-array seemed more natural to me.
And we could have been asking something like sealed-Foo for classes.

but here is a more entertaining one.

With object, you have https://phpstan.org/r/b808b62f-08a6-4a89-805d-7f64f9ce5855
and nobody complains about this error. And it doesn't mean you can remove the line.
Code must be updated to https://phpstan.org/r/b6a7e4cc-844b-40b0-8b49-76bbf3810d14.

Maybe error message should be improved ?
Maybe https://phpstan.org/r/d228d69a-83a3-46e4-9aed-72ea41ffaba2 should be valid ?
I dunno. On this opposite side, we have
https://phpstan.org/r/bda38be1-9895-4cf5-bb95-0196159bd462 which should report an error
like https://phpstan.org/r/327201df-d8c6-4e02-85d9-6d0006ea33f1 does.

I would just like to point out that it's not entirely true that array shapes are currently treated as unsealed.

I would say that PHPStan tried to take the best of both world (like Hannah Montana 😄).

Most of the time you're only reading the data from a given array shape, so it doesn't require to be sealed.

[stof]

The example given in the psalm doc or release notes (I don't remember which one it is) shows an issue with the unsealed shapes (or the partially unsealed ones): it is not safe to consider array{foo: string, bar: string} as a type accepted by the type array<string, string> (or even string[]) even though all values in the shape are strings because you accept any other key that can have other types. And this does not involve writing into the shape.
Wanting to make array shape sound in in the type system is a worthy goal, and that cannot be achieved with a single type of shapes that is partially sealed (having only sealed shapes would be sound as well, but annoying for some use cases).

[jrmajor]

With object, you have phpstan.org/r/b808b62f-08a6-4a89-805d-7f64f9ce5855
and nobody complains about this error. And it doesn't mean you can remove the line.
Code must be updated to phpstan.org/r/b6a7e4cc-844b-40b0-8b49-76bbf3810d14.

Maybe https://phpstan.org/r/d228d69a-83a3-46e4-9aed-72ea41ffaba2 should be valid ?

@VincentLanglet You're correct, I completely missed a point with that example. What I was intending to show is that checking for key existence in this case will be reported as always false (meaning that the shape is treated as sealed). I've added a corrected example to my original comment.

I would say that PHPStan tried to take the best of both world (like Hannah Montana 😄).

It causes real problems though. When adding type annotations to PHP-CS-Fixer, I've come across something like that and wasn't able to find a solution other than @phpstan-ignore.

[VincentLanglet]

The example given in the psalm doc or release notes (I don't remember which one it is) shows an issue with the unsealed shapes (or the partially unsealed ones): it is not safe to consider array{foo: string, bar: string} as a type accepted by the type array<string, string> (or even string[]) even though all values in the shape are strings because you accept any other key that can have other types.

I didn't said we didn't need to differentiate sealed and unsealed array.
And the syntax ... is interesting.

The Psalm syntax

array{key: string}&array<int, int>

which wasn't supported by PHPStan could be changed to

array{key: string, ...<int, int>}

I was just wondering in which side we had more BC-break.
Psalm can easily introduce BC break with the v5 release, but it forces PHPStan a quick support of the syntax, and a possible behavior change (in bleeding edge) for people which uses both tools. A psalm option to change the behavior, removed in v6, could have introduce less frictions maybe.

Edit: just found another topic about it vimeo/psalm#8744

[stof]

for people which uses both tools

It is not just for people that use both tools. It is for phpstan users relying on a dependency where the maintainer uses psalm (and so uses the psalm semantic of shapes) or the opposite.