Skip to content

Commit 0f28194

Browse files
staabmstaabm
authored
Don't lose known offset-types in array_merge()
1 parent f46fe27 commit 0f28194

File tree

7 files changed

+247
-12
lines changed

7 files changed

+247
-12
lines changed

‎src/Type/Accessory/HasOffsetType.php‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,7 @@ public function __construct(private ConstantStringType|ConstantIntegerType $offs
5353
{
5454
}
5555

56-
/**
57-
* @return ConstantStringType|ConstantIntegerType
58-
*/
59-
public function getOffsetType(): Type
56+
public function getOffsetType(): ConstantStringType|ConstantIntegerType
6057
{
6158
return $this->offsetType;
6259
}

‎src/Type/Php/ArrayMergeFunctionDynamicReturnTypeExtension.php‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
use PHPStan\Reflection\FunctionReflection;
99
use PHPStan\TrinaryLogic;
1010
use PHPStan\Type\Accessory\AccessoryArrayListType;
11+
use PHPStan\Type\Accessory\HasOffsetType;
12+
use PHPStan\Type\Accessory\HasOffsetValueType;
1113
use PHPStan\Type\Accessory\NonEmptyArrayType;
1214
use PHPStan\Type\ArrayType;
1315
use PHPStan\Type\Constant\ConstantArrayType;
@@ -16,12 +18,15 @@
1618
use PHPStan\Type\Constant\ConstantStringType;
1719
use PHPStan\Type\DynamicFunctionReturnTypeExtension;
1820
use PHPStan\Type\IntegerType;
21+
use PHPStan\Type\MixedType;
1922
use PHPStan\Type\NeverType;
2023
use PHPStan\Type\Type;
2124
use PHPStan\Type\TypeCombinator;
25+
use PHPStan\Type\TypeUtils;
2226
use function array_keys;
2327
use function count;
2428
use function in_array;
29+
use function is_int;
2530

2631
#[AutowiredService]
2732
final class ArrayMergeFunctionDynamicReturnTypeExtension implements DynamicFunctionReturnTypeExtension
@@ -96,6 +101,44 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection,
96101
return $newArrayBuilder->getArray();
97102
}
98103

104+
$offsetTypes = [];
105+
foreach ($argTypes as $argType) {
106+
$constArrays = $argType->getConstantArrays();
107+
if ($constArrays !== []) {
108+
foreach ($constArrays as $constantArray) {
109+
foreach ($constantArray->getKeyTypes() as $keyType) {
110+
$hasOffsetValue = TrinaryLogic::createFromBoolean($argType->hasOffsetValueType($keyType)->yes());
111+
$offsetTypes[$keyType->getValue()] = [
112+
$hasOffsetValue,
113+
$argType->getOffsetValueType($keyType),
114+
];
115+
}
116+
}
117+
} else {
118+
foreach ($offsetTypes as $key => [$hasOffsetValue, $offsetValueType]) {
119+
$offsetTypes[$key] = [
120+
$hasOffsetValue->and(TrinaryLogic::createMaybe()),
121+
new MixedType(),
122+
];
123+
}
124+
}
125+
126+
foreach (TypeUtils::getAccessoryTypes($argType) as $accessoryType) {
127+
if (
128+
!($accessoryType instanceof HasOffsetType)
129+
&& !($accessoryType instanceof HasOffsetValueType)
130+
) {
131+
continue;
132+
}
133+
134+
$offsetType = $accessoryType->getOffsetType();
135+
$offsetTypes[$offsetType->getValue()] = [
136+
TrinaryLogic::createYes(),
137+
$argType->getOffsetValueType($offsetType),
138+
];
139+
}
140+
}
141+
99142
$keyTypes = [];
100143
$valueTypes = [];
101144
$nonEmpty = false;
@@ -132,6 +175,36 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection,
132175
if ($isList) {
133176
$arrayType = TypeCombinator::intersect($arrayType, new AccessoryArrayListType());
134177
}
178+
if ($offsetTypes !== []) {
179+
$knownOffsetValues = [];
180+
foreach ($offsetTypes as $key => [$hasOffsetValue, $offsetType]) {
181+
if (is_int($key)) {
182+
// int keys will be appended and renumbered.
183+
// at this point we can't reason about them, because unknown arrays are in the mix.
184+
continue;
185+
}
186+
$keyType = new ConstantStringType($key);
187+
188+
if ($hasOffsetValue->yes()) {
189+
// the last string-keyed offset will overwrite previous values
190+
$hasOffsetType = new HasOffsetValueType(
191+
$keyType,
192+
$offsetType,
193+
);
194+
} elseif ($hasOffsetValue->maybe()) {
195+
$hasOffsetType = new HasOffsetType(
196+
$keyType,
197+
);
198+
} else {
199+
continue;
200+
}
201+
202+
$knownOffsetValues[] = $hasOffsetType;
203+
}
204+
if ($knownOffsetValues !== []) {
205+
$arrayType = TypeCombinator::intersect($arrayType, ...$knownOffsetValues);
206+
}
207+
}
135208

136209
return $arrayType;
137210
}

‎tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4697,11 +4697,11 @@ public static function dataArrayFunctions(): array
46974697
'array_merge($generalStringKeys, $generalDateTimeValues)',
46984698
],
46994699
[
4700-
'non-empty-array<1|string, int|stdClass>',
4700+
"non-empty-array<1|string, int|stdClass>&hasOffsetValue('foo', stdClass)",
47014701
'array_merge($generalStringKeys, $stringOrIntegerKeys)',
47024702
],
47034703
[
4704-
'non-empty-array<1|string, int|stdClass>',
4704+
"non-empty-array<1|string, int|stdClass>&hasOffset('foo')",
47054705
'array_merge($stringOrIntegerKeys, $generalStringKeys)',
47064706
],
47074707
[

‎tests/PHPStan/Analyser/nsrt/array-merge-const-non-const.php‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
<?php
2+
3+
namespace ArrayMergeConstNonConst;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
function doFoo(array $post): void {
8+
assertType(
9+
"non-empty-array&hasOffset('a')&hasOffset('b')",
10+
array_merge(['a' => 1, 'b' => false, 10 => 99], $post)
11+
);
12+
}
13+
14+
function doBar(array $array): void {
15+
assertType(
16+
"non-empty-array&hasOffsetValue('a', 1)&hasOffsetValue('b', false)",
17+
array_merge($array, ['a' => 1, 'b' => false, 10 => 99])
18+
);
19+
}
20+
21+
function doFooBar(array $array): void {
22+
assertType(
23+
"non-empty-array&hasOffset('x')&hasOffsetValue('a', 1)&hasOffsetValue('b', false)&hasOffsetValue('c', 'e')",
24+
array_merge(['c' => 'd', 'x' => 'y'], $array, ['a' => 1, 'b' => false, 'c' => 'e'])
25+
);
26+
}
27+
28+
function doFooInts(array $array): void {
29+
// int keys will be renumbered therefore we can't reason about them in case we don't know all arrays involved
30+
assertType(
31+
"non-empty-array&hasOffsetValue('a', 1)&hasOffsetValue('c', 'e')",
32+
array_merge([1 => 'd'], $array, ['a' => 1, 3 => false, 'c' => 'e'])
33+
);
34+
}
35+
36+
/**
37+
* @param array<string> $array
38+
*/
39+
function floatKey(array $array): void {
40+
assertType(
41+
"non-empty-array<string>&hasOffsetValue('a', '1')&hasOffsetValue('c', 'e')",
42+
array_merge([4.23 => 'd'], $array, ['a' => '1', 3 => 'false', 'c' => 'e'])
43+
);
44+
}
45+
46+
function doOptKeys(array $array, array $arr2): void {
47+
if (rand(0, 1)) {
48+
$array['abc'] = 'def';
49+
}
50+
assertType("array", array_merge($arr2, $array));
51+
assertType("array", array_merge($array, $arr2));
52+
}
53+
54+
/**
55+
* @param array{a?: 1, b: 2} $array
56+
*/
57+
function doOptShapeKeys(array $array, array $arr2): void {
58+
assertType("non-empty-array&hasOffsetValue('b', 2)", array_merge($arr2, $array));
59+
assertType("non-empty-array&hasOffset('b')", array_merge($array, $arr2));
60+
}
61+
62+
function hasOffsetKeys(array $array, array $arr2): void {
63+
if (array_key_exists('b', $array)) {
64+
assertType("non-empty-array&hasOffsetValue('b', mixed)", array_merge($arr2, $array));
65+
assertType("non-empty-array&hasOffset('b')", array_merge($array, $arr2));
66+
}
67+
}
68+
69+
function maybeHasOffsetKeys(array $array): void {
70+
$arr2 = [];
71+
if (rand(0,1)) {
72+
$arr2 ['ab'] = 'def';
73+
}
74+
75+
assertType("array", array_merge($arr2, $array));
76+
assertType("array", array_merge($array, $arr2));
77+
}
78+
79+
function hasOffsetValueKeys(array $hasB, array $mixedArray, array $hasC): void {
80+
$hasB['b'] = 123;
81+
$hasC['c'] = 'def';
82+
83+
assertType("non-empty-array&hasOffsetValue('b', 123)", array_merge($mixedArray, $hasB));
84+
assertType("non-empty-array&hasOffset('b')", array_merge($hasB, $mixedArray));
85+
86+
assertType(
87+
"non-empty-array&hasOffset('b')&hasOffsetValue('c', 'def')",
88+
array_merge($mixedArray, $hasB, $hasC)
89+
);
90+
assertType(
91+
"non-empty-array&hasOffset('b')&hasOffsetValue('c', 'def')",
92+
array_merge($hasB, $mixedArray, $hasC)
93+
);
94+
95+
assertType(
96+
"non-empty-array&hasOffset('c')&hasOffsetValue('b', 123)",
97+
array_merge($hasC, $mixedArray, $hasB)
98+
);
99+
assertType(
100+
"non-empty-array&hasOffset('b')&hasOffset('c')",
101+
array_merge($hasC, $hasB, $mixedArray)
102+
);
103+
104+
if (rand(0, 1)) {
105+
$hasBorC = ['b' => 1];
106+
} else {
107+
$hasBorC = ['c' => 2];
108+
}
109+
assertType('array{b: 1}|array{c: 2}', $hasBorC);
110+
assertType("non-empty-array", array_merge($mixedArray, $hasBorC));
111+
assertType("non-empty-array", array_merge($hasBorC, $mixedArray));
112+
113+
if (rand(0, 1)) {
114+
$differentCs = ['c' => 10];
115+
} else {
116+
$differentCs = ['c' => 20];
117+
}
118+
assertType('array{c: 10}|array{c: 20}', $differentCs);
119+
assertType("non-empty-array&hasOffsetValue('c', 10|20)", array_merge($mixedArray, $differentCs));
120+
assertType("non-empty-array&hasOffset('c')", array_merge($differentCs, $mixedArray));
121+
122+
assertType("non-empty-array&hasOffsetValue('c', 10|20)", array_merge($mixedArray, $hasBorC, $differentCs));
123+
assertType("non-empty-array", array_merge($differentCs, $mixedArray, $hasBorC)); // could be non-empty-array&hasOffset('c')
124+
assertType("non-empty-array&hasOffsetValue('c', 10|20)", array_merge($hasBorC, $mixedArray, $differentCs));
125+
assertType("non-empty-array", array_merge($differentCs, $hasBorC, $mixedArray)); // could be non-empty-array&hasOffset('c')
126+
}
127+
128+
/**
129+
* @param array{a?: 1, b?: 2} $allOptional
130+
*/
131+
function doAllOptional(array $allOptional, array $arr2): void {
132+
assertType("array", array_merge($arr2, $allOptional));
133+
assertType("array", array_merge($allOptional, $arr2));
134+
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,23 +49,23 @@ public function __construct(MutatorConfig $config)
4949
private function getResultSettings(array $settings): array
5050
{
5151
$settings = array_merge(self::DEFAULT_SETTINGS, $settings);
52-
assertType('non-empty-array<string, mixed>', $settings);
52+
assertType("non-empty-array<string, mixed>&hasOffset('limit')&hasOffset('remove')", $settings);
5353

5454
if (!is_string($settings['remove'])) {
5555
throw $this->configException($settings, 'remove');
5656
}
5757

58-
assertType("non-empty-array<string, mixed>&hasOffsetValue('remove', string)", $settings);
58+
assertType("non-empty-array<string, mixed>&hasOffset('limit')&hasOffsetValue('remove', string)", $settings);
5959

6060
$settings['remove'] = strtolower($settings['remove']);
6161

62-
assertType("non-empty-array<string, mixed>&hasOffsetValue('remove', lowercase-string)", $settings);
62+
assertType("non-empty-array<string, mixed>&hasOffset('limit')&hasOffsetValue('remove', lowercase-string)", $settings);
6363

6464
if (!in_array($settings['remove'], ['first', 'last', 'all'], true)) {
6565
throw $this->configException($settings, 'remove');
6666
}
6767

68-
assertType("non-empty-array<string, mixed>&hasOffsetValue('remove', 'all'|'first'|'last')", $settings);
68+
assertType("non-empty-array<string, mixed>&hasOffset('limit')&hasOffsetValue('remove', 'all'|'first'|'last')", $settings);
6969

7070
if (!is_numeric($settings['limit']) || $settings['limit'] < 1) {
7171
throw $this->configException($settings, 'limit');
@@ -110,13 +110,13 @@ private function getResultSettings(array $settings): array
110110
{
111111
$settings = array_merge(self::DEFAULT_SETTINGS, $settings);
112112

113-
assertType('non-empty-array<string, mixed>', $settings);
113+
assertType("non-empty-array<string, mixed>&hasOffset('limit')&hasOffset('remove')", $settings);
114114

115115
if (!is_string($settings['remove'])) {
116116
throw new Exception();
117117
}
118118

119-
assertType("non-empty-array<string, mixed>&hasOffsetValue('remove', string)", $settings);
119+
assertType("non-empty-array<string, mixed>&hasOffset('limit')&hasOffsetValue('remove', string)", $settings);
120120

121121
if (!is_int($settings['limit'])) {
122122
throw new Exception();

‎tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,4 +1281,9 @@ public function testBug11160(): void
12811281
$this->analyse([__DIR__ . '/data/bug-11160.php'], []);
12821282
}
12831283

1284+
public function testBug8438(): void
1285+
{
1286+
$this->analyse([__DIR__ . '/data/bug-8438.php'], []);
1287+
}
1288+
12841289
}

‎tests/PHPStan/Rules/Methods/data/bug-8438.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
3+
namespace Bug8438;
4+
5+
class HelloWorld
6+
{
7+
/**
8+
* @param array<string, string> $array
9+
*
10+
* @return array{expr: mixed, ...}
11+
*/
12+
protected function foo(array $array): array
13+
{
14+
$rnd = mt_rand();
15+
if ($rnd === 0) {
16+
return ['expr' => 'test'];
17+
} elseif ($rnd === 1) {
18+
// no error with checkBenevolentUnionTypes: false (default even with l9 + strict rules)
19+
return ['expr' => 'test', 1 => 'ok'];
20+
} else {
21+
// phpstan must understand 'expr' key is always present in the result,
22+
// then there will be no error here neither
23+
return array_merge($array, ['expr' => 'test', 1 => 'ok']);
24+
}
25+
}
26+
}

0 commit comments

Comments
 (0)