Fix phpstan/phpstan#13092: Using result of random_int in another random_int results in an error#5411
Closed
phpstan-bot wants to merge 2 commits intophpstan:2.1.xfrom
Closed
Conversation
…args - Skip reporting "maybe" errors in RandomIntParametersRule when both arguments share a variable reference - When $min and $max are derived from the same variable (e.g. random_int($x, $x * 3)), the ranges are correlated and max can never be less than min - Added sharesVariable/extractVariableNames helper methods to detect shared variables in argument expressions - New regression test in tests/PHPStan/Rules/Functions/data/bug-13092.php
Contributor
VincentLanglet
left a comment
VincentLanglet
left a comment
There was a problem hiding this comment.
I don't like this this because it won't report anymore
\random_int($shoppers, $shoppers - 1);
…anket suppression Instead of suppressing all "maybe" errors when arguments share a variable, evaluate both expressions at the boundary values of shared variables and only suppress when max >= min for all combinations. This ensures cases like random_int($x, $x - 1) are still reported as errors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
|
Both background tasks confirmed complete — |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When using the result of
random_int()in anotherrandom_int()call (e.g.random_int($shoppers, $shoppers * 3)), PHPStan incorrectly reported that$minexpects a lower number than$max. This is a false positive because the ranges are correlated through the shared variable - if$shoppersis 1000 then max is 3000, if$shoppersis 10000 then max is 30000, so max is always >= min.Changes
src/Rules/Functions/RandomIntParametersRule.phpto skip reporting "maybe" errors when both arguments share a variable referencesharesVariable()andextractVariableNames()helper methods that recursively extract variable names from argument expressions and check for overlaptests/PHPStan/Rules/Functions/data/bug-13092.phpRoot cause
The
RandomIntParametersRulechecks if$max < $minis possible. With$shoppersasint<1000, 10000>and$shoppers * 3asint<3000, 30000>, the ranges overlap (min could be 10000, max could be 3000), soisSmallerThanreturns "maybe". With strict rules enabled (reportMaybes=true), this was reported as an error. However, since both arguments are derived from the same variable, the ranges are correlated and max can never actually be less than min. The fix detects shared variables between the two argument expressions and suppresses "maybe" reports in that case.Test
Added
tests/PHPStan/Rules/Functions/data/bug-13092.phpwhich reproduces the exact scenario from the issue:$shoppers = random_int(1000, 10000); $transactions = random_int($shoppers, $shoppers * 3);- expects no errors.Fixes phpstan/phpstan#13092