Skip to content

Commit 7f70b4d

Browse files
github-actions[bot]VincentLanglet
authored andcommitted
Fix false positive on array item modification with non-constant key
- When modifying a sub-key of an array element at a non-constant offset (e.g. $list[$k]['test'] = true), PHPStan incorrectly replaced the item type for ALL elements instead of unioning with the original item type - Added shouldUnionExistingItemType() check in AssignHandler to detect when the composed value changes existing constant-array key values, forcing a union to preserve unmodified elements' types - Updated test expectations in bug-11679 and slevomat-foreach tests to reflect the more correct union behavior - New regression test in tests/PHPStan/Analyser/nsrt/bug-8270.php Closes phpstan/phpstan#8270
1 parent f077b36 commit 7f70b4d

4 files changed

Lines changed: 87 additions & 4 deletions

File tree

src/Analyser/ExprHandler/AssignHandler.php

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -940,7 +940,9 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar
940940
$offsetValueTypeStack[] = $offsetValueType;
941941
}
942942

943-
foreach (array_reverse($offsetTypes) as $i => [$offsetType]) {
943+
$reversedOffsetTypes = array_reverse($offsetTypes);
944+
$lastOffsetIndex = count($reversedOffsetTypes) - 1;
945+
foreach ($reversedOffsetTypes as $i => [$offsetType]) {
944946
/** @var Type $offsetValueType */
945947
$offsetValueType = array_pop($offsetValueTypeStack);
946948
if (
@@ -981,7 +983,11 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar
981983
}
982984

983985
} else {
984-
$valueToWrite = $offsetValueType->setOffsetValueType($offsetType, $valueToWrite, $i === 0);
986+
$unionValues = $i === 0;
987+
if (!$unionValues && $i === $lastOffsetIndex && $offsetType !== null) {
988+
$unionValues = $this->shouldUnionExistingItemType($offsetValueType, $valueToWrite);
989+
}
990+
$valueToWrite = $offsetValueType->setOffsetValueType($offsetType, $valueToWrite, $unionValues);
985991
}
986992

987993
if ($arrayDimFetch === null || !$offsetValueType->isList()->yes()) {
@@ -1076,4 +1082,34 @@ private function isSameVariable(Expr $a, Expr $b): bool
10761082
return false;
10771083
}
10781084

1085+
/**
1086+
* When modifying a nested array dimension with a non-constant key,
1087+
* check if the composed value changes any existing constant-array
1088+
* key values. If it does, the existing item type should be unioned
1089+
* because unmodified elements still have their original types.
1090+
*/
1091+
private function shouldUnionExistingItemType(Type $offsetValueType, Type $composedValue): bool
1092+
{
1093+
$existingItemType = $offsetValueType->getIterableValueType();
1094+
1095+
if (!$existingItemType->isConstantArray()->yes() || !$composedValue->isConstantArray()->yes()) {
1096+
return false;
1097+
}
1098+
1099+
foreach ($existingItemType->getConstantArrays() as $existingArray) {
1100+
foreach ($existingArray->getKeyTypes() as $i => $keyType) {
1101+
$existingValue = $existingArray->getValueTypes()[$i];
1102+
if ($composedValue->hasOffsetValueType($keyType)->no()) {
1103+
continue;
1104+
}
1105+
$newValue = $composedValue->getOffsetValueType($keyType);
1106+
if (!$newValue->isSuperTypeOf($existingValue)->yes()) {
1107+
return true;
1108+
}
1109+
}
1110+
}
1111+
1112+
return false;
1113+
}
1114+
10791115
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug8270;
6+
7+
use function PHPStan\Testing\assertType;
8+
9+
function () {
10+
/** @var non-empty-list<array{test: false, value: int}> $list */
11+
$list = [];
12+
$list[0]['test'] = true;
13+
14+
foreach ($list as $item) {
15+
assertType('array{test: bool, value: int}', $item);
16+
if ($item['test']) {
17+
assertType('true', $item['test']);
18+
echo $item['value'];
19+
}
20+
}
21+
};
22+
23+
function () {
24+
$list = [];
25+
26+
for ($i = 0; $i < 10; $i++) {
27+
$list[] = [
28+
'test' => false,
29+
'value' => rand(),
30+
];
31+
}
32+
33+
if ($list === []) {
34+
return;
35+
}
36+
37+
$k = array_key_first($list);
38+
assertType('int<0, max>', $k);
39+
$list[$k]['test'] = true;
40+
41+
foreach ($list as $item) {
42+
assertType('array{test: bool, value: int<0, max>}', $item);
43+
if ($item['test']) {
44+
echo $item['value'];
45+
}
46+
}
47+
};

tests/PHPStan/Rules/Arrays/data/bug-11679.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function sayHello(int $index): bool
3131
assertType('array<int, array{foo?: bool}>', $this->arr);
3232
if (!isset($this->arr[$index]['foo'])) {
3333
$this->arr[$index]['foo'] = true;
34-
assertType('non-empty-array<int, array{foo: true}>', $this->arr);
34+
assertType('non-empty-array<int, array{foo?: bool}>', $this->arr);
3535
}
3636
assertType('array<int, array{foo?: bool}>', $this->arr);
3737
return $this->arr[$index]['foo']; // PHPStan does not realize 'foo' is set

tests/PHPStan/Rules/Arrays/data/slevomat-foreach-array-key-exists-bug.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public function doFoo(array $percentageIntervals, array $changes): void
1818
assertType('non-empty-array<array{itemsCount: mixed, interval: mixed}>', $intervalResults);
1919
assertType('array{itemsCount: mixed, interval: mixed}', $intervalResults[$key]);
2020
$intervalResults[$key]['itemsCount'] += $itemsCount;
21-
assertType('non-empty-array<array{itemsCount: (array|float|int), interval: mixed}>', $intervalResults);
21+
assertType('non-empty-array<array{itemsCount: mixed, interval: mixed}>', $intervalResults);
2222
assertType('array{itemsCount: (array|float|int), interval: mixed}', $intervalResults[$key]);
2323
} else {
2424
assertType('array<array{itemsCount: mixed, interval: mixed}>', $intervalResults);

0 commit comments

Comments
 (0)