diff --git a/src/Rules/Functions/RandomIntParametersRule.php b/src/Rules/Functions/RandomIntParametersRule.php index 1205283002d..819129943b9 100644 --- a/src/Rules/Functions/RandomIntParametersRule.php +++ b/src/Rules/Functions/RandomIntParametersRule.php @@ -14,7 +14,9 @@ use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\IntegerRangeType; use PHPStan\Type\VerbosityLevel; +use function array_unique; use function count; +use function is_string; use function sprintf; /** @@ -65,7 +67,7 @@ public function processNode(Node $node, Scope $scope): array $isSmaller = $maxType->isSmallerThan($minType, $this->phpVersion); - if ($isSmaller->yes() || $isSmaller->maybe() && $this->reportMaybes) { + if ($isSmaller->yes() || $isSmaller->maybe() && $this->reportMaybes && !$this->isAlwaysValidDueToSharedVariables($args[0]->value, $args[1]->value, $scope)) { $message = 'Parameter #1 $min (%s) of function random_int expects lower number than parameter #2 $max (%s).'; return [ RuleErrorBuilder::message(sprintf( @@ -79,4 +81,142 @@ public function processNode(Node $node, Scope $scope): array return []; } + private function isAlwaysValidDueToSharedVariables(Node\Expr $minExpr, Node\Expr $maxExpr, Scope $scope): bool + { + $vars1 = $this->extractVariableNames($minExpr); + if ($vars1 === []) { + return false; + } + + $vars2 = $this->extractVariableNames($maxExpr); + + $hasShared = false; + foreach ($vars1 as $var => $_) { + if (isset($vars2[$var])) { + $hasShared = true; + break; + } + } + + if (!$hasShared) { + return false; + } + + // Get all variables from both expressions + $allVars = $vars1 + $vars2; + + // Get boundary values for each variable + $varBoundaries = []; + foreach ($allVars as $var => $_) { + $varType = $scope->getType(new Node\Expr\Variable($var))->toInteger(); + if ($varType instanceof ConstantIntegerType) { + $varBoundaries[$var] = [$varType->getValue()]; + } elseif ($varType instanceof IntegerRangeType) { + $min = $varType->getMin(); + $max = $varType->getMax(); + if ($min === null || $max === null) { + return false; + } + $varBoundaries[$var] = array_unique([$min, $max]); + } else { + return false; + } + } + + // Generate all combinations of boundary values + /** @var array> $combinations */ + $combinations = [[]]; + foreach ($varBoundaries as $var => $values) { + $newCombinations = []; + foreach ($combinations as $combo) { + foreach ($values as $value) { + $newCombo = $combo; + $newCombo[$var] = $value; + $newCombinations[] = $newCombo; + } + } + $combinations = $newCombinations; + } + + // For each combination, evaluate both expressions and check max >= min + foreach ($combinations as $combo) { + $minValue = $this->evaluateExpr($minExpr, $combo); + $maxValue = $this->evaluateExpr($maxExpr, $combo); + + if ($minValue === null || $maxValue === null) { + return false; + } + + if ($maxValue < $minValue) { + return false; + } + } + + return true; + } + + /** + * @param array $varValues + */ + private function evaluateExpr(Node\Expr $expr, array $varValues): ?int + { + if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) { + return $varValues[$expr->name] ?? null; + } + + if ($expr instanceof Node\Scalar\Int_) { + return $expr->value; + } + + if ($expr instanceof Node\Expr\BinaryOp) { + $left = $this->evaluateExpr($expr->left, $varValues); + $right = $this->evaluateExpr($expr->right, $varValues); + if ($left === null || $right === null) { + return null; + } + + return match (true) { + $expr instanceof Node\Expr\BinaryOp\Plus => $left + $right, + $expr instanceof Node\Expr\BinaryOp\Minus => $left - $right, + $expr instanceof Node\Expr\BinaryOp\Mul => $left * $right, + default => null, + }; + } + + if ($expr instanceof Node\Expr\UnaryMinus) { + $val = $this->evaluateExpr($expr->expr, $varValues); + return $val !== null ? -$val : null; + } + + if ($expr instanceof Node\Expr\UnaryPlus) { + return $this->evaluateExpr($expr->expr, $varValues); + } + + return null; + } + + /** + * @return array + */ + private function extractVariableNames(Node\Expr $expr): array + { + $vars = []; + if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) { + $vars[$expr->name] = true; + } + + foreach ($expr->getSubNodeNames() as $name) { + $subNode = $expr->{$name}; + if (!($subNode instanceof Node\Expr)) { + continue; + } + + foreach ($this->extractVariableNames($subNode) as $var => $_) { + $vars[$var] = true; + } + } + + return $vars; + } + } diff --git a/tests/PHPStan/Rules/Functions/RandomIntParametersRuleTest.php b/tests/PHPStan/Rules/Functions/RandomIntParametersRuleTest.php index 39160f054fc..cc2ea31da6f 100644 --- a/tests/PHPStan/Rules/Functions/RandomIntParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/RandomIntParametersRuleTest.php @@ -74,4 +74,14 @@ public function testBug6361(): void $this->analyse([__DIR__ . '/data/bug-6361.php'], []); } + public function testBug13092(): void + { + $this->analyse([__DIR__ . '/data/bug-13092.php'], [ + [ + 'Parameter #1 $min (int<1000, 10000>) of function random_int expects lower number than parameter #2 $max (int<999, 9999>).', + 16, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/bug-13092.php b/tests/PHPStan/Rules/Functions/data/bug-13092.php new file mode 100644 index 00000000000..d1f42c688fa --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-13092.php @@ -0,0 +1,18 @@ +