Repository navigation
Normalize array keys through toArrayKey() in array_fill_keys and array_combine return types
#6078
Next
Next commit
Normalize array keys through
toArrayKey() in array_fill_keys and …
…`array_combine` return types - `ArrayType::fillKeysArray()` now passes the resulting key type through `toArrayKey()`, so `decimal-int-string` (and other integer-like string keys) collapse to `int`, matching how PHP coerces array keys. This mirrors what `ArrayType::flipArray()` already does. - `MixedType::fillKeysArray()` normalizes the key type the same way, so `array_fill_keys` on a `mixed` array no longer yields a `mixed` key type (array keys can only be `int|string`). - `ArrayCombineHelper` (the non-constant path of `array_combine`) had the identical missing normalization and is fixed the same way. - Probed the other value-as-key functions: `array_flip`, `array_count_values` and `array_column` already normalize via `toArrayKey()` and were correct; no change needed there. - Updated existing `array-fill-keys` / `array-combine` expectations that encoded the un-normalized keys (float keys now include `int`, `bool` keys collapse to `int|string`, decimal-int-string keys collapse to `int`).
- Loading branch information
commit 6810d228c8679a6a597912f5032a13f31431f11f
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| <?php // lint >= 8.0 | ||
|
|
||
| declare(strict_types = 1); | ||
|
|
||
| namespace Bug14980; | ||
|
|
||
| use function PHPStan\Testing\assertType; | ||
|
|
||
| /** | ||
| * @param list<int> $ints | ||
| */ | ||
| function decimalIntStringKeys(array $ints): void | ||
| { | ||
| $keys = []; | ||
| foreach ($ints as $int) { | ||
| $keys[] = $int; | ||
| $keys[] = (string) $int; | ||
| } | ||
|
|
||
| assertType('list<int|decimal-int-string>', $keys); | ||
| assertType('array<int, true>', array_fill_keys($keys, true)); | ||
| } | ||
|
|
||
| /** | ||
| * @param list<numeric-string> $keys | ||
| */ | ||
| function numericStringKeys(array $keys): void | ||
| { | ||
| assertType('array<int|numeric-string, true>', array_fill_keys($keys, true)); | ||
| } | ||
|
|
||
| function fillKeysMixedArray(mixed $m): void | ||
| { | ||
| if (is_array($m)) { | ||
| assertType('array<true>', array_fill_keys($m, true)); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @param list<int> $ints | ||
| */ | ||
| function combineDecimalIntStringKeys(array $ints): void | ||
| { | ||
| $keys = []; | ||
| foreach ($ints as $int) { | ||
| $keys[] = $int; | ||
| $keys[] = (string) $int; | ||
| } | ||
|
|
||
| assertType('array<int, int|decimal-int-string>', array_combine($keys, $keys)); | ||
| } | ||
|
|
||
| /** | ||
| * @param list<numeric-string> $keys | ||
| * @param list<string> $values | ||
| */ | ||
| function combineNumericStringKeys(array $keys, array $values): void | ||
| { | ||
| assertType('array<int|numeric-string, string>', array_combine($keys, $values)); | ||
| } | ||
|
|
||
| class DecimalIntBug | ||
| { | ||
|
|
||
| /** @var array<int, true> */ | ||
| private array $data = []; | ||
|
|
||
| /** | ||
| * @param list<int> $ints | ||
| */ | ||
| public function set(array $ints): void | ||
| { | ||
| $keys = []; | ||
| foreach ($ints as $int) { | ||
| $keys[] = $int; | ||
| $keys[] = (string) $int; | ||
| } | ||
|
|
||
| $this->data = array_fill_keys($keys, true); | ||
| } | ||
|
|
||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have more tests in the codebase with constant int-strings in array keys?
If so, these would need a similar fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed as
c03c891f7.Answer to the review question
No other tests need a fix. I audited this two ways:
1. Mechanical sweep of every test expectation. I wrote a parser that extracts
array<…>/non-empty-array<…>/iterable<…>generics from all test files, splits the top-level type arguments, and flags any key position containing a quoted decimal-int string — plus a grep for the constant-array-shape form (array{'1': …}). Validated it against the pre-fix tree: it found exactly the 2 lines this PR already changed (array-combine-php8.php:113and:126). On the current tree: 0 hits. So those two were the only affected expectations in the repo.2. Empirical probe of the source paths (in case a path had no test at all). I dumped types for every array function that turns values into keys, plus the other ways an int-string key can enter a type, with both constant
'1'|'2'and non-constantnumeric-stringinputs. All already normalize:array_fill_keys,array_combine,array_flip,array_count_valuestoArrayKey())array_column(index_key)castToArrayKeyType()iterator_to_arraytoArrayKey()array_merge/_replace/_filter/_slice/_chunk/_unique/_reverse/_diff_key/_pad,+)array{'1': string},array<'1'|'2', string>, unsealed tailsTypeNodeResolverconst, class constants,foreachkey,array_key_first/lastConstantArrayTypeBuilder::setOffsetValueType()normalizes unconditionallyThe one real gap I found is
ConstantArrayTypeBuilder::makeUnsealed(), which does not normalize — callers must. I checked all 30 call sites; every value-derived one (ArrayCombineHelper:82,ConstantArrayType::fillKeysArray/flipArray) normalizes, the rest propagate already-normalized keys.Added coverage
Since the sweep showed the constant-int-string shapes were only covered incidentally, I pinned them explicitly. Verified they fail on the pre-fix tree for the right reason (
array<'1'|'2', 'b'>→array<1|2, 'b'>), so they're genuine guards, not just green tests.array_fliphad no int-string coverage at all despiteArrayType::flipArray()already normalizing — added it so the sibling path is locked in too.make tests: 17608 tests OK (97 skipped).make phpstan: no errors.Separate bug found, deliberately not in this PR
array_flip()ignores float and bool values at runtime (warns, skips), but PHPStan models them as keys:array-count-values.phpalready documents and handles exactly this (array{}for[true, 1.0, false, 0.0, null]);flipArray()never got the equivalent. It's a different defect from key normalization, so I left this PR focused — happy to open it separately.