Describe Random\Randomizer array, bytes and integer methods with dedicated dynamic return type extensions

[phpstan-bot]

Summary

Random\Randomizer::shuffleArray() was described as plain array and pickArrayKeys() as non-empty-array<int|string>, so generic wrappers around them could not return list<T> / non-empty-list<TKey>. Both methods actually return a list — shuffleArray() the values of the given array, pickArrayKeys() some of its keys — and both can keep the value/key type of the argument.

The fix describes those two methods precisely, and sweeps the rest of the Random family (and the global functions that mirror it) for the same kind of imprecision.

Changes

New services in src/Type/Php/:

• RandomizerMethodReturnTypeExtension — Random\Randomizer::shuffleArray(), pickArrayKeys(), shuffleBytes(), getBytesFromString() and getInt().
• RandomArrayKeysReturnTypeHelper — the type of keys picked out of an array at random; shared by pickArrayKeys() and array_rand().
• StringBytesReturnTypeHelper — the type of a string built out of the bytes of another string, either reordered (strrev(), str_shuffle(), shuffleBytes()) or selected (getBytesFromString()).
• StrShuffleFunctionReturnTypeExtension — str_shuffle().
• RandomIntRangeHelper — the integer range of a bounded random number generator, extracted from RandomIntFunctionReturnTypeExtension and reused by Randomizer::getInt().

Changed:

• src/Type/Php/ArrayRandFunctionReturnTypeExtension.php — returns the array's own key type (so constant arrays give 'a'|'b' instead of string) and a non-empty-list instead of array<int, ...> when more than one key is picked.
• src/Type/Php/StrrevFunctionReturnTypeExtension.php — accessory handling moved into StringBytesReturnTypeHelper.
• src/Type/Php/RandomIntFunctionReturnTypeExtension.php — range computation moved into RandomIntRangeHelper.
• resources/functionMap_php82delta.php — Randomizer::getBytes() → non-empty-string with a positive-int length, Randomizer::nextInt() → int<0, max>, Random\Engine\Mt19937|PcgOneseq128XslRr64|Secure|Xoshiro256StarStar::generate() → non-empty-string, and pickArrayKeys()'s fallback return updated to non-empty-list<int|string>.
• resources/functionMap_php83delta.php — Randomizer::getBytesFromString() → non-empty-string, with a non-empty-string subject and a positive-int length (both are ValueErrors otherwise).
• stubs/core.stub — dropped str_shuffle()'s conditional @return, now subsumed by the extension.

Analogous cases probed and left alone:

• shuffle(), random_int(), rand(), mt_rand(), random_bytes() and strrev() were already precise.
• Randomizer::getFloat() / nextFloat() cannot be improved — PHPStan has no float ranges.
• Random\Engine::generate() was deliberately not narrowed on the interface: the four built-in implementations are final, but narrowing the interface would make every user-land engine declaring string report a covariance error.
• openssl_random_pseudo_bytes() has the same "random bytes of a positive length" shape but its signature still carries a PHP 7-era string|false; changing that is a separate decision.

Root cause

Two patterns, each appearing in more than one place:

1. Functions that pick keys out of an array were described with a widened key type and a non-list array. Random\Randomizer::pickArrayKeys() only had a signature-map entry (non-empty-array<int|string>), and array_rand()'s extension collapsed the key type to int, string or int|string and built a plain array<int, ...>. Both actually return the keys re-indexed from zero, so they are non-empty lists of the array's own key type. Centralised in RandomArrayKeysReturnTypeHelper.
2. Accessory string types were not preserved through byte-level string operations. strrev() preserved non-empty/non-falsy/lowercase/uppercase, but str_shuffle() only had a conditional @return for non-emptiness and Randomizer::shuffleBytes() / getBytesFromString() had nothing at all, even though all of them build their result from the bytes of the input. Centralised in StringBytesReturnTypeHelper.

shuffleArray() is fixed by routing it through the existing Type::shuffleArray(), the same primitive shuffle() already uses, which is what makes the result a list and keeps non-emptiness and the value type (including template types, which is what the reported snippet needed).

Test

• tests/PHPStan/Analyser/nsrt/bug-15256.php — the reporter's playground snippet (@template T/@param T[] around shuffleArray(), @template TKey of array-key around pickArrayKeys()) plus coverage for non-empty arrays, lists, mixed-key arrays, constant arrays, all the accessory string flavours through shuffleBytes(), getBytes(), getInt(), nextInt() and the four engines' generate().
• tests/PHPStan/Analyser/nsrt/bug-15256-83.php — getBytesFromString() (PHP 8.3+).
• tests/PHPStan/Analyser/nsrt/str-shuffle.php — extended to the same matrix as strrev.php; the previously asserted non-empty-string results are now non-falsy-string / lowercase-string / uppercase-string.
• tests/PHPStan/Analyser/nsrt/array-functions.php, tests/PHPStan/Rules/Functions/data/bug-9803.php and the bug-12981 expectations in InvalidKeyInArrayDimFetchRuleTest / NonexistentOffsetInArrayDimFetchRuleTest — updated to the more precise array_rand() types.

All three new/extended test files were verified to fail before the fix.

Fixes phpstan/phpstan#15256

[VincentLanglet]

@staabm I have mixed feeling about this PR and I would like your opinion.
I feel like this introduce too many helper so I try to find solutions...

1. We could ask PHPStan to rewrite Randomizer::getBytesFromString($str) as str_shuffle($str) and rely on the strShuffle extension (and so on for others).

2. We could try to introduce some new method in the TypeSysteme, like

• shuffleString
• arrayPickKey
• ...

3. We could merge some random helpers class

Which solution(s) do you prefer ?

[staabm]

I think this PR implements a lot of stuff which is not necessary to solve phpstan/phpstan#15256

atm I see no reason why we should built such a extensive extensions for super precise result-types.

[VincentLanglet]

Rather than introducing lot of helper to deduplicate some utility method, we could write the Randomizer extension by relying on similar method.

For instance

• pickArrayKeys($a, $b) could just return the return value of array_rand($a, $b)
• shuffleBytes same idea with str_shuffle
• getInt, same idea with random_int

[VincentLanglet]

I think this PR implements a lot of stuff which is not necessary to solve phpstan/phpstan#15256

atm I see no reason why we should built such a extensive extensions for super precise result-types.

I consider splitting this PR, but still I think some extensions are usefull like the one for str_shuffle

And, considering lot of Randomizer method are just identical to native php functions which already have dynamicExtensions (and Randomizer methods are sometimes recommend like in https://www.php.net/manual/en/function.array-rand.php) ; It could be great to not lose type inference when switching them.

I'll try to improve/simplify the pr

[phpstan-bot]

Done — pushed to create-pull-request/patch-ctxs42z.

What changed

I took the review literally: the three new helper services are gone, and RandomizerMethodReturnTypeExtension now asks the extensions of the functions each method mirrors.

be1315f78 — Describe Randomizer methods through the extensions of the functions they mirror

• RandomArrayKeysReturnTypeHelper → folded into ArrayRandFunctionReturnTypeExtension as public getPickedKeyType() / getPickedKeysListType(). pickArrayKeys() calls the list variant — it can't reuse array_rand()'s return type wholesale, because $num is validated to be ≥ 1, so pickArrayKeys() always returns a list where array_rand() may return a bare key.
• StringBytesReturnTypeHelper → folded into StrShuffleFunctionReturnTypeExtension as public getShuffledStringType(). shuffleBytes() calls it, and so does StrrevFunctionReturnTypeExtension outside its constant-string case — reversing is a reordering of bytes just like shuffling.
• RandomIntRangeHelper → createRange() went back where it came from, as a public method on RandomIntFunctionReturnTypeExtension; getInt() calls it.
• shuffleArray() keeps using the Type::shuffleArray() primitive that shuffle() is built on.

41c8af89e — Leave Randomizer::getBytesFromString() to the function map

One thing I dropped rather than rewired, which is a deliberate scope reduction — flagging it in case you'd rather have it back. getBytesFromString() can't reuse the shuffle logic: a selection of bytes doesn't keep non-falsy-ness ('10' can yield '0'), so it needed a second, near-duplicate flavour of the helper. All it added over the non-empty-string the function map already gives is lowercase/uppercase preservation, so the extension case is gone and the functionMap_php83delta.php entry does the work. This also lines up with staabm's comment that the PR does more than #15256 needs. bug-15256-83.php now asserts the function-map types.

Net: −3 classes, −87 lines against the previous head.

Verification

• make tests: 21462 tests, 96660 assertions, 0 failures (95 pre-existing skips).
• make phpstan: no errors.
• No type-inference expectations changed apart from bug-15256-83.php — the 8.3 getBytesFromString() assertions, which follow from the dropped case above.

One note on the diff: composer dump-autoload is needed after adding/removing an #[AutowiredService] class, since the attributes file is generated. That's local-only, nothing tracked changed.

[VincentLanglet]

I didn't meant to inject the FunctionReturnTypeExtension as a dependency ; I meant to use something like $scope->getType(new FuncCall('array_shuffle')) ; would it be possible ?

And for Strrev and StrShuffle, it's ok to duplicate the logic ; so let strtrev untouched.

[phpstan-bot]

Done — pushed to create-pull-request/patch-ctxs42z.

What changed

6f7f4a62e — Ask the scope for the type of the function calls Randomizer methods mirror

RandomizerMethodReturnTypeExtension no longer takes any constructor dependencies. It builds the equivalent FuncCall and hands it to $scope->getType():

• pickArrayKeys($a, $n) → array_rand($a, 2)
• shuffleBytes($s) → str_shuffle($s)
• getInt($min, $max) → random_int($min, $max)

The 2 in the array_rand() call is deliberate and commented in the code: pickArrayKeys()'s $num is validated to be ≥ 1, so a successful call always returns a list of keys, which is the shape array_rand() describes when asked for more than one key. Passing $num through verbatim would produce the bare-key branch (or a union with it) for pickArrayKeys($a, 1) / pickArrayKeys($a, $n), which pickArrayKeys() never returns.

shuffleArray() stays on the Type::shuffleArray() primitive — shuffle() takes its array by reference and returns bool, so there is no function call to borrow a type from.

The three helper methods that were made public for the injection approach are private again: ArrayRandFunctionReturnTypeExtension::getPickedKeyType() / getPickedKeysListType() and RandomIntFunctionReturnTypeExtension::createRange() (the latter back to its original form, docblock and all).

2432be021 — Leave strrev() alone, keep the accessory logic inside each extension

StrrevFunctionReturnTypeExtension is byte-for-byte back to its pre-PR state — no constructor, no injected extension. The accessory-type logic lives in StrShuffleFunctionReturnTypeExtension::getTypeFromFunctionCall() directly, duplicated rather than shared.

Verification

• make tests: 21462 tests, 96660 assertions, 0 failures (95 pre-existing skips).
• make phpstan: no errors.
• make cs: clean.
• No test expectations changed — the inferred types are identical to the previous head, only the wiring differs.

[VincentLanglet]

I'm happy with the current state of the PR, I'm not sure it's needed to split it for review.

Lot of tests are failing, but I think it's already the case on 2.2.x, isn't it @staabm.
Do you know why ? Is someone on it ?

[staabm]

I think this looks good.

I would not merge this atm until the CI pipeline is green on 2.2.x again

[VincentLanglet]

I checked and didn't see extra error coming from this PR.