Skip to content

Commit 51b2db1

Browse files
phpstan-botclaude
authored andcommitted
Check for impure sub-expressions in TypeSpecifier::createForExpr
Instead of only checking for `new` in the expression chain at type resolution time, prevent TypeSpecifier from storing narrowed types when the receiver chain or arguments contain impure calls. This generalizes the fix to handle named constructor patterns like `Repository::create()->getAll()` where `create()` is impure, and impure arguments like `$repo->getAll(Repository::create())`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e69f874 commit 51b2db1

1 file changed

Lines changed: 151 additions & 0 deletions

File tree

src/Analyser/TypeSpecifier.php

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2409,6 +2409,14 @@ private function createForExpr(
24092409
}
24102410
}
24112411

2412+
if ($this->subExpressionsHaveSideEffects($expr, $scope)) {
2413+
if (isset($containsNull) && !$containsNull) {
2414+
return $this->createNullsafeTypes($originalExpr, $scope, $context, $type);
2415+
}
2416+
2417+
return new SpecifiedTypes([], []);
2418+
}
2419+
24122420
$sureTypes = [];
24132421
$sureNotTypes = [];
24142422
if ($context->false()) {
@@ -2437,6 +2445,149 @@ private function createForExpr(
24372445
return $types;
24382446
}
24392447

2448+
private function subExpressionsHaveSideEffects(Expr $expr, Scope $scope): bool
2449+
{
2450+
if (
2451+
$expr instanceof MethodCall
2452+
|| $expr instanceof Expr\NullsafeMethodCall
2453+
|| $expr instanceof PropertyFetch
2454+
|| $expr instanceof Expr\NullsafePropertyFetch
2455+
|| $expr instanceof ArrayDimFetch
2456+
) {
2457+
if ($this->expressionHasSideEffects($expr->var, $scope)) {
2458+
return true;
2459+
}
2460+
} elseif (
2461+
$expr instanceof StaticCall
2462+
|| $expr instanceof StaticPropertyFetch
2463+
) {
2464+
if ($expr->class instanceof Expr && $this->expressionHasSideEffects($expr->class, $scope)) {
2465+
return true;
2466+
}
2467+
}
2468+
2469+
if ($expr instanceof Expr\CallLike && !$expr->isFirstClassCallable()) {
2470+
foreach ($expr->getArgs() as $arg) {
2471+
if ($this->expressionHasSideEffects($arg->value, $scope)) {
2472+
return true;
2473+
}
2474+
}
2475+
}
2476+
2477+
return false;
2478+
}
2479+
2480+
private function expressionHasSideEffects(Expr $expr, Scope $scope): bool
2481+
{
2482+
if ($expr instanceof Expr\New_) {
2483+
return true;
2484+
}
2485+
2486+
if ($expr instanceof FuncCall) {
2487+
if ($expr->isFirstClassCallable()) {
2488+
return false;
2489+
}
2490+
if ($expr->name instanceof Name) {
2491+
if (!$this->reflectionProvider->hasFunction($expr->name, $scope)) {
2492+
return true;
2493+
}
2494+
$functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope);
2495+
$hasSideEffects = $functionReflection->hasSideEffects();
2496+
if ($hasSideEffects->yes()) {
2497+
return true;
2498+
}
2499+
if (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()) {
2500+
return true;
2501+
}
2502+
} else {
2503+
return true;
2504+
}
2505+
foreach ($expr->getArgs() as $arg) {
2506+
if ($this->expressionHasSideEffects($arg->value, $scope)) {
2507+
return true;
2508+
}
2509+
}
2510+
return false;
2511+
}
2512+
2513+
if ($expr instanceof MethodCall || $expr instanceof Expr\NullsafeMethodCall) {
2514+
if ($expr->isFirstClassCallable()) {
2515+
return $this->expressionHasSideEffects($expr->var, $scope);
2516+
}
2517+
if ($expr->name instanceof Node\Identifier) {
2518+
$calledOnType = $scope->getType($expr->var);
2519+
$methodReflection = $scope->getMethodReflection($calledOnType, $expr->name->toString());
2520+
if (
2521+
$methodReflection === null
2522+
|| $methodReflection->hasSideEffects()->yes()
2523+
|| (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no())
2524+
) {
2525+
return true;
2526+
}
2527+
} else {
2528+
return true;
2529+
}
2530+
foreach ($expr->getArgs() as $arg) {
2531+
if ($this->expressionHasSideEffects($arg->value, $scope)) {
2532+
return true;
2533+
}
2534+
}
2535+
return $this->expressionHasSideEffects($expr->var, $scope);
2536+
}
2537+
2538+
if ($expr instanceof StaticCall) {
2539+
if ($expr->isFirstClassCallable()) {
2540+
if ($expr->class instanceof Expr) {
2541+
return $this->expressionHasSideEffects($expr->class, $scope);
2542+
}
2543+
return false;
2544+
}
2545+
if ($expr->name instanceof Node\Identifier) {
2546+
if ($expr->class instanceof Name) {
2547+
$calledOnType = $scope->resolveTypeByName($expr->class);
2548+
} else {
2549+
$calledOnType = $scope->getType($expr->class);
2550+
}
2551+
$methodReflection = $scope->getMethodReflection($calledOnType, $expr->name->toString());
2552+
if (
2553+
$methodReflection === null
2554+
|| $methodReflection->hasSideEffects()->yes()
2555+
|| (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no())
2556+
) {
2557+
return true;
2558+
}
2559+
} else {
2560+
return true;
2561+
}
2562+
foreach ($expr->getArgs() as $arg) {
2563+
if ($this->expressionHasSideEffects($arg->value, $scope)) {
2564+
return true;
2565+
}
2566+
}
2567+
if ($expr->class instanceof Expr) {
2568+
return $this->expressionHasSideEffects($expr->class, $scope);
2569+
}
2570+
return false;
2571+
}
2572+
2573+
if ($expr instanceof PropertyFetch || $expr instanceof Expr\NullsafePropertyFetch) {
2574+
return $this->expressionHasSideEffects($expr->var, $scope);
2575+
}
2576+
2577+
if ($expr instanceof ArrayDimFetch) {
2578+
return $this->expressionHasSideEffects($expr->var, $scope);
2579+
}
2580+
2581+
if ($expr instanceof StaticPropertyFetch) {
2582+
if ($expr->class instanceof Expr) {
2583+
return $this->expressionHasSideEffects($expr->class, $scope);
2584+
}
2585+
return false;
2586+
}
2587+
2588+
return false;
2589+
}
2590+
24402591
private function createNullsafeTypes(Expr $expr, Scope $scope, TypeSpecifierContext $context, ?Type $type): SpecifiedTypes
24412592
{
24422593
if ($expr instanceof Expr\NullsafePropertyFetch) {

0 commit comments

Comments
 (0)