Open
Description
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
.
Metadata
Metadata
Assignees
Labels
No labels
Activity
[-]ParameterBag->keys return type is incorrect[/-][+]`ParameterBag->keys()` return type is incorrect[/+]staabm commentedon May 1, 2025
//cc @stof as you added the more precise type with b055cab back then. wdyt?
stof commentedon May 13, 2025
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 commentedon May 13, 2025
could you give a 3v4l.org example of what you mean?
nreynis commentedon May 13, 2025
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:string|int
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.ParameterBag
typehints are incorrect symfony/symfony#60411stof commentedon May 14, 2025
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 commentedon May 14, 2025
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
.