Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Type/ArrayType.php
Original file line number Diff line number Diff line change
Expand Up @@ -484,10 +484,10 @@ public function fillKeysArray(Type $valueType): Type
return $stringKeyType;
}

return new ArrayType($stringKeyType, $valueType);
return new ArrayType($stringKeyType->toArrayKey(), $valueType);
}

return new ArrayType($itemType, $valueType);
return new ArrayType($itemType->toArrayKey(), $valueType);
}

public function flipArray(): Type
Expand Down
2 changes: 1 addition & 1 deletion src/Type/MixedType.php
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ public function fillKeysArray(Type $valueType): Type
return new ErrorType();
}

return new ArrayType($this->getIterableValueType(), $valueType);
return new ArrayType($this->getIterableValueType()->toArrayKey(), $valueType);
}

public function flipArray(): Type
Expand Down
4 changes: 2 additions & 2 deletions src/Type/Php/ArrayCombineHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ public function getReturnAndThrowType(Expr $firstArg, Expr $secondArg, Scope $sc
return [new NeverType(), TrinaryLogic::createNo()];
}

$keyType = $itemType->toString();
$keyType = $itemType->toString()->toArrayKey();
} else {
$keyType = $itemType;
$keyType = $itemType->toArrayKey();
}
} else {
$keyType = new MixedType();
Expand Down
4 changes: 2 additions & 2 deletions tests/PHPStan/Analyser/nsrt/array-combine-php8.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ function withUnionConstArraysDifferentArraysCount(): void
}
}

assertType("non-empty-array<1|'2'|'3', 'apple'|'avocado'|'banana'|'pear'>", array_combine($a, $b));
assertType("non-empty-array<1|2|3, 'apple'|'avocado'|'banana'|'pear'>", array_combine($a, $b));
}

function withUnionConstArraysAndDifferentFiniteKeysCount(bool $bool): void
Expand All @@ -123,7 +123,7 @@ function withUnionConstArraysAndDifferentFiniteKeysCount(bool $bool): void
$b = ['apple', 'banana'];
}

assertType("non-empty-array<''|'1'|'2', 'apple'|'avocado'|'banana'>", array_combine($a, $b));

Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Collaborator Author

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:113 and :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-constant numeric-string inputs. All already normalize:

path result
array_fill_keys, array_combine, array_flip, array_count_values ✅ (this PR + existing toArrayKey())
array_column (index_key) ✅ via castToArrayKeyType()
iterator_to_array ✅ explicit toArrayKey()
key-preserving fns (array_merge/_replace/_filter/_slice/_chunk/_unique/_reverse/_diff_key/_pad, +) ✅ keys inherited already-normalized
PHPDoc array{'1': string}, array<'1'|'2', string>, unsealed tails ✅ TypeNodeResolver
literals, const, class constants, foreach key, array_key_first/last ✅ ConstantArrayTypeBuilder::setOffsetValueType() normalizes unconditionally

The 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_flip had no int-string coverage at all despite ArrayType::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_flip([1.5])   // runtime: []   PHPStan: array{1: 0}
array_flip([true])  // runtime: []   PHPStan: array{1: 0}

array-count-values.php already 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.

assertType("non-empty-array<1|2|'', 'apple'|'avocado'|'banana'>", array_combine($a, $b));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/nsrt/array-fill-keys-php8.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
function mixedAndSubtractedArray($mixed): void
{
if (is_array($mixed)) {
assertType("array<mixed, 'b'>", array_fill_keys($mixed, 'b'));
assertType("array<'b'>", array_fill_keys($mixed, 'b'));
} else {
assertType("*NEVER*", array_fill_keys($mixed, 'b'));
}
Expand Down
8 changes: 4 additions & 4 deletions tests/PHPStan/Analyser/nsrt/array-fill-keys.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,17 +91,17 @@ function withNotConstantArray(array $foo, array $bar, array $baz, array $floats,
assertType("array<string, null>", array_fill_keys($foo, null));
assertType("array<int, null>", array_fill_keys($bar, null));
assertType("array<'foo', null>", array_fill_keys($baz, null));
assertType("array<numeric-string&uppercase-string, null>", array_fill_keys($floats, null));
assertType("array<bool|int|string, null>", array_fill_keys($mixed, null));
assertType("array<int|(numeric-string&uppercase-string), null>", array_fill_keys($floats, null));
assertType("array<int|string, null>", array_fill_keys($mixed, null));
assertType('array<string, null>', array_fill_keys($list, null));
assertType('*ERROR*', array_fill_keys($objectsWithoutToString, null));

if (array_key_exists(17, $mixed)) {
assertType('non-empty-array<bool|int|string, null>', array_fill_keys($mixed, null));
assertType('non-empty-array<int|string, null>', array_fill_keys($mixed, null));
}

if (array_key_exists(17, $mixed) && $mixed[17] === 'foo') {
assertType('non-empty-array<bool|int|string, null>', array_fill_keys($mixed, null));
assertType('non-empty-array<int|string, null>', array_fill_keys($mixed, null));
}
}

Expand Down
82 changes: 82 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-14980.php
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);
}

}
Loading