Prevent array{a: Foo} going cleanly into array<Foo>

[muglug]

With this PR the following unsound code:

/**
 * @param array{a: float, b: float} $params
 */
function avg(array $params): void {
  takesArrayOfFloats($params);
}

/**
 * @param array<array-key, float> $arr
 */
function takesArrayOfFloats(array $arr): void {
    foreach ($arr as $a) {
        echo $a;
    }
}

now generates the error:

Argument 1 of takesArrayOfFloats expects array<array-key, float>, but parent type array{a: float, b: float} provided

Internal changes

TKeyedArray gets a simplified signature since $sealed always implied a null key/value param.

Future work

We need to decide if we want to change the behaviour of array{a: float, b: float} to be closed by default.

[othercorey]

@muglug Could you explain why a string key isn't valid for array-key?

[weirdan]

@othercorey it's not about the key type (in this example). It's about unknown values that may be present in the unsealed shape. E.g. Psalm currently sees nothing wrong with https://psalm.dev/r/fb73a41c80, but it actually produces a fatal error: https://3v4l.org/b7XcPK

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/fb73a41c80

<?php declare(strict_types=1);
/**
 * @param array{a: float, b: float} $params
 */
function f(array $params): void {
  mapAtan($params);
}

/**
 * @param array<array-key, float> $arr
 * @return array<array-key, float>
 */
function mapAtan(array $arr): array {
    $ret = [];
    foreach ($arr as $k => $v) {
        $ret[$k] = atan($v);
    }
    return $ret;
}

f(['a' => 0.5, 'b' => 10, 'c' => new Exception()]);

Psalm output (using commit 0cd1f13):

No issues!

[othercorey]

@weirdan ah, so array{} doesn't exclude keys not specified? I thought it was always exclusive.

[danog]

Why this change?
Full Union immutability was recently introduced by one of my PRs, since the class is marked as psalm-immutable it makes no sense to clone it (and after I removed all clones in a later-merged PR performance improved by some 15%) :)

[muglug]

That's great! Sadly that PR failed our end-to-end tests, because PSL apparently has a Psalm plugin that calls Union::__clone. Not sure how to work around that — @azjezz do you have ideas?

[muglug]

In the meantime I'm going to merge this — we can always add the trait back once the end-to-end test has been fixed.

[danog]

@azjezz Fixing this is actually pretty simple, just replace the last three lines with return $argument_type->setPossiblyUndefined(true) :)

[azjezz]

@danog Thanks! will do so :)

[danog]

@azjezz You should generally also remove all clones of Unions and atomics from your plugin code :)

[danog]

(Please note that this is only valid for Psalm v5, Psalm v4 still generally requires clones and your legacy logic)

[azjezz]

ops! didn't think of that :D not released broken 2.0.3 :D

[azjezz]

actually, this can't be fixed until Psalm v5 is released, psalm uses a different repository for testing psl ( github.com/psalm/endtoend-test-psl ), which is still in PSL 1.x, the plugin used is also 1.x, so fixing those two for Psalm 5 doesn't seem worth it to me, maybe disable testing on psl for 5.0 for now? i can add it back when psalm 5 is released.

[danog]

I actually already did this in #8697 (basically provided a custom patch file for our version), this was just a heads-up of the required changes for v5 :)