Skip to content

Commit 1fe954b

Browse files
phpstan-botclaude
andcommitted
Evaluate shared variable expressions at boundary values instead of blanket 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>
1 parent 8cf5d64 commit 1fe954b

File tree

3 files changed

+115
-7
lines changed

3 files changed

+115
-7
lines changed

src/Rules/Functions/RandomIntParametersRule.php

Lines changed: 103 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use PHPStan\Type\Constant\ConstantIntegerType;
1515
use PHPStan\Type\IntegerRangeType;
1616
use PHPStan\Type\VerbosityLevel;
17+
use function array_unique;
1718
use function count;
1819
use function is_string;
1920
use function sprintf;
@@ -66,7 +67,7 @@ public function processNode(Node $node, Scope $scope): array
6667

6768
$isSmaller = $maxType->isSmallerThan($minType, $this->phpVersion);
6869

69-
if ($isSmaller->yes() || $isSmaller->maybe() && $this->reportMaybes && !$this->sharesVariable($args[0]->value, $args[1]->value)) {
70+
if ($isSmaller->yes() || $isSmaller->maybe() && $this->reportMaybes && !$this->isAlwaysValidDueToSharedVariables($args[0]->value, $args[1]->value, $scope)) {
7071
$message = 'Parameter #1 $min (%s) of function random_int expects lower number than parameter #2 $max (%s).';
7172
return [
7273
RuleErrorBuilder::message(sprintf(
@@ -80,22 +81,118 @@ public function processNode(Node $node, Scope $scope): array
8081
return [];
8182
}
8283

83-
private function sharesVariable(Node\Expr $expr1, Node\Expr $expr2): bool
84+
private function isAlwaysValidDueToSharedVariables(Node\Expr $minExpr, Node\Expr $maxExpr, Scope $scope): bool
8485
{
85-
$vars1 = $this->extractVariableNames($expr1);
86+
$vars1 = $this->extractVariableNames($minExpr);
8687
if ($vars1 === []) {
8788
return false;
8889
}
8990

90-
$vars2 = $this->extractVariableNames($expr2);
91+
$vars2 = $this->extractVariableNames($maxExpr);
9192

93+
$hasShared = false;
9294
foreach ($vars1 as $var => $_) {
9395
if (isset($vars2[$var])) {
94-
return true;
96+
$hasShared = true;
97+
break;
9598
}
9699
}
97100

98-
return false;
101+
if (!$hasShared) {
102+
return false;
103+
}
104+
105+
// Get all variables from both expressions
106+
$allVars = $vars1 + $vars2;
107+
108+
// Get boundary values for each variable
109+
$varBoundaries = [];
110+
foreach ($allVars as $var => $_) {
111+
$varType = $scope->getType(new Node\Expr\Variable($var))->toInteger();
112+
if ($varType instanceof ConstantIntegerType) {
113+
$varBoundaries[$var] = [$varType->getValue()];
114+
} elseif ($varType instanceof IntegerRangeType) {
115+
$min = $varType->getMin();
116+
$max = $varType->getMax();
117+
if ($min === null || $max === null) {
118+
return false;
119+
}
120+
$varBoundaries[$var] = array_unique([$min, $max]);
121+
} else {
122+
return false;
123+
}
124+
}
125+
126+
// Generate all combinations of boundary values
127+
/** @var array<array<string, int>> $combinations */
128+
$combinations = [[]];
129+
foreach ($varBoundaries as $var => $values) {
130+
$newCombinations = [];
131+
foreach ($combinations as $combo) {
132+
foreach ($values as $value) {
133+
$newCombo = $combo;
134+
$newCombo[$var] = $value;
135+
$newCombinations[] = $newCombo;
136+
}
137+
}
138+
$combinations = $newCombinations;
139+
}
140+
141+
// For each combination, evaluate both expressions and check max >= min
142+
foreach ($combinations as $combo) {
143+
$minValue = $this->evaluateExpr($minExpr, $combo);
144+
$maxValue = $this->evaluateExpr($maxExpr, $combo);
145+
146+
if ($minValue === null || $maxValue === null) {
147+
return false;
148+
}
149+
150+
if ($maxValue < $minValue) {
151+
return false;
152+
}
153+
}
154+
155+
return true;
156+
}
157+
158+
/**
159+
* @param array<string, int> $varValues
160+
*/
161+
private function evaluateExpr(Node\Expr $expr, array $varValues): ?int
162+
{
163+
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
164+
return $varValues[$expr->name] ?? null;
165+
}
166+
167+
if ($expr instanceof Node\Scalar\Int_) {
168+
return $expr->value;
169+
}
170+
171+
if ($expr instanceof Node\Expr\BinaryOp) {
172+
$left = $this->evaluateExpr($expr->left, $varValues);
173+
$right = $this->evaluateExpr($expr->right, $varValues);
174+
if ($left === null || $right === null) {
175+
return null;
176+
}
177+
178+
return match (true) {
179+
$expr instanceof Node\Expr\BinaryOp\Plus => $left + $right,
180+
$expr instanceof Node\Expr\BinaryOp\Minus => $left - $right,
181+
$expr instanceof Node\Expr\BinaryOp\Mul => $left * $right,
182+
default => null,
183+
};
184+
}
185+
186+
if ($expr instanceof Node\Expr\UnaryMinus) {
187+
$val = $this->evaluateExpr($expr->expr, $varValues);
188+
return $val !== null ? -$val : null;
189+
}
190+
191+
if ($expr instanceof Node\Expr\UnaryPlus) {
192+
return $this->evaluateExpr($expr->expr, $varValues);
193+
}
194+
195+
return null;
99196
}
100197

101198
/**

tests/PHPStan/Rules/Functions/RandomIntParametersRuleTest.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,12 @@ public function testBug6361(): void
7676

7777
public function testBug13092(): void
7878
{
79-
$this->analyse([__DIR__ . '/data/bug-13092.php'], []);
79+
$this->analyse([__DIR__ . '/data/bug-13092.php'], [
80+
[
81+
'Parameter #1 $min (int<1000, 10000>) of function random_int expects lower number than parameter #2 $max (int<999, 9999>).',
82+
16,
83+
],
84+
]);
8085
}
8186

8287
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,10 @@ public function sayHello(): void
99
$shoppers = \random_int(1000, 10000);
1010
$transactions = \random_int($shoppers, $shoppers * 3);
1111
}
12+
13+
public function shouldStillReport(): void
14+
{
15+
$shoppers = \random_int(1000, 10000);
16+
$transactions = \random_int($shoppers, $shoppers - 1);
17+
}
1218
}

0 commit comments

Comments
 (0)