Modifier to disallow non-documented keys from array shape

[ntzm]

I would like to be able to do the following:

https://psalm.dev/r/cdf24c2204

The current behaviour is obviously wanted in most cases, but I would like a modifier that disallows non-documented keys from array shapes, something like array-shape-strict{foo: string} or array!{foo: string}

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/cdf24c2204

<?php declare(strict_types = 1);

/** @param array{status?: int} $data */
function create(array $data): void {
	return;
}

create(['status' => 1]);
create([]);
create(['sstatus' => 1]); // I want this to error

Psalm output (using commit ad8c04c):

INFO: UnusedParam - 4:23 - Param $data is never referenced in this method

[muglug]

This was slightly poor design on my part.

We could add a config that would make array{a: int, b: string} strict and array{a: int, b: string}&array the flexible version. In v5 that config could become the default.

[ntzm]

That would work for me

[AndrolGenhald]

I've been thinking the best way to do this would be to do something like TypeScript's index signatures (and maybe eventually mapped types) for keyed arrays. Unsealed arrays (I'm say "sealed" and "unsealed" because that's what Psalm currently calls it. Sealed means no extra keys, unsealed means extra keys are mixed.) are currently the default, but in Psalm 5 (or probably 6 at this point) we could have a breaking change and say unsealed arrays must be represented as array{a: int, [array-key]: mixed}. We could also think about providing a tool to ease the transition by automatically changing array{a: int} to array{a: int, [array-key]: mixed}.

@ondrejmirtes I know we want to make sure Psalm and PHPStan are in agreement on this since it's such an important feature. I saw you had some proposals here, but I'm not much of a fan of array{foo: Bar, []}. I think if we want unsealed arrays to remain the default, a better option would be either strict-array{foo: Bar} or array{foo: Bar, [array-key]: never}, and I'm leaning towards the second option. Maybe we could have a config option for whether arrays are sealed or unsealed by default? That way array{foo: Bar} would use that config option, array{foo: Bar, [array-key]: mixed} would always be unsealed regardless of config, and array{foo: Bar, [array-key]: never} would always be sealed regardless of config.

If we want to go with index types, there are couple of things to consider.

I think it's fine to remove the name (eg array{foo: Bar, [array-key]: mixed} rather than array{foo: Bar, [key: array-key]: mixed}). It seems not to matter for normal index signatures, and while it's useful for mapped types I think we can add syntax for that later while remaining backwards compatible.

Overlapping types: array{a: int, [lowercase-string]: lowercase-string, [string]: string, [int]: int}. Is the type valid? Do earlier keys take precedence? TypeScript requires that defined keys match the index signature, but if we want array{a: int, [array-key]: never} to work I think we have to be more lax. I think actual keys should always take precedence, but I'm not sure what to do about multiple index signatures. Should we limit it to a single index signature to start out, and improve it from there?

[danog]

Before I found the alternative solutions proposed in this issue, I started working on a PR that adds support for explicitly sealed-* types, even though even with sealed types enabled some checks don't work as planned (as also mentioned in a separate comment by @orklah in the PR): #8397

[weirdan]

This is now the default behaviour: https://psalm.dev/r/19e15d40ad

Old-style, unsealed shapes can be expressed using array{T1,T2,T3,...} syntax.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/19e15d40ad

<?php declare(strict_types = 1);

/** @param array{status?: int} $_data */
function create(array $_data): void {
	return;
}

create(['status' => 1]);
create([]);
create(['sstatus' => 1]); // I want this to error

Psalm output (using commit 8f39de9):

ERROR: InvalidArgument - 10:8 - Argument 1 of create expects array{status?: int}, but array{sstatus: 1} provided

[ondrejmirtes]

@weirdan I thought this isn't going to change in Psalm 5. The array shape (the only one that existed in Psalm 4) shouldn't have got any stricter in Psalm 5.

Because:

1. This is not a bug in PHPStan: https://phpstan.org/r/d0de23d5-003d-470e-ad55-3790aea7253f
2. There was supposed to be a new variant called "sealed-array" or "strict-array" introduced with this behaviour by @danog but neither of them works in Psalm playground.

So please can you tell us what's the behaviour supposed to be now?

[bendavies]

REF #8701

[muglug]

This is all on me, but I pushed for this to be clarified in Psalm 5.

Both Phan and PHPStan have unsound behaviour.

In both cases you were following Psalm’s lead, but unsound behaviour is bad and wrong.

The correct behaviour here, IMO, is for array shapes to be sealed by default. I did not want to release another major version with unsound behaviour, and I didn’t want to have everyone start adding sealed-array annotations everywhere, because that looks horrible.

[ondrejmirtes]

I wasn’t aware of any changes because three weeks ago we agreed with orklah that array shapes by default shouldn’t change because of the impact on the ecosystem #8395 (comment) and only sealed/strict variants should have the stricter behaviour. That’s what PHPStan will continue to do.

[muglug]

It's not "stricter" behaviour that we're discussing, though — it's sound vs unsound. Unlike other unsound stuff in typecheckers (like array keys being converted to ints in some cases) this is unsound behaviour introduced by the typecheckers themselves.

Psalm has led the way when it comes to preventing unsound behaviour (e.g. making mixed issues a thing years before anyone else) and this will be another example.

My guilt is that it was an unsoundness introduced by Psalm. This is a chance to remedy that, at the expense of some friction for users.

[muglug]

Psalm also still supports (actually also incorrectly-thought-out) array annotation array{a: Foo, b: Bar}&array for unsealed array shapes. That annotation is (correctly) a no-op in PHPStan.

For users who encounter this issue who want their docblocks to work as-is in both tools, adding &array for unsealed array shapes is the smallest possible change.

[ntzm]

Nice, thank you everyone :)

[ondrejmirtes]

I'll eventually adopt this too, but I'm not gonna hurry, because I can already foresee the amount of support this change will generate for me for several months in a row :)

[bendavies]

An optional bleeding edge or phpstan-strict-rules rule would be great. I'm always after more strictness!

…

On Thu, 24 Nov 2022, 15:55 Ondřej Mirtes, ***@***.***> wrote: I'll eventually adopt this too, but I'm not gonna hurry, because I can already foresee the amount of support this change will generate for me for several months in a row :) — Reply to this email directly, view it on GitHub <#5299 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEYV4F3NZDMEUZ3A6TQ7ATWJ6FXTANCNFSM4YMHLZVQ> . You are receiving this because you commented.Message ID: ***@***.***>