psalm: Psalm 5 breaking changes

  • Redundant casts should become their own issue (currently RedundantCondition)

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Reactions: 2
  • Comments: 43 (12 by maintainers)

Commits related to this issue

Most upvoted comments

= make array shapes explicit — array{foo: Bar} means an array with exactly one element. array{foo: Bar}&array<string, object> can be used to denote an array with optional extra elements

make array shapes explicit

I don’t think this is a good idea. Same as you wouldn’t want to make arrays behave differently in the PHP language, you don’t want to change interpretation of such a basic building block when there’s already a lot of code out there that uses it.

I think the new behaviour needs to be connected with a different type, like strict-array{foo: Bar} or something like that. It’s not beautiful but that’s the price of mistakes of the past 😃

Also, I don’t love the array{foo: Bar}&array<string, object> syntax - without any special handling, this gets normalized to array{foo: Bar} because out of pair of two types, the subtype always wins the intersection.

Maybe something more similar to TypeScript’s notation would be better, maybe: array{foo: Foo, [k: string]: Bar}. More about it here: https://www.typescriptlang.org/docs/handbook/2/objects.html#index-signatures

make array shapes explicit

I don’t think this is a good idea. Same as you wouldn’t want to make arrays behave differently in the PHP language, you don’t want to change interpretation of such a basic building block when there’s already a lot of code out there that uses it.

I think the new behaviour needs to be connected with a different type, like strict-array{foo: Bar} or something like that. It’s not beautiful but that’s the price of mistakes of the past 😃

Also, in the same way that a method which accept a class Foo accept all the class extending Foo, I find more natural that array{foo: string} accepts array with extra values.

I would prefer strict-array

The best move I see is

  • Keeping array untouched to avoid the BC-break
  • Introducing something like strict-array in both Psalm and PHPStan
  • Waiting for PHPStan to make a move about intersection and following the syntax.

I was quite fond of array{} being strict by default (because I feel that’s the most intuitive for clean code, someone who makes the effort of describing the structure seems likely to describe the whole array most of the times), so maybe lax-array instead of strict-array would be better.

Most of the time, when I describe the array shape, it’s because I need to access to some values of this array. I wrote tons of array-shape for my work project and open-source project, and never had the need for a strict definition. For instance

/** @param array{id: string, name: string} */
public function getFoo($array): string {
    return new Foo($array['id'], $array['name']);
}

works fine if the array has an extra key foo.

The same way if your method accept a class A, it accepts a class B if it’s extends A. Here I could consider that array{foo: int, bar: int} extends array{foo: int}.

Saying that it’s intuitive that array{} is strict seems really opinionated to me. I would vote for strict-array.

It also keep consistency with the fact we’re adding constraint to the type. array => non-empty-array is stricter. string => numeric-string is stricter. array => array<string> is stricter. array => strict-array would follow the same pattern.

About the issue with intersection, what about changing

array{foo: Bar}&array<string, object>

to

array{foo: Bar}<string, object>

?