diff --git a/src/Type/Php/ArrayFirstLastDynamicReturnTypeExtension.php b/src/Type/Php/ArrayFirstLastDynamicReturnTypeExtension.php index 859f36f1f02..248fdbb9638 100644 --- a/src/Type/Php/ArrayFirstLastDynamicReturnTypeExtension.php +++ b/src/Type/Php/ArrayFirstLastDynamicReturnTypeExtension.php @@ -9,6 +9,7 @@ use PHPStan\ShouldNotHappenException; use PHPStan\Type\DynamicFunctionReturnTypeExtension; use PHPStan\Type\NullType; +use PHPStan\Type\Traverser\UnsafeArrayStringKeyCastingTraverser; use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; use function count; @@ -46,8 +47,12 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection, switch ($functionReflection->getName()) { case 'array_key_first': case 'array_key_last': - $resultType = $argType->getIterableKeyType(); - break; + $keyType = $argType->getIterableKeyType(); + if ($iterableAtLeastOnce->yes()) { + return UnsafeArrayStringKeyCastingTraverser::castReadKeyType($keyType); + } + + return UnsafeArrayStringKeyCastingTraverser::unionWithReadKeyType($keyType, new NullType()); case 'array_first': case 'array_last': $resultType = $argType->getIterableValueType(); diff --git a/src/Type/Php/ArrayRandFunctionReturnTypeExtension.php b/src/Type/Php/ArrayRandFunctionReturnTypeExtension.php index 01018174977..d1f90907fd1 100644 --- a/src/Type/Php/ArrayRandFunctionReturnTypeExtension.php +++ b/src/Type/Php/ArrayRandFunctionReturnTypeExtension.php @@ -6,21 +6,30 @@ use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Reflection\FunctionReflection; +use PHPStan\Type\Accessory\AccessoryArrayListType; +use PHPStan\Type\Accessory\NonEmptyArrayType; use PHPStan\Type\ArrayType; +use PHPStan\Type\Constant\ConstantArrayTypeBuilder; use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\DynamicFunctionReturnTypeExtension; use PHPStan\Type\IntegerRangeType; use PHPStan\Type\IntegerType; -use PHPStan\Type\StringType; +use PHPStan\Type\Traverser\UnsafeArrayStringKeyCastingTraverser; use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; -use PHPStan\Type\UnionType; use function count; +use function is_int; #[AutowiredService] final class ArrayRandFunctionReturnTypeExtension implements DynamicFunctionReturnTypeExtension { + /** + * Above this many picked keys the shape stops being worth its cost, and + * ConstantArrayTypeBuilder would degrade it to a general array anyway. + */ + private const KEY_COUNT_LIMIT = 100; + public function isFunctionSupported(FunctionReflection $functionReflection): bool { return $functionReflection->getName() === 'array_rand'; @@ -35,34 +44,56 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection, } $firstArgType = $scope->getType($args[0]->value); - $isInteger = $firstArgType->getIterableKeyType()->isInteger(); - $isString = $firstArgType->getIterableKeyType()->isString(); - - if ($isInteger->yes()) { - $valueType = new IntegerType(); - } elseif ($isString->yes()) { - $valueType = new StringType(); - } else { - $valueType = new UnionType([new IntegerType(), new StringType()]); - } + // The picked keys come back as values of their own, so PHP's array key + // cast applies to them. + $keyType = UnsafeArrayStringKeyCastingTraverser::castReadKeyType($firstArgType->getIterableKeyType()); if ($argsCount < 2) { - return $valueType; + return $keyType; } $secondArgType = $scope->getType($args[1]->value); $one = new ConstantIntegerType(1); if ($one->isSuperTypeOf($secondArgType)->yes()) { - return $valueType; + return $keyType; + } + + $pickedKeys = $this->pickedKeysType($keyType, $secondArgType); + if (IntegerRangeType::fromInterval(2, null)->isSuperTypeOf($secondArgType)->yes()) { + return $pickedKeys; } - $bigger2 = IntegerRangeType::fromInterval(2, null); - if ($bigger2->isSuperTypeOf($secondArgType)->yes()) { - return new ArrayType(new IntegerType(), $valueType); + return TypeCombinator::union($keyType, $pickedKeys); + } + + /** + * array_rand() picks $num distinct keys and hands them back in the array's + * own order, so a known $num gives an exact tuple. Returning an array at all + * means $num was at least 2 - one key comes back on its own. + */ + private function pickedKeysType(Type $keyType, Type $numType): Type + { + $constantNums = $numType->getConstantScalarValues(); + if ( + count($constantNums) === 1 + && is_int($constantNums[0]) + && $constantNums[0] >= 2 + && $constantNums[0] <= self::KEY_COUNT_LIMIT + ) { + $builder = ConstantArrayTypeBuilder::createEmpty(); + for ($i = 0; $i < $constantNums[0]; $i++) { + $builder->setOffsetValueType(new ConstantIntegerType($i), $keyType); + } + + return $builder->getArray(); } - return TypeCombinator::union($valueType, new ArrayType(new IntegerType(), $valueType)); + return TypeCombinator::intersect( + new ArrayType(new IntegerType(), $keyType), + new AccessoryArrayListType(), + new NonEmptyArrayType(), + ); } } diff --git a/src/Type/Traverser/UnsafeArrayStringKeyCastingTraverser.php b/src/Type/Traverser/UnsafeArrayStringKeyCastingTraverser.php index 2e5b5b1edb5..f1b97254000 100644 --- a/src/Type/Traverser/UnsafeArrayStringKeyCastingTraverser.php +++ b/src/Type/Traverser/UnsafeArrayStringKeyCastingTraverser.php @@ -4,6 +4,7 @@ use PHPStan\DependencyInjection\ReportUnsafeArrayStringKeyCastingToggle; use PHPStan\Type\Accessory\AccessoryDecimalIntegerStringType; +use PHPStan\Type\BenevolentUnionType; use PHPStan\Type\IntegerType; use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; @@ -12,23 +13,82 @@ use PHPStan\Type\UnionType; /** - * Under `reportUnsafeArrayStringKeyCasting: detect`, PHP casts a decimal-integer - * string array key ("123") to int when iterating, so the iterable key type widens - * from `string` to `int | non-decimal-int-string`. Shared by ArrayType and - * ConstantArrayType so both representations agree — otherwise comparing a general - * array (cast key) against a constant-array shape (raw key) yields a spurious - * `Maybe`. + * PHP casts a decimal-integer string array key ("123") to int, so an array with + * a `string` key type can hand back an int. There are two places that matters, + * and they widen differently on purpose. + * + * {@see self::castKeyType()} widens the key type an array *has*. That type also + * decides how the array describes itself and what it accepts, so only + * `reportUnsafeArrayStringKeyCasting: detect` widens there — it opts into the + * resulting reports. Under `prevent` there is nothing to do, because PHPDoc + * string key types are narrowed to `non-decimal-int-string` when resolved. + * + * {@see self::castReadKeyType()} widens a key *taken out* of an array and handed + * back as a value of its own — `array_key_first()`, `array_keys()`, `key()`, + * `array_flip()`, … With the toggle off it widens `string` to the benevolent + * `(int|string)`, which stops `array_key_first([$string => null])` from looking + * like a certain `string` without making either branch report an error. + * + * `foreach` keys are deliberately not widened with the toggle off: the key + * usually goes straight back into another array (`$result[$k] = …`), and a + * benevolent `(int|string)` key collapses that array to `array`. + * `detect` is the level that gets accurate `foreach` keys. + * + * Both are shared by ArrayType and ConstantArrayType so the two representations + * agree — otherwise comparing a general array (cast key) against a + * constant-array shape (raw key) yields a spurious `Maybe`. */ final class UnsafeArrayStringKeyCastingTraverser implements TypeTraverserCallable { + private function __construct(private bool $precise) + { + } + public static function castKeyType(Type $keyType): Type { if (ReportUnsafeArrayStringKeyCastingToggle::getLevel() !== ReportUnsafeArrayStringKeyCastingToggle::DETECT) { return $keyType; } - return TypeTraverser::map($keyType, new self()); + return TypeTraverser::map($keyType, new self(true)); + } + + public static function castReadKeyType(Type $keyType): Type + { + $level = ReportUnsafeArrayStringKeyCastingToggle::getLevel(); + if ($level !== null) { + // `detect` already widened the key type the array carries, and `prevent` + // made sure it can't hold a decimal-integer string in the first place. + return self::castKeyType($keyType); + } + + // A key type that already covers int has nothing to gain from the widening. + // Leaving it alone also keeps it out of a BenevolentUnionType, so what is + // checked against it stays as strict as it is today. + if ($keyType->isSuperTypeOf(new IntegerType())->yes()) { + return $keyType; + } + + return TypeTraverser::map($keyType, new self(false)); + } + + /** + * Adds the "there is no key" result an accessor returns for an empty array + * (`null` for array_key_first(), `false` for array_search(), …). + * + * TypeCombinator alone would drop the benevolence of a widened key type and + * start reporting on the very code the widening exists to leave alone. + */ + public static function unionWithReadKeyType(Type $keyType, Type $noKeyType): Type + { + $keyType = self::castReadKeyType($keyType); + $union = TypeCombinator::union($keyType, $noKeyType); + if ($keyType instanceof BenevolentUnionType && $union instanceof UnionType && !$union instanceof BenevolentUnionType) { + return new BenevolentUnionType($union->getTypes()); + } + + return $union; } /** @@ -41,6 +101,14 @@ public function traverse(Type $type, callable $traverse): Type } if ($type->isString()->yes() && !$type->isDecimalIntegerString()->no()) { + if (!$this->precise) { + if ($type->isDecimalIntegerString()->yes()) { + return new IntegerType(); + } + + return new BenevolentUnionType([new IntegerType(), $type]); + } + return TypeCombinator::union( new IntegerType(), TypeCombinator::intersect($type, new AccessoryDecimalIntegerStringType(inverse: true)), diff --git a/tests/PHPStan/Analyser/nsrt/array-functions.php b/tests/PHPStan/Analyser/nsrt/array-functions.php index dbbbe0cf76c..f01d8511086 100644 --- a/tests/PHPStan/Analyser/nsrt/array-functions.php +++ b/tests/PHPStan/Analyser/nsrt/array-functions.php @@ -351,19 +351,19 @@ assertType('string|null', key($generalStringKeys)); assertType('int|string|null', key($generalIntegerOrStringKeysMixedValues)); assertType('\'foo\'', $poppedFoo); -assertType('int', array_rand([1 => 1, 2 => "2"])); -assertType('string', array_rand(["a" => 1, "b" => "2"])); -assertType('int|string', array_rand(["a" => 1, 2 => "b"])); +assertType('1|2', array_rand([1 => 1, 2 => "2"])); +assertType('\'a\'|\'b\'', array_rand(["a" => 1, "b" => "2"])); +assertType('2|\'a\'', array_rand(["a" => 1, 2 => "b"])); assertType('int|string', array_rand([1 => 1, 2 => "b", $mixed => $mixed])); -assertType('int', array_rand([1 => 1, 2 => "b"], 1)); -assertType('string', array_rand(["a" => 1, "b" => "b"], 1)); -assertType('int|string', array_rand(["a" => 1, 2 => "b"], 1)); +assertType('1|2', array_rand([1 => 1, 2 => "b"], 1)); +assertType('\'a\'|\'b\'', array_rand(["a" => 1, "b" => "b"], 1)); +assertType('2|\'a\'', array_rand(["a" => 1, 2 => "b"], 1)); assertType('int|string', array_rand([1 => 1, 2 => "b", $mixed => $mixed], 1)); -assertType('array', array_rand([1 => 1, 2 => "b"], 2)); -assertType('array', array_rand(["a" => 1, "b" => "b"], 2)); -assertType('array', array_rand(["a" => 1, 2 => "b"], 2)); -assertType('array', array_rand([1 => 1, 2 => "2", $mixed => $mixed], 2)); -assertType('array|int', array_rand([1 => 1, 2 => "b"], $mixed)); -assertType('array|string', array_rand(["a" => 1, "b" => "b"], $mixed)); -assertType('array|int|string', array_rand(["a" => 1, 2 => "b"], $mixed)); -assertType('array|int|string', array_rand([1 => 1, 2 => "b", $mixed => $mixed], $mixed)); +assertType('array{1|2, 1|2}', array_rand([1 => 1, 2 => "b"], 2)); +assertType('array{\'a\'|\'b\', \'a\'|\'b\'}', array_rand(["a" => 1, "b" => "b"], 2)); +assertType('array{2|\'a\', 2|\'a\'}', array_rand(["a" => 1, 2 => "b"], 2)); +assertType('array{int|string, int|string}', array_rand([1 => 1, 2 => "2", $mixed => $mixed], 2)); +assertType('1|2|non-empty-list<1|2>', array_rand([1 => 1, 2 => "b"], $mixed)); +assertType('\'a\'|\'b\'|non-empty-list<\'a\'|\'b\'>', array_rand(["a" => 1, "b" => "b"], $mixed)); +assertType('2|\'a\'|non-empty-list<2|\'a\'>', array_rand(["a" => 1, 2 => "b"], $mixed)); +assertType('int|non-empty-list|string', array_rand([1 => 1, 2 => "b", $mixed => $mixed], $mixed)); diff --git a/tests/PHPStan/Analyser/nsrt/array-rand.php b/tests/PHPStan/Analyser/nsrt/array-rand.php new file mode 100644 index 00000000000..d5232539b6c --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/array-rand.php @@ -0,0 +1,39 @@ + $list + * @param non-empty-array $strKeyed + * @param int<2, max> $atLeastTwo + * @param positive-int $positive + */ +function f(array $shape, array $list, array $strKeyed, int $atLeastTwo, int $positive, int $int): void +{ + assertType("'a'|'b'|'c'", array_rand($shape)); + assertType("'a'|'b'|'c'", array_rand($shape, 1)); + assertType("array{'a'|'b'|'c', 'a'|'b'|'c'}", array_rand($shape, 2)); + assertType("array{'a'|'b'|'c', 'a'|'b'|'c', 'a'|'b'|'c'}", array_rand($shape, 3)); + + assertType('int<0, max>', array_rand($list)); + assertType('array{int<0, max>, int<0, max>}', array_rand($list, 2)); + + // a decimal-integer string key comes back as an int, see #15073 + assertType('(int|string)', array_rand($strKeyed)); + assertType('array{(int|string), (int|string)}', array_rand($strKeyed, 2)); + + // $num is known to be 2 or more, but not by how much + assertType("non-empty-list<'a'|'b'|'c'>", array_rand($shape, $atLeastTwo)); + + // $num may be 1, which gives back a single key instead of a list + assertType("'a'|'b'|'c'|non-empty-list<'a'|'b'|'c'>", array_rand($shape, $positive)); + assertType("'a'|'b'|'c'|non-empty-list<'a'|'b'|'c'>", array_rand($shape, $int)); + + // past KEY_COUNT_LIMIT the shape gives way to a list + assertType('non-empty-list<(int|string)>', array_rand($strKeyed, 200)); +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-14245.php b/tests/PHPStan/Analyser/nsrt/bug-14245.php index 63333b5de2e..2b93e6d5745 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-14245.php +++ b/tests/PHPStan/Analyser/nsrt/bug-14245.php @@ -104,7 +104,7 @@ function keyDifferentArray(array $arr): void { $list = foo(); assertType('list', $list); $list[array_key_first($arr)] = 37; - assertType('non-empty-array', $list); + assertType('non-empty-array', $list); } function overwriteArraySearch($needle): void { diff --git a/tests/PHPStan/Analyser/nsrt/bug-15073.php b/tests/PHPStan/Analyser/nsrt/bug-15073.php new file mode 100644 index 00000000000..63834eaf266 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-15073.php @@ -0,0 +1,49 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug15073; + +use function PHPStan\Testing\assertType; + +/** + * @param array-key $array_key + * @param numeric-string $numeric_string + * @param decimal-int-string $decimal_string + * @param non-decimal-int-string $non_decimal_string + */ +function readKeys(int|string $int_or_str, int|string $array_key, string $string, string $numeric_string, string $decimal_string, string $non_decimal_string): void +{ + assertType('int|string', array_key_first([$int_or_str => null])); + assertType('(int|string)', array_key_first([$array_key => null])); + assertType('(int|string)', array_key_first([$string => null])); + assertType('int|numeric-string', array_key_first([$numeric_string => null])); + assertType('int', array_key_first([$decimal_string => null])); + assertType('non-decimal-int-string', array_key_first([$non_decimal_string => null])); + + assertType('(int|string)', array_key_last([$string => null])); +} + +/** + * @param non-empty-array $intOrString + * @param non-empty-array|string, int> $partlyInt + */ +function keyTypesThatStopShortOfTheWidening(array $intOrString, array $partlyInt): void +{ + // int|string already covers int, so it is left alone and stays strict + assertType('int|string', array_key_first($intOrString)); + + // int<0, max>|string covers only part of int, so the string half still widens + assertType('(int|string)', array_key_first($partlyInt)); +} + +function isDecimalIntString(mixed $val): bool +{ + if (!is_string($val)) { + return false; + } + + assertType('(int|string)', array_key_first([$val => null])); + + return is_int(array_key_first([$val => null])); +} diff --git a/tests/PHPStan/Analyser/nsrt/php73_functions.php b/tests/PHPStan/Analyser/nsrt/php73_functions.php index 09775c21ef0..d434709226e 100644 --- a/tests/PHPStan/Analyser/nsrt/php73_functions.php +++ b/tests/PHPStan/Analyser/nsrt/php73_functions.php @@ -54,12 +54,12 @@ public function doFoo( assertType('mixed', json_decode($mixed)); assertType('mixed', json_decode($mixed, false, 512, JSON_THROW_ON_ERROR | JSON_NUMERIC_CHECK)); assertType('mixed', json_decode($mixed, false, 512, $integer | JSON_THROW_ON_ERROR | JSON_NUMERIC_CHECK)); - assertType('int|string|null', array_key_first($mixedArray)); - assertType('int|string|null', array_key_last($mixedArray)); + assertType('(int|string|null)', array_key_first($mixedArray)); + assertType('(int|string|null)', array_key_last($mixedArray)); assertType('(int|string)', array_key_first($nonEmptyArray)); assertType('(int|string)', array_key_last($nonEmptyArray)); - assertType('string|null', array_key_first($arrayWithStringKeys)); - assertType('string|null', array_key_last($arrayWithStringKeys)); + assertType('(int|string|null)', array_key_first($arrayWithStringKeys)); + assertType('(int|string|null)', array_key_last($arrayWithStringKeys)); assertType('null', array_key_first($emptyArray)); assertType('null', array_key_last($emptyArray)); assertType('0|1|2', array_key_first($literalArray)); diff --git a/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php index 45911ff6d6a..87fff7a6a79 100644 --- a/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php @@ -157,19 +157,19 @@ public function testBug12981(): void { $this->analyse([__DIR__ . '/data/bug-12981.php'], [ [ - 'Invalid array key type array.', + 'Invalid array key type array.', 31, ], [ - 'Invalid array key type array.', + 'Invalid array key type array.', 33, ], [ - 'Possibly invalid array key type array|int|string.', + 'Possibly invalid array key type int|list<(int|string)>|string.', 39, ], [ - 'Possibly invalid array key type array|int|string.', + 'Possibly invalid array key type int|list<(int|string)>|string.', 41, ], ]); diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index 9bd4e9c6b15..f7014798234 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -977,11 +977,11 @@ public function testBug12981(): void $this->analyse([__DIR__ . '/data/bug-12981.php'], [ [ - 'Offset array|int|string might not exist on non-empty-array.', + 'Offset int|non-empty-list<(int|string)>|string might not exist on non-empty-array.', 39, ], [ - 'Offset array|int|string might not exist on non-empty-array.', + 'Offset int|non-empty-list<(int|string)>|string might not exist on non-empty-array.', 41, ], ]); diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index df3def0c39d..e07e06d8cd6 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -2528,20 +2528,20 @@ public function testArrayRand(): void ], [ 'Parameter #1 $input of function array_rand expects non-empty-array, array{} given.', - 8, + 14, 'array{} is empty.', ], [ 'Parameter #2 $num_req of function array_rand expects int<1, max>, int given.', - 8, + 14, ], [ 'Parameter #2 $num_req of function array_rand expects int<1, max>, -5 given.', - 13, + 19, ], [ 'Parameter #2 $num_req of function array_rand expects int<1, max>, 0 given.', - 14, + 20, ], ]); } diff --git a/tests/PHPStan/Rules/Functions/data/array_rand.php b/tests/PHPStan/Rules/Functions/data/array_rand.php index 92fa5a808ce..0bd52214749 100644 --- a/tests/PHPStan/Rules/Functions/data/array_rand.php +++ b/tests/PHPStan/Rules/Functions/data/array_rand.php @@ -2,9 +2,15 @@ namespace ArrayRand; -function doFoo(int $i) { +function doFoo() { $arr = []; $x = array_rand($arr); +} + +// array_rand() on an empty array never returns, so this needs its own function +// to stay reachable. +function doFooWithNum(int $i) { + $arr = []; $y = array_rand($arr, $i); } diff --git a/tests/PHPStan/Rules/Functions/data/bug-9803.php b/tests/PHPStan/Rules/Functions/data/bug-9803.php index 6e02f6ea991..d7f6a6a8d20 100644 --- a/tests/PHPStan/Rules/Functions/data/bug-9803.php +++ b/tests/PHPStan/Rules/Functions/data/bug-9803.php @@ -13,16 +13,16 @@ function doFoo() { $keys = array(); if ($random == 1) { $keys = array(array_rand($array)); - assertType('array{int}', $keys); + assertType('array{0|1|2|3|4|5|6|7|8|9}', $keys); } else { $keys = array_rand($array, $random); - assertType('array', $keys); + assertType('non-empty-list<0|1|2|3|4|5|6|7|8|9>', $keys); } - assertType('array', $keys); + assertType('non-empty-list<0|1|2|3|4|5|6|7|8|9>', $keys); $theKeys = array_keys($keys); - assertType('list', $theKeys); + assertType('non-empty-list>', $theKeys); }