Skip to content

Commit 8f8a19b

Browse files
phpstan-botclaude
andcommitted
Guard ExprPrinter comparison with determinism check for impure method calls
ExprPrinter comparison alone is insufficient because impure method calls like `$obj->impureMethod()` may return different values on each invocation, even though the printed expression is identical. Add isDeterministicExpr() to verify the expression is safe to compare: variables, property fetches, static property fetches, and pure method/static calls are deterministic. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a09c9df commit 8f8a19b

1 file changed

Lines changed: 44 additions & 3 deletions

File tree

src/Rules/Arrays/NonexistentOffsetInArrayDimFetchRule.php

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace PHPStan\Rules\Arrays;
44

55
use PhpParser\Node;
6+
use PhpParser\Node\Expr;
67
use PhpParser\Node\Expr\ArrayDimFetch;
78
use PhpParser\Node\Expr\Variable;
89
use PHPStan\Analyser\NullsafeOperatorHelper;
@@ -122,7 +123,8 @@ public function processNode(Node $node, Scope $scope): array
122123
$arrayArg = $node->dim->getArgs()[0]->value;
123124
$arrayType = $scope->getType($arrayArg);
124125
if (
125-
$this->exprPrinter->printExpr($arrayArg) === $this->exprPrinter->printExpr($node->var)
126+
$this->isDeterministicExpr($node->var, $scope)
127+
&& $this->exprPrinter->printExpr($arrayArg) === $this->exprPrinter->printExpr($node->var)
126128
&& $arrayType->isArray()->yes()
127129
&& $arrayType->isIterableAtLeastOnce()->yes()
128130
) {
@@ -145,7 +147,8 @@ public function processNode(Node $node, Scope $scope): array
145147
$arrayType = $scope->getType($arrayArg);
146148

147149
if (
148-
$this->exprPrinter->printExpr($arrayArg) === $this->exprPrinter->printExpr($node->var)
150+
$this->isDeterministicExpr($node->var, $scope)
151+
&& $this->exprPrinter->printExpr($arrayArg) === $this->exprPrinter->printExpr($node->var)
149152
&& $arrayType->isArray()->yes()
150153
&& $arrayType->isIterableAtLeastOnce()->yes()
151154
&& ($numArg === null || $one->isSuperTypeOf($scope->getType($numArg))->yes())
@@ -166,7 +169,8 @@ public function processNode(Node $node, Scope $scope): array
166169
$arrayArg = $node->dim->left->getArgs()[0]->value;
167170
$arrayType = $scope->getType($arrayArg);
168171
if (
169-
$this->exprPrinter->printExpr($arrayArg) === $this->exprPrinter->printExpr($node->var)
172+
$this->isDeterministicExpr($node->var, $scope)
173+
&& $this->exprPrinter->printExpr($arrayArg) === $this->exprPrinter->printExpr($node->var)
170174
&& $arrayType->isList()->yes()
171175
&& $arrayType->isIterableAtLeastOnce()->yes()
172176
) {
@@ -182,6 +186,43 @@ public function processNode(Node $node, Scope $scope): array
182186
);
183187
}
184188

189+
private function isDeterministicExpr(Expr $expr, Scope $scope): bool
190+
{
191+
if ($expr instanceof Variable) {
192+
return true;
193+
}
194+
195+
if ($expr instanceof Expr\PropertyFetch) {
196+
return $this->isDeterministicExpr($expr->var, $scope);
197+
}
198+
199+
if ($expr instanceof Expr\StaticPropertyFetch) {
200+
return true;
201+
}
202+
203+
if ($expr instanceof Expr\MethodCall && $expr->name instanceof Node\Identifier) {
204+
$callerType = $scope->getType($expr->var);
205+
$methodReflection = $scope->getMethodReflection($callerType, $expr->name->name);
206+
if ($methodReflection !== null && $methodReflection->hasSideEffects()->no()) {
207+
return $this->isDeterministicExpr($expr->var, $scope);
208+
}
209+
210+
return false;
211+
}
212+
213+
if ($expr instanceof Expr\StaticCall && $expr->name instanceof Node\Identifier && $expr->class instanceof Node\Name) {
214+
$classType = $scope->resolveTypeByName($expr->class);
215+
$methodReflection = $scope->getMethodReflection($classType, $expr->name->name);
216+
if ($methodReflection !== null && $methodReflection->hasSideEffects()->no()) {
217+
return true;
218+
}
219+
220+
return false;
221+
}
222+
223+
return false;
224+
}
225+
185226
private function isImplicitArrayCreation(Node\Expr\ArrayDimFetch $node, Scope $scope): TrinaryLogic
186227
{
187228
$varNode = $node->var;

0 commit comments

Comments
 (0)