`ParameterBag->keys()` return type is incorrect

[nreynis]

trafficstars

In the ParameterBag stub the return type is improperly narrowed to list<string>.

The implementation use a simple array_keys call. Therefore the return type MUST be list<array-key>.

With the current type narrowing it's impossible to correctly parse a query string with numeric keys (ie: https://domain.tld/search?0=value&78=something). Worse, trusting PHPStan than the key is a string might lead to a runtime TypeError.

[staabm]

//cc @stof as you added the more precise type with https://github.com/phpstan/phpstan-symfony/commit/b055cab8b0dc01fb462220e3796ace29d9dab27d back then. wdyt?

[stof]

The ParameterBag is documented as having string keys. See also @implements \IteratorAggregate<string, mixed> in the same stub.

btw, this case where PHP automatically casts numeric-string keys into an integer is a case where using strict_types in PHP is actually breaking things (without strict_types, PHP would safely cast that integer back to a numeric string when using it as a string).

[staabm]

a case where using strict_types in PHP is actually breaking things

could you give a 3v4l.org example of what you mean?

[nreynis]

The ParameterBag is documented as having string keys. See also @implements \IteratorAggregate<string, mixed> in the same stub.

Which is also incorrect. It would be more accurate to use \IteratorAggregate<string|int, mixed>. While the original intent may have been to use strings as keys, in practice the code allows both strings and integers at runtime.

From a static analysis perspective, runtime behavior is the only thing that should matter. I want PHPStan to help me identify defects not to obscure them by relying on idealized type annotations that doesn't reflect actual behavior.

This might warrant a follow-up issue in symfony/http-foundation to either:

• Force cast the key to strings (a potentially breaking change), or
• Document the keys as string|int

btw, this case where PHP automatically casts numeric-string keys into an integer is a case where using strict_types in PHP is actually breaking things (without strict_types, PHP would safely cast that integer back to a numeric string when using it as a string).

This nuance is irrelevant in this context. The fact that you don’t get a TypeError without strict_types doesn’t change the underlying issue: the returned value is of a different type, and PHPStan should account for that.

[stof]

This nuance is irrelevant in this context. The fact that you don’t get a TypeError without strict_types doesn’t change the underlying issue: the returned value is of a different type, and PHPStan should account for that.

Are you then forbidding all usages of array<string, T> in your projects ? Technically, there is no way to ensure keys are always strings in a PHP array, unless you forbid those strings to be numeric strings.

[nreynis]

Yes, if I cannot ensure the keys will be strings I do not typehint them as strings.

This PHP behaviour is unfortunate but we have to deal with the language quirks. Array keys that are external/unsafe inputs should always be treated as string|int.

[github-actions[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.