Skip to content

Commit ec82e22

Browse files
staabmstaabm
authored
Fix incorrect narrowing of nested array after assignment (#5258)
1 parent b5b9149 commit ec82e22

File tree

6 files changed

+143
-5
lines changed

6 files changed

+143
-5
lines changed

‎src/Analyser/ExprHandler/AssignHandler.php‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -938,7 +938,7 @@ private function isImplicitArrayCreation(array $dimFetchStack, Scope $scope): Tr
938938

939939
/**
940940
* @param list<ArrayDimFetch> $dimFetchStack
941-
* @param list<array{Type|null, ArrayDimFetch}> $offsetTypes
941+
* @param non-empty-list<array{Type|null, ArrayDimFetch}> $offsetTypes
942942
*
943943
* @return array{Type, list<array{Expr, Type}>}
944944
*/
@@ -947,10 +947,11 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar
947947
$originalValueToWrite = $valueToWrite;
948948

949949
$offsetValueTypeStack = [$offsetValueType];
950+
$generalizeOnWrite = $offsetTypes[array_key_last($offsetTypes)][0] !== null;
950951
foreach (array_slice($offsetTypes, 0, -1) as [$offsetType, $dimFetch]) {
951952
if ($offsetType === null) {
952953
$offsetValueType = new ConstantArrayType([], []);
953-
954+
$generalizeOnWrite = false;
954955
} else {
955956
$has = $offsetValueType->hasOffsetValueType($offsetType);
956957
if ($has->yes()) {
@@ -959,9 +960,11 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar
959960
if (!$scope->hasExpressionType($dimFetch)->yes()) {
960961
$offsetValueType = TypeCombinator::union($offsetValueType->getOffsetValueType($offsetType), new ConstantArrayType([], []));
961962
} else {
963+
$generalizeOnWrite = false;
962964
$offsetValueType = $offsetValueType->getOffsetValueType($offsetType);
963965
}
964966
} else {
967+
$generalizeOnWrite = false;
965968
$offsetValueType = new ConstantArrayType([], []);
966969
}
967970
}
@@ -1010,7 +1013,24 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar
10101013
}
10111014

10121015
} else {
1013-
$valueToWrite = $offsetValueType->setOffsetValueType($offsetType, $valueToWrite, $i === 0);
1016+
// when $unionValues=false the array item-type will be replaced with $valueToWrite
1017+
// when $unionValues=true the existing array item-type will be union'ed with $valueToWrite -> type gets wider
1018+
$unionValues = false;
1019+
if ($i === 0) {
1020+
$unionValues = true;
1021+
} elseif (
1022+
$generalizeOnWrite
1023+
&& $i === count($offsetTypes) - 1
1024+
&&
1025+
(
1026+
$originalValueToWrite->isConstantScalarValue()->yes()
1027+
|| !$offsetValueType->getIterableValueType()->isSuperTypeOf($valueToWrite)->yes()
1028+
)
1029+
) {
1030+
$unionValues = true;
1031+
}
1032+
1033+
$valueToWrite = $offsetValueType->setOffsetValueType($offsetType, $valueToWrite, $unionValues);
10141034
}
10151035

10161036
if ($arrayDimFetch === null || !$offsetValueType->isList()->yes()) {

‎tests/PHPStan/Analyser/nsrt/assign-nested-arrays.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@ public function doFoo(int $i)
1212
$array = [];
1313

1414
$array[$i]['bar'] = 1;
15-
$array[$i]['baz'] = 2;
15+
assertType('non-empty-array<int, array{bar: 1}>', $array);
1616

17+
$array[$i]['baz'] = 2;
1718
assertType('non-empty-array<int, array{bar: 1, baz: 2}>', $array);
1819
}
1920

‎tests/PHPStan/Analyser/nsrt/bug-10089.php‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
namespace Bug10089;
4+
5+
6+
use function PHPStan\Testing\assertType;
7+
8+
class Test
9+
{
10+
11+
protected function create_matrix(int $size): array
12+
{
13+
$size = min(8, $size);
14+
$matrix = [];
15+
for ($i = 0; $i < $size; $i++) {
16+
$matrix[] = array_fill(0, $size, 0);
17+
}
18+
19+
// array<int<0, max>, non-empty-array<int, 0>>
20+
assertType('list<non-empty-list<0>>', $matrix);
21+
22+
$matrix[$size - 1][8] = 3;
23+
24+
// non-empty-array<int, non-empty-array<int, 0|3>&hasOffsetValue(8, 3)>
25+
assertType('non-empty-list<non-empty-array<int<0, max>, 0|3>>', $matrix);
26+
27+
for ($i = 0; $i <= $size; $i++) {
28+
if ($matrix[$i][8] === 0) {
29+
// ...
30+
}
31+
if ($matrix[8][$i] === 0) {
32+
// ...
33+
}
34+
if ($matrix[$size - 1 - $i][8] === 0) {
35+
// ...
36+
}
37+
}
38+
39+
return $matrix;
40+
}
41+
42+
}
43+
44+

‎tests/PHPStan/Analyser/nsrt/bug-13857.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug13857;
6+
7+
use function PHPStan\Testing\assertType;
8+
9+
/**
10+
* @param array<int, array{state: string}> $array
11+
*/
12+
function test(array $array, int $id): void {
13+
$array[$id]['state'] = 'foo';
14+
// only one element was set to 'foo', not all of them.
15+
assertType("non-empty-array<int, array{state: string}>", $array);
16+
}
17+
18+
/**
19+
* @param array<int, array{state?: string}> $array
20+
*/
21+
function testMaybe(array $array, int $id): void {
22+
$array[$id]['state'] = 'foo';
23+
// only one element was set to 'foo', not all of them.
24+
assertType("non-empty-array<int, array{state?: string}>", $array);
25+
}
26+
27+
/**
28+
* @param array<int, array{state: string|bool}> $array
29+
*/
30+
function testUnionValue(array $array, int $id): void {
31+
$array[$id]['state'] = 'foo';
32+
// only one element was set to 'foo', not all of them.
33+
assertType("non-empty-array<int, array{state: bool|string}>", $array);
34+
}
35+
36+
/**
37+
* @param array<int, array{state: string}|array{foo: int}> $array
38+
*/
39+
function testUnionArray(array $array, int $id): void {
40+
$array[$id]['state'] = 'foo';
41+
// only one element was set to 'foo', not all of them.
42+
assertType("non-empty-array<int, non-empty-array{foo?: int, state?: string}>", $array);
43+
}
44+
45+
/**
46+
* @param array<int, array{state: string}|array{foo: int}> $array
47+
*/
48+
function testUnionArrayDifferentType(array $array, int $id): void {
49+
$array[$id]['state'] = true;
50+
assertType("non-empty-array<int, array{state: string}|non-empty-array{foo?: int, state?: true}>", $array);
51+
}
52+
53+
/**
54+
* @param array<int, array{state: 'foo'}> $array
55+
*/
56+
function testConstantArray(array $array, int $id): void {
57+
$array[$id]['state'] = 'bar';
58+
assertType("non-empty-array<int, array{state: 'bar'}|array{state: 'foo'}>", $array);
59+
}
60+
61+
/**
62+
* @param array<int, array{state: 'foo'}> $array
63+
*/
64+
function testConstantArrayNonScalarAssign(array $array, int $id, bool $b): void {
65+
$array[$id]['state'] = $b;
66+
assertType("non-empty-array<int, array{state: 'foo'}|array{state: bool}>", $array);
67+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ 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);
35+
assertType('true', $this->arr[$index]['foo']);
3536
}
3637
assertType('array<int, array{foo?: bool}>', $this->arr);
3738
return $this->arr[$index]['foo']; // PHPStan does not realize 'foo' is set

‎tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,6 +1046,11 @@ public function testBug13282(): void
10461046
$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-13282.php'], []);
10471047
}
10481048

1049+
public function testBug10089(): void
1050+
{
1051+
$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-10089.php'], []);
1052+
}
1053+
10491054
public function testBug11609(): void
10501055
{
10511056
$this->analyse([__DIR__ . '/data/bug-11609.php'], [

0 commit comments

Comments
 (0)