Skip to content

Widen the key type of functions that return an array key - #6222

Open
zonuexe wants to merge 4 commits into
phpstan:2.2.xfrom
zonuexe:fix/issue-15073
Open

Widen the key type of functions that return an array key#6222
zonuexe wants to merge 4 commits into
phpstan:2.2.xfrom
zonuexe:fix/issue-15073

Conversation

@zonuexe

@zonuexe zonuexe commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Closes phpstan/phpstan#15073

Continues the numeric-string / decimal-int-string consistency work of #3326, phpstan/phpstan#6847 and #5328.

Problem

PHP casts a decimal-integer string array key ("123") to int, so a key read out of an array
with a string key type can be an int:

function is_decimal_int_string(mixed $val): bool {
    return is_string($val) && is_int(array_key_first([ $val => null ]));
    // Call to function is_int() with string will always evaluate to false.
}

array_key_last(), array_keys()[0], key(), array_find_key(), array_search() and the values of array_flip() have the same problem.

Change

UnsafeArrayStringKeyCastingTraverser already models this cast for the key type an array has. That type also drives describe(), accepts() and type subtraction, which is why only reportUnsafeArrayStringKeyCasting: detect widens it. I tried widening it with the toggle off: array<string, X> turns into array<X> everywhere and 113 tests fail, including mixed~array<string, mixed> losing every array.

So this adds a second entry point, for keys that leave the array as a value:

method widens toggle off detect
castKeyType() the key type an array has nothing int|non-decimal-int-string
castReadKeyType() a key returned as a value (int|string) same
unionWithReadKeyType() the above, plus the null/false for an empty array keeps the union benevolent same

The benevolent union keeps new errors out: is_int($key) reports maybe instead of "always false", and strlen($key) still passes. unionWithReadKeyType() exists because TypeCombinator::union() drops the benevolence, which would break strlen(array_key_first($a) ?? '') on a possibly empty array<string, …>.

assertType('(int|string)', array_key_first([$string => null]));            // was: string
assertType('non-empty-list<(int|string)>', array_keys([$string => null])); // was: non-empty-list<string>
assertType('int', array_key_first([$decimal_string => null]));
assertType('non-decimal-int-string', array_key_first([$non_decimal_string => null]));

Left alone

  • foreach keys, with the toggle off. You write $result[$k] = … right after, and a benevolent (int|string) key turns that array into array<mixed, …>. Six nsrt files lose their key type that way. Use detect for accurate foreach keys.
  • The key type of arrays that array_flip() and array_fill_keys() build. They keep the raw string, and reading a key back out of them runs the new cast anyway.
  • array_rand(). Same bug, but its result collapses to int|string by construction, so strlen(array_rand($stringKeyed)) would start failing. Separate PR.

Also in here

The first two commits stand on their own:

  • fillKeysArray() on a general array only called toString(), so array_fill_keys() disagreed with array_flip() next to it.
  • getIterableKeyType(), getFirstIterableKeyType() and getLastIterableKeyType() repeated the same unsealed-tail normalization, and the first/last pair missed the detect cast.

Two more things ride along in the third commit. The @var int|string over key([$string => null]) in ArgumentsNormalizer and ConstantStringType covered for this bug; both are gone, with their baseline entries. The $file rename in OptimizedDirectorySourceLocatorFactory is fallout: PHPStan now proves $file defined inside if ($findInFiles !== []), so strict-rules flags the reuse.

Tests

nsrt/bug-15073.php is new and fails on 8 assertions without the src/ change. The detect fixture gets read-key cases. Other expectation updates come in two shapes: int|string|null to (int|string|null) where the benevolence survives now, and string to (int|string) where the fix applies.

zonuexe and others added 2 commits August 14, 2026 03:29
getIterableKeyType(), getFirstIterableKeyType() and getLastIterableKeyType()
each spelled out the same "implicit mixed tail means array-key" normalization.
The first/last pair was also missing the reportUnsafeArrayStringKeyCasting
cast that getIterableKeyType() right next to them already applied.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
fillKeysArray() on a general array only called toString(), so array_fill_keys()
disagreed with array_flip() right next to it: `list<decimal-int-string>` gave
`array<decimal-int-string, …>` instead of `array<int, …>`, and `list<float>`
kept a numeric-string key instead of the int PHP actually stores.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@zonuexe
zonuexe force-pushed the fix/issue-15073 branch 3 times, most recently from 96f72e6 to 1fd8041 Compare August 13, 2026 21:09
zonuexe and others added 2 commits August 14, 2026 13:02
PHP casts a decimal-integer string array key ("123") to int, so
`array_key_first([$string => null])` is not necessarily a string. PHPStan
inferred `string` for it and reported `is_int()` on the result as always false.

UnsafeArrayStringKeyCastingTraverser already modelled that cast, but only for
the key type an array *has* — a type that also decides how the array describes
itself and what it accepts, which is why only
`reportUnsafeArrayStringKeyCasting: detect` widens it. Widening it with the
toggle off turns `array<string, X>` into `array<X>` everywhere.

castReadKeyType() is the second entry point, for a key that leaves the array as
a value of its own. With the toggle off it widens `string` to the benevolent
`(int|string)`, so neither branch reports anything; `detect` and `prevent` keep
the types they have today. unionWithReadKeyType() adds the `null` an empty array
gives back without losing the benevolence on the way.

The remaining accessors follow in the next commit.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
PHP casts a decimal-integer string array key ("123") to int, so
`array_key_first([$string => null])` is not necessarily a string. PHPStan
inferred `string` for it and reported `is_int()` on the result as always false.

array_key_first() and array_key_last() went first; call castReadKeyType() from
the rest of the accessors that hand a key back as a value of its own:
array_keys(), key(), array_find_key(), array_search() and the values of
array_flip().

`foreach` keys stay unwidened with the toggle off: the key usually goes straight
back into another array, and a benevolent `(int|string)` key collapses that
array to `array<mixed, …>`. `detect` remains the level with accurate `foreach`
keys.

The rename in OptimizedDirectorySourceLocatorFactory is fallout: with the wider
`array_keys()` type PHPStan now proves `$file` defined inside
`if ($findInFiles !== [])`, which is correct and makes strict-rules flag the
reuse.

The `@var int|string` workarounds over `key([$string => null])` in
ArgumentsNormalizer and ConstantStringType are what this bug looks like from the
inside; both are redundant now and go away with their baseline entries. The two
calls also read better as array_key_first(), which says what they are after.

Closes phpstan/phpstan#15073

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

array_key_first([<string> => null]) is not always a string.

1 participant