Skip to content

Commit a5a5b0a

Browse files
phpstan-botondrejmirtes
authored andcommitted
Replace instead of union when writing to optional keys via union offset in ConstantArrayTypeBuilder
- In `ConstantArrayTypeBuilder::setOffsetValueType`, when handling a union offset type (e.g. `'a'|'b'`) where all scalar types match existing keys, the builder always unioned the new value with the old value at each matching key. For optional keys, this is incorrect: the key's presence implies it was the target of the write, so the old value is stale and should be replaced, not unioned. - The fix: when `$optional` is false and the matching key is in `$this->optionalKeys`, replace the value instead of unioning. Required keys still union correctly since their old values represent valid alternative states. - Updated bug-11846.php assertions: the old test was asserting the buggy overapproximate behavior (`array{}|array{array{}}` instead of the correct `array{array{}}`).
1 parent 2f627eb commit a5a5b0a

3 files changed

Lines changed: 107 additions & 4 deletions

File tree

src/Type/Constant/ConstantArrayTypeBuilder.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,11 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt
269269
continue;
270270
}
271271

272-
$valueTypes[$i] = TypeCombinator::union($valueTypes[$i], $valueType);
272+
if (!$optional && in_array($i, $this->optionalKeys, true)) {
273+
$valueTypes[$i] = $valueType;
274+
} else {
275+
$valueTypes[$i] = TypeCombinator::union($valueTypes[$i], $valueType);
276+
}
273277
$offsetMatch = true;
274278
}
275279

tests/PHPStan/Analyser/nsrt/bug-11846.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,16 @@ function demo2(array $idList): void
3838
$outerList[$id] = [];
3939
array_push($outerList[$id], []);
4040
}
41-
assertType('non-empty-array{1?: array{}|array{array{}}, 2?: array{}|array{array{}}}', $outerList);
41+
assertType('non-empty-array{1?: array{array{}}, 2?: array{array{}}}', $outerList);
4242

4343
foreach ($outerList as $key => $outerElement) {
4444
$result = false;
4545

46-
assertType('array{}|array{array{}}', $outerElement);
46+
assertType('array{array{}}', $outerElement);
4747
foreach ($outerElement as $innerElement) {
4848
$result = true;
4949
}
50-
assertType('bool', $result);
50+
assertType('true', $result);
5151

5252
}
5353
}
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug14551;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
/**
8+
* @param non-empty-list<'a'|'b'> $keys
9+
*/
10+
function foreachTwoKeys(array $keys): void
11+
{
12+
$result = [];
13+
foreach ($keys as $k) {
14+
$result[$k]['x'] = 1;
15+
$result[$k]['y'] = 2;
16+
}
17+
18+
assertType("array{a?: array{x: 1, y: 2}, b?: array{x: 1, y: 2}}", $result);
19+
}
20+
21+
/**
22+
* @param non-empty-list<'a'|'b'> $keys
23+
*/
24+
function foreachThreeKeys(array $keys): void
25+
{
26+
$result = [];
27+
foreach ($keys as $k) {
28+
$result[$k]['x'] = 1;
29+
$result[$k]['y'] = 2;
30+
$result[$k]['z'] = 3;
31+
}
32+
33+
assertType("array{a?: array{x: 1, y: 2, z: 3}, b?: array{x: 1, y: 2, z: 3}}", $result);
34+
}
35+
36+
/**
37+
* Test without foreach: union-key nested assignment
38+
* @param 'a'|'b' $k
39+
*/
40+
function withoutForeach(string $k): void
41+
{
42+
$result = [];
43+
$result[$k]['x'] = 1;
44+
$result[$k]['y'] = 2;
45+
46+
assertType("array{a?: array{x: 1, y: 2}, b?: array{x: 1, y: 2}}", $result);
47+
}
48+
49+
/**
50+
* Test with integer union keys
51+
* @param 0|1 $k
52+
*/
53+
function integerKeys(int $k): void
54+
{
55+
$result = [];
56+
$result[$k]['x'] = 1;
57+
$result[$k]['y'] = 2;
58+
59+
assertType("array{0?: array{x: 1, y: 2}, 1?: array{x: 1, y: 2}}", $result);
60+
}
61+
62+
/**
63+
* Test three-way union key
64+
* @param 'a'|'b'|'c' $k
65+
*/
66+
function threeWayUnion(string $k): void
67+
{
68+
$result = [];
69+
$result[$k]['x'] = 1;
70+
$result[$k]['y'] = 2;
71+
72+
assertType("array{a?: array{x: 1, y: 2}, b?: array{x: 1, y: 2}, c?: array{x: 1, y: 2}}", $result);
73+
}
74+
75+
/**
76+
* Required keys should still union (not replace) — existing behavior must be preserved
77+
*/
78+
function requiredKeysStillUnion(): void
79+
{
80+
$result = ['a' => ['x' => 1], 'b' => ['x' => 1]];
81+
/** @var 'a'|'b' $k */
82+
$k = 'a';
83+
$result[$k]['y'] = 2;
84+
85+
assertType("array{a: array{x: 1, y?: 2}, b: array{x: 1, y?: 2}}", $result);
86+
}
87+
88+
/**
89+
* Non-nested union-key overwrites with optional keys
90+
* @param 'a'|'b' $k
91+
*/
92+
function nonNested(string $k): void
93+
{
94+
$result = [];
95+
$result[$k] = 1;
96+
$result[$k] = 2;
97+
98+
assertType("non-empty-array{a?: 2, b?: 2}", $result);
99+
}

0 commit comments

Comments
 (0)