Skip to content

Invalidate maybe-impure function return values after impure method/static calls #5667

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
VincentLanglet merged 3 commits into from
Sep 21, 2026
+283 −80
Merged

Invalidate maybe-impure function return values after impure method/static calls #5667

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3ce9635
Invalidate maybe-impure function return values after impure method/st…
VincentLanglet May 14, 2026
617aaf0
Walk the specified expression without NodeFinder
phpstan-bot Sep 21, 2026
e90c57a
Remember specified expressions that contain no call at all
phpstan-bot Sep 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Invalidate maybe-impure function return values after impure method/st… 
…atic calls

When a maybe-impure function call's return value was narrowed via assert()
(e.g. assert(count(MyRecord::find()) === 1)), the narrowed type persisted
even after an impure method call like $msg2->insert(). This happened because
invalidateExpression() only invalidated expressions containing the specific
callee variable, not unrelated maybe-impure expressions whose results could
have been affected by the side effects.

Add invalidateAllMaybeImpureFunctionReturnValues() to MutatingScope which
walks stored expression types and removes any that contain maybe-impure
function/method/static calls. Call it from MethodCallHandler and
StaticCallHandler when processing calls with definite side effects
(hasSideEffects()->yes()), skipping $this-> calls which already invalidate
via invalidateExpression().

Closes phpstan/phpstan#13416
  • Loading branch information
@VincentLanglet
VincentLanglet committed Sep 21, 2026
commit 3ce9635e3b0662f7a6f27a9ec11c3cad34478db8
156 changes: 76 additions & 80 deletions src/Analyser/TypeSpecifier.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\NodeFinder;
use PHPStan\DependencyInjection\AutowiredService;
use PHPStan\DependencyInjection\Container;
use PHPStan\Node\Expr\AlwaysRememberedExpr;
Expand Down Expand Up @@ -573,91 +575,17 @@ private function createForExpr(
if (
Comment thread
staabm marked this conversation as resolved.
Resolved
$expr instanceof FuncCall
&& $expr->name instanceof Name
&& !$this->reflectionProvider->hasFunction($expr->name, $scope)
) {
$has = $this->reflectionProvider->hasFunction($expr->name, $scope);
if (!$has) {
// backwards compatibility with previous behaviour
return new SpecifiedTypes([], []);
}

$functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope);
$hasSideEffects = $functionReflection->hasSideEffects();
if ($hasSideEffects->yes()) {
return new SpecifiedTypes([], []);
}

if (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()) {
return new SpecifiedTypes([], []);
}
}

if (
$expr instanceof FuncCall
&& !$expr->name instanceof Name
) {
$nameType = $scope->getType($expr->name);
if ($nameType->isCallable()->yes()) {
$isPure = null;
foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) {
$variantIsPure = $variant->isPure();
$isPure = $isPure === null ? $variantIsPure : $isPure->and($variantIsPure);
}

if ($isPure !== null) {
if ($isPure->no()) {
return new SpecifiedTypes([], []);
}

if (!$this->rememberPossiblyImpureFunctionValues && !$isPure->yes()) {
return new SpecifiedTypes([], []);
}
}
}
}

if (
$expr instanceof MethodCall
&& $expr->name instanceof Node\Identifier
) {
$methodName = $expr->name->toString();
$calledOnType = $scope->getType($expr->var);
$methodReflection = $scope->getMethodReflection($calledOnType, $methodName);
if (
$methodReflection === null
|| $methodReflection->hasSideEffects()->yes()
|| (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no())
) {
if (isset($containsNull) && !$containsNull) {
return $this->createNullsafeTypes($originalExpr, $scope, $context, $type);
}

return new SpecifiedTypes([], []);
}
return new SpecifiedTypes([], []);
}

if (
$expr instanceof StaticCall
&& $expr->name instanceof Node\Identifier
) {
$methodName = $expr->name->toString();
if ($expr->class instanceof Name) {
$calledOnType = $scope->resolveTypeByName($expr->class);
} else {
$calledOnType = $scope->getType($expr->class);
if (!($expr instanceof AlwaysRememberedExpr) && $this->expressionContainsNonPureCall($expr, $scope)) {
if (isset($containsNull) && !$containsNull) {
return $this->createNullsafeTypes($originalExpr, $scope, $context, $type);
}

$methodReflection = $scope->getMethodReflection($calledOnType, $methodName);
if (
$methodReflection === null
|| $methodReflection->hasSideEffects()->yes()
|| (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no())
) {
if (isset($containsNull) && !$containsNull) {
return $this->createNullsafeTypes($originalExpr, $scope, $context, $type);
}

return new SpecifiedTypes([], []);
}
return new SpecifiedTypes([], []);
}

$sureTypes = [];
Expand Down Expand Up @@ -688,6 +616,74 @@ private function createForExpr(
return $types;
}

private function expressionContainsNonPureCall(Expr $expr, Scope $scope): bool
{
$nodeFinder = new NodeFinder();
$found = $nodeFinder->findFirst([$expr], function (Node $node) use ($scope): bool {

@staabm staabm May 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have the same bug on impure ArrowFunctions and impure Closures?
if so, add tests and expand the callable to cover it

@phpstan-bot phpstan-bot May 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed. Here's a summary of the changes:

Investigation: The reviewer asked whether impure ArrowFunctions and Closures are handled by expressionContainsNonPureCall. The answer is yes, the current code handles them correctly through two mechanisms:

  1. NodeFinder descends into closure/arrow function bodies — impure FuncCall/MethodCall/StaticCall nodes inside closure bodies are found and flagged (e.g., (fn() => MyRecord::find())())
  2. The FuncCall dynamic callable path checks ClosureType::isPure() via getCallableParametersAcceptors(), which catches impurity from non-call sources like echo

Tests added (7 new test cases):

  • Arrow function IIFE with impure static call in body
  • Closure IIFE with impure static call in body
  • strlen wrapping impure arrow function IIFE
  • Impure closure called via variable
  • Closure IIFE with echo + impure calls in body
  • Pure closure IIFE stays narrowed (positive case)
  • Pure function wrapping impure function with coalesce (covers #12734 pattern)

All 12071 tests pass and PHPStan self-analysis reports no errors.

Comment thread
VincentLanglet marked this conversation as resolved.
Resolved
if ($node instanceof FuncCall) {
if ($node->name instanceof Name) {
if (!$this->reflectionProvider->hasFunction($node->name, $scope)) {
return false;
}
$hasSideEffects = $this->reflectionProvider->getFunction($node->name, $scope)->hasSideEffects();
return $hasSideEffects->yes()
|| (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no());
}

$nameType = $scope->getType($node->name);
if ($nameType->isCallable()->yes()) {
$isPure = null;
foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) {
$variantIsPure = $variant->isPure();
$isPure = $isPure === null ? $variantIsPure : $isPure->and($variantIsPure);
}
if ($isPure !== null) {
return $isPure->no()
|| (!$this->rememberPossiblyImpureFunctionValues && !$isPure->yes());
}
}

return false;
}

if ($node instanceof MethodCall) {
if ($node->name instanceof Identifier) {
$calledOnType = $scope->getType($node->var);
$methodReflection = $scope->getMethodReflection($calledOnType, $node->name->name);
if ($methodReflection === null) {
return true;
}
$hasSideEffects = $methodReflection->hasSideEffects();
return $hasSideEffects->yes()
|| (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no());
}
return true;
Comment thread
staabm marked this conversation as resolved.
Resolved
}

if ($node instanceof StaticCall) {
if ($node->name instanceof Identifier) {
if ($node->class instanceof Name) {
$calledOnType = $scope->resolveTypeByName($node->class);
} else {
$calledOnType = $scope->getType($node->class);
}
$methodReflection = $scope->getMethodReflection($calledOnType, $node->name->name);
if ($methodReflection === null) {
return true;
}
$hasSideEffects = $methodReflection->hasSideEffects();
return $hasSideEffects->yes()
|| (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no());
}
return true;
}

return false;
});

return $found !== null;
}

private function createNullsafeTypes(Expr $expr, Scope $scope, TypeSpecifierContext $context, ?Type $type): SpecifiedTypes
{
if ($expr instanceof Expr\NullsafePropertyFetch) {
Expand Down
147 changes: 147 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-13416.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
<?php

declare(strict_types = 1);

namespace Bug13416;

use function PHPStan\Testing\assertType;

class MyRecord
{
/** @var list<self> */
private static array $storage = [];

/**
* @return list<self>
* @phpstan-impure
*/
public static function find(): array
{
return self::$storage;
}

/** @phpstan-impure */
public function insert(): void
{
self::$storage[] = $this;
}

/**
* @return non-empty-string
* @phpstan-impure
*/
public function getName(): string
{
return 'test';
}
}

class Repository
{
/**
* @return list<MyRecord>
* @phpstan-impure
*/
public function findAll(): array
{
return [];
}

/** @phpstan-impure */
public function save(MyRecord $record): void
{
}
}

function testImpureStaticCallNotNarrowedByCount(): void
{
assert(count(MyRecord::find()) === 1);
// Impure call result should not be narrowed
assertType('int<0, max>', count(MyRecord::find()));
}

function testImpureMethodCallNotNarrowedByCount(): void
{
$repo = new Repository();

assert(count($repo->findAll()) === 1);
// Impure call result should not be narrowed
assertType('int<0, max>', count($repo->findAll()));
}

function testStrlenOfImpureCallNotNarrowed(): void
{
$record = new MyRecord();

assert(strlen($record->getName()) === 3);
// strlen wrapping an impure call should not be narrowed
assertType('int<1, max>', strlen($record->getName()));
}

function testPureFunctionStaysNarrowed(): void
{
/** @var list<int> $arr */
$arr = [1];
assert(count($arr) === 1);
assertType('1', count($arr));

$x = rand(0, 10);

// Pure expressions stay narrowed
assertType('1', count($arr));
}

function testImpureArrowFunctionIIFE(): void
{
assert(count((fn() => MyRecord::find())()) === 1);
assertType('int<0, max>', count((fn() => MyRecord::find())()));
}

function testImpureClosureIIFE(): void
{
assert(count((function() { return MyRecord::find(); })()) === 1);
assertType('int<0, max>', count((function() { return MyRecord::find(); })()));
}

function testStrlenOfImpureArrowFunctionIIFE(): void
{
$record = new MyRecord();
assert(strlen((fn() => $record->getName())()) === 3);
assertType('int<1, max>', strlen((fn() => $record->getName())()));
}

function testImpureClosureViaVariable(): void
{
$fn = function(): array { return MyRecord::find(); };
assert(count($fn()) === 1);
assertType('int<0, max>', count($fn()));
}

function testImpureClosureWithEchoIIFE(): void
{
assert(strlen((function() { echo 'side-effect'; return MyRecord::find()[0]->getName(); })()) === 5);
assertType('int<1, max>', strlen((function() { echo 'side-effect'; return MyRecord::find()[0]->getName(); })()));
}

function testPureClosureIIFEStaysNarrowed(): void
{
/** @var list<int> $arr */
$arr = [1, 2, 3];
assert(count((fn() => $arr)()) === 3);
assertType('3', count((fn() => $arr)()));
}

/**
* @param string|null $val
* @phpstan-impure
*/
function impureFunction(?string $val): ?string
{
return $val;
}

function testPureOfImpureNotNarrowedByCoalesce(): void
{
$a = strlen(impureFunction('hello') ?? '') > 0;
assertType('bool', strlen(impureFunction('hello') ?? '') > 0);
}
Loading