Skip to content

Commit 7601b6a

Browse files
zonuexeclaude
andcommitted
Return the picked keys from array_rand()
array_rand() collapsed the array's key type into int, string or int|string, and returned `array<int, key>` whenever $num was more than one. Hand back the key type itself, and build a tuple when $num is a known constant: array_rand(['a' => 1, 'b' => 2]) 'a'|'b' (was string) array_rand($list) int<0, max> (was int) array_rand($shape, 2) array{key, key} (was array<int, key>) array_rand($shape, $atLeastTwo) non-empty-list<key> An array comes back only when $num is at least 2, since a single pick returns the key on its own, so the list is non-empty in every branch that produces one. KEY_COUNT_LIMIT caps the tuple at 100 elements, past which ConstantArrayTypeBuilder would degrade the shape anyway. The keys run through castReadKeyType(), so `array<string, X>` gives `(int|string)` rather than a certain `string`. array_rand([]) returns never now, which is what PHP 8 does (ValueError). That also makes everything after the call unreachable, so the two calls in data/array_rand.php moved into separate functions to keep their diagnostics. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent dcc3c79 commit 7601b6a

8 files changed

Lines changed: 123 additions & 47 deletions

src/Type/Php/ArrayRandFunctionReturnTypeExtension.php

Lines changed: 49 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,30 @@
66
use PHPStan\Analyser\Scope;
77
use PHPStan\DependencyInjection\AutowiredService;
88
use PHPStan\Reflection\FunctionReflection;
9+
use PHPStan\Type\Accessory\AccessoryArrayListType;
10+
use PHPStan\Type\Accessory\NonEmptyArrayType;
911
use PHPStan\Type\ArrayType;
12+
use PHPStan\Type\Constant\ConstantArrayTypeBuilder;
1013
use PHPStan\Type\Constant\ConstantIntegerType;
1114
use PHPStan\Type\DynamicFunctionReturnTypeExtension;
1215
use PHPStan\Type\IntegerRangeType;
1316
use PHPStan\Type\IntegerType;
14-
use PHPStan\Type\StringType;
17+
use PHPStan\Type\Traverser\UnsafeArrayStringKeyCastingTraverser;
1518
use PHPStan\Type\Type;
1619
use PHPStan\Type\TypeCombinator;
17-
use PHPStan\Type\UnionType;
1820
use function count;
21+
use function is_int;
1922

2023
#[AutowiredService]
2124
final class ArrayRandFunctionReturnTypeExtension implements DynamicFunctionReturnTypeExtension
2225
{
2326

27+
/**
28+
* Above this many picked keys the shape stops being worth its cost, and
29+
* ConstantArrayTypeBuilder would degrade it to a general array anyway.
30+
*/
31+
private const KEY_COUNT_LIMIT = 100;
32+
2433
public function isFunctionSupported(FunctionReflection $functionReflection): bool
2534
{
2635
return $functionReflection->getName() === 'array_rand';
@@ -35,34 +44,56 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection,
3544
}
3645

3746
$firstArgType = $scope->getType($args[0]->value);
38-
$isInteger = $firstArgType->getIterableKeyType()->isInteger();
39-
$isString = $firstArgType->getIterableKeyType()->isString();
40-
41-
if ($isInteger->yes()) {
42-
$valueType = new IntegerType();
43-
} elseif ($isString->yes()) {
44-
$valueType = new StringType();
45-
} else {
46-
$valueType = new UnionType([new IntegerType(), new StringType()]);
47-
}
47+
// The picked keys come back as values of their own, so PHP's array key
48+
// cast applies to them.
49+
$keyType = UnsafeArrayStringKeyCastingTraverser::castReadKeyType($firstArgType->getIterableKeyType());
4850

4951
if ($argsCount < 2) {
50-
return $valueType;
52+
return $keyType;
5153
}
5254

5355
$secondArgType = $scope->getType($args[1]->value);
5456

5557
$one = new ConstantIntegerType(1);
5658
if ($one->isSuperTypeOf($secondArgType)->yes()) {
57-
return $valueType;
59+
return $keyType;
60+
}
61+
62+
$pickedKeys = $this->pickedKeysType($keyType, $secondArgType);
63+
if (IntegerRangeType::fromInterval(2, null)->isSuperTypeOf($secondArgType)->yes()) {
64+
return $pickedKeys;
5865
}
5966

60-
$bigger2 = IntegerRangeType::fromInterval(2, null);
61-
if ($bigger2->isSuperTypeOf($secondArgType)->yes()) {
62-
return new ArrayType(new IntegerType(), $valueType);
67+
return TypeCombinator::union($keyType, $pickedKeys);
68+
}
69+
70+
/**
71+
* array_rand() picks $num distinct keys and hands them back in the array's
72+
* own order, so a known $num gives an exact tuple. Returning an array at all
73+
* means $num was at least 2 - one key comes back on its own.
74+
*/
75+
private function pickedKeysType(Type $keyType, Type $numType): Type
76+
{
77+
$constantNums = $numType->getConstantScalarValues();
78+
if (
79+
count($constantNums) === 1
80+
&& is_int($constantNums[0])
81+
&& $constantNums[0] >= 2
82+
&& $constantNums[0] <= self::KEY_COUNT_LIMIT
83+
) {
84+
$builder = ConstantArrayTypeBuilder::createEmpty();
85+
for ($i = 0; $i < $constantNums[0]; $i++) {
86+
$builder->setOffsetValueType(new ConstantIntegerType($i), $keyType);
87+
}
88+
89+
return $builder->getArray();
6390
}
6491

65-
return TypeCombinator::union($valueType, new ArrayType(new IntegerType(), $valueType));
92+
return TypeCombinator::intersect(
93+
new ArrayType(new IntegerType(), $keyType),
94+
new AccessoryArrayListType(),
95+
new NonEmptyArrayType(),
96+
);
6697
}
6798

6899
}

tests/PHPStan/Analyser/nsrt/array-functions.php

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -351,19 +351,19 @@
351351
assertType('string|null', key($generalStringKeys));
352352
assertType('int|string|null', key($generalIntegerOrStringKeysMixedValues));
353353
assertType('\'foo\'', $poppedFoo);
354-
assertType('int', array_rand([1 => 1, 2 => "2"]));
355-
assertType('string', array_rand(["a" => 1, "b" => "2"]));
356-
assertType('int|string', array_rand(["a" => 1, 2 => "b"]));
354+
assertType('1|2', array_rand([1 => 1, 2 => "2"]));
355+
assertType('\'a\'|\'b\'', array_rand(["a" => 1, "b" => "2"]));
356+
assertType('2|\'a\'', array_rand(["a" => 1, 2 => "b"]));
357357
assertType('int|string', array_rand([1 => 1, 2 => "b", $mixed => $mixed]));
358-
assertType('int', array_rand([1 => 1, 2 => "b"], 1));
359-
assertType('string', array_rand(["a" => 1, "b" => "b"], 1));
360-
assertType('int|string', array_rand(["a" => 1, 2 => "b"], 1));
358+
assertType('1|2', array_rand([1 => 1, 2 => "b"], 1));
359+
assertType('\'a\'|\'b\'', array_rand(["a" => 1, "b" => "b"], 1));
360+
assertType('2|\'a\'', array_rand(["a" => 1, 2 => "b"], 1));
361361
assertType('int|string', array_rand([1 => 1, 2 => "b", $mixed => $mixed], 1));
362-
assertType('array<int, int>', array_rand([1 => 1, 2 => "b"], 2));
363-
assertType('array<int, string>', array_rand(["a" => 1, "b" => "b"], 2));
364-
assertType('array<int, int|string>', array_rand(["a" => 1, 2 => "b"], 2));
365-
assertType('array<int, int|string>', array_rand([1 => 1, 2 => "2", $mixed => $mixed], 2));
366-
assertType('array<int, int>|int', array_rand([1 => 1, 2 => "b"], $mixed));
367-
assertType('array<int, string>|string', array_rand(["a" => 1, "b" => "b"], $mixed));
368-
assertType('array<int, int|string>|int|string', array_rand(["a" => 1, 2 => "b"], $mixed));
369-
assertType('array<int, int|string>|int|string', array_rand([1 => 1, 2 => "b", $mixed => $mixed], $mixed));
362+
assertType('array{1|2, 1|2}', array_rand([1 => 1, 2 => "b"], 2));
363+
assertType('array{\'a\'|\'b\', \'a\'|\'b\'}', array_rand(["a" => 1, "b" => "b"], 2));
364+
assertType('array{2|\'a\', 2|\'a\'}', array_rand(["a" => 1, 2 => "b"], 2));
365+
assertType('array{int|string, int|string}', array_rand([1 => 1, 2 => "2", $mixed => $mixed], 2));
366+
assertType('1|2|non-empty-list<1|2>', array_rand([1 => 1, 2 => "b"], $mixed));
367+
assertType('\'a\'|\'b\'|non-empty-list<\'a\'|\'b\'>', array_rand(["a" => 1, "b" => "b"], $mixed));
368+
assertType('2|\'a\'|non-empty-list<2|\'a\'>', array_rand(["a" => 1, 2 => "b"], $mixed));
369+
assertType('int|non-empty-list<int|string>|string', array_rand([1 => 1, 2 => "b", $mixed => $mixed], $mixed));
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
declare(strict_types = 1);
4+
5+
namespace ArrayRandReturnType;
6+
7+
use function PHPStan\Testing\assertType;
8+
9+
/**
10+
* @param array{a: 1, b: 2, c: 3} $shape
11+
* @param non-empty-list<string> $list
12+
* @param non-empty-array<string, int> $strKeyed
13+
* @param int<2, max> $atLeastTwo
14+
* @param positive-int $positive
15+
*/
16+
function f(array $shape, array $list, array $strKeyed, int $atLeastTwo, int $positive, int $int): void
17+
{
18+
assertType("'a'|'b'|'c'", array_rand($shape));
19+
assertType("'a'|'b'|'c'", array_rand($shape, 1));
20+
assertType("array{'a'|'b'|'c', 'a'|'b'|'c'}", array_rand($shape, 2));
21+
assertType("array{'a'|'b'|'c', 'a'|'b'|'c', 'a'|'b'|'c'}", array_rand($shape, 3));
22+
23+
assertType('int<0, max>', array_rand($list));
24+
assertType('array{int<0, max>, int<0, max>}', array_rand($list, 2));
25+
26+
// a decimal-integer string key comes back as an int, see #15073
27+
assertType('(int|string)', array_rand($strKeyed));
28+
assertType('array{(int|string), (int|string)}', array_rand($strKeyed, 2));
29+
30+
// $num is known to be 2 or more, but not by how much
31+
assertType("non-empty-list<'a'|'b'|'c'>", array_rand($shape, $atLeastTwo));
32+
33+
// $num may be 1, which gives back a single key instead of a list
34+
assertType("'a'|'b'|'c'|non-empty-list<'a'|'b'|'c'>", array_rand($shape, $positive));
35+
assertType("'a'|'b'|'c'|non-empty-list<'a'|'b'|'c'>", array_rand($shape, $int));
36+
37+
// past KEY_COUNT_LIMIT the shape gives way to a list
38+
assertType('non-empty-list<(int|string)>', array_rand($strKeyed, 200));
39+
}

tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,19 +157,19 @@ public function testBug12981(): void
157157
{
158158
$this->analyse([__DIR__ . '/data/bug-12981.php'], [
159159
[
160-
'Invalid array key type array<int, int|string>.',
160+
'Invalid array key type array<int, (int|string)>.',
161161
31,
162162
],
163163
[
164-
'Invalid array key type array<int, int|string>.',
164+
'Invalid array key type array<int, (int|string)>.',
165165
33,
166166
],
167167
[
168-
'Possibly invalid array key type array<int, int|string>|int|string.',
168+
'Possibly invalid array key type int|list<(int|string)>|string.',
169169
39,
170170
],
171171
[
172-
'Possibly invalid array key type array<int, int|string>|int|string.',
172+
'Possibly invalid array key type int|list<(int|string)>|string.',
173173
41,
174174
],
175175
]);

tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -977,11 +977,11 @@ public function testBug12981(): void
977977

978978
$this->analyse([__DIR__ . '/data/bug-12981.php'], [
979979
[
980-
'Offset array<int, int|string>|int|string might not exist on non-empty-array<bool|float|int|string>.',
980+
'Offset int|non-empty-list<(int|string)>|string might not exist on non-empty-array<bool|float|int|string>.',
981981
39,
982982
],
983983
[
984-
'Offset array<int, int|string>|int|string might not exist on non-empty-array<bool|float|int|string>.',
984+
'Offset int|non-empty-list<(int|string)>|string might not exist on non-empty-array<bool|float|int|string>.',
985985
41,
986986
],
987987
]);

tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2528,20 +2528,20 @@ public function testArrayRand(): void
25282528
],
25292529
[
25302530
'Parameter #1 $input of function array_rand expects non-empty-array, array{} given.',
2531-
8,
2531+
14,
25322532
'array{} is empty.',
25332533
],
25342534
[
25352535
'Parameter #2 $num_req of function array_rand expects int<1, max>, int given.',
2536-
8,
2536+
14,
25372537
],
25382538
[
25392539
'Parameter #2 $num_req of function array_rand expects int<1, max>, -5 given.',
2540-
13,
2540+
19,
25412541
],
25422542
[
25432543
'Parameter #2 $num_req of function array_rand expects int<1, max>, 0 given.',
2544-
14,
2544+
20,
25452545
],
25462546
]);
25472547
}

tests/PHPStan/Rules/Functions/data/array_rand.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,15 @@
22

33
namespace ArrayRand;
44

5-
function doFoo(int $i) {
5+
function doFoo() {
66
$arr = [];
77
$x = array_rand($arr);
8+
}
9+
10+
// array_rand() on an empty array never returns, so this needs its own function
11+
// to stay reachable.
12+
function doFooWithNum(int $i) {
13+
$arr = [];
814
$y = array_rand($arr, $i);
915
}
1016

tests/PHPStan/Rules/Functions/data/bug-9803.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,16 @@ function doFoo() {
1313
$keys = array();
1414
if ($random == 1) {
1515
$keys = array(array_rand($array));
16-
assertType('array{int}', $keys);
16+
assertType('array{0|1|2|3|4|5|6|7|8|9}', $keys);
1717
}
1818
else {
1919
$keys = array_rand($array, $random);
20-
assertType('array<int, int>', $keys);
20+
assertType('non-empty-list<0|1|2|3|4|5|6|7|8|9>', $keys);
2121
}
2222

23-
assertType('array<int, int>', $keys);
23+
assertType('non-empty-list<0|1|2|3|4|5|6|7|8|9>', $keys);
2424
$theKeys = array_keys($keys);
25-
assertType('list<int>', $theKeys);
25+
assertType('non-empty-list<int<0, max>>', $theKeys);
2626
}
2727

2828

0 commit comments

Comments
 (0)