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

Open
phpstan-bot wants to merge 11 commits into
base: 2.1.x
Choose a base branch
from
+240 −80
Open

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e9ba032
Invalidate maybe-impure function return values after impure method/st…
VincentLanglet May 14, 2026
923d2b1
Prevent type narrowing of expressions containing impure calls
phpstan-bot May 15, 2026
e731e88
Remove redundant early returns subsumed by expressionContainsNonPureCall
phpstan-bot May 15, 2026
452a3c8
Return TrinaryLogic from expressionContainsNonPureCall
phpstan-bot May 15, 2026
7687df3
Add tests for impure closures and arrow functions
phpstan-bot May 15, 2026
1fd7cb8
Revert "Return TrinaryLogic from expressionContainsNonPureCall"
staabm May 15, 2026
968e01a
Update TypeSpecifier.php
staabm May 15, 2026
aadf81a
Do not treat unknown functions as non-pure in sub-expression purity c…
phpstan-bot May 15, 2026
9d80f7d
Add non-regression test for unknown function sub-expression handling
phpstan-bot May 15, 2026
5ef8c3d
Replace unit test with NSRT integration test for FAUX_FUNCTION handling
phpstan-bot May 15, 2026
159e29a
Restore unit test for unknown function sub-expression handling
phpstan-bot May 15, 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
164 changes: 78 additions & 86 deletions src/Analyser/TypeSpecifier.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Expr\StaticPropertyFetch;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\NodeFinder;
use PHPStan\Analyser\ExprHandler\BooleanAndHandler;
use PHPStan\DependencyInjection\AutowiredService;
use PHPStan\Node\Expr\AlwaysRememberedExpr;
Expand Down Expand Up @@ -2536,94 +2538,12 @@
}
}

if (
$expr instanceof FuncCall
&& $expr->name instanceof Name
) {
$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 AlwaysRememberedExpr) && $this->expressionContainsNonPureCall($expr, $scope)) {
if (isset($containsNull) && !$containsNull) {
return $this->createNullsafeTypes($originalExpr, $scope, $context, $type);
}
}

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([], []);
}
}

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);
}

$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 @@ -2654,6 +2574,78 @@
return $types;
}

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

@staabm staabm May 15, 2026 •
edited
Loading

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

Copy link
Copy Markdown
Collaborator Author

@phpstan-bot phpstan-bot May 15, 2026

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.

if (!$node instanceof Expr) {
return false;
}

if ($node instanceof FuncCall) {
if ($node->name instanceof Name) {
if (!$this->reflectionProvider->hasFunction($node->name, $scope)) {
return true;
}
$hasSideEffects = $this->reflectionProvider->getFunction($node->name, $scope)->hasSideEffects();
return $hasSideEffects->yes()

Check warning on line 2591 in src/Analyser/TypeSpecifier.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 						return true;
 					}
 					$hasSideEffects = $this->reflectionProvider->getFunction($node->name, $scope)->hasSideEffects();
-					return $hasSideEffects->yes()
+					return !$hasSideEffects->no()
 						|| (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no());
 				}

Check warning on line 2591 in src/Analyser/TypeSpecifier.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 						return true;
 					}
 					$hasSideEffects = $this->reflectionProvider->getFunction($node->name, $scope)->hasSideEffects();
-					return $hasSideEffects->yes()
+					return !$hasSideEffects->no()
 						|| (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no());
 				}
|| (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no());
}

$nameType = $scope->getType($node->name);
if ($nameType->isCallable()->yes()) {

Check warning on line 2596 in src/Analyser/TypeSpecifier.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 				}
 
 				$nameType = $scope->getType($node->name);
-				if ($nameType->isCallable()->yes()) {
+				if (!$nameType->isCallable()->no()) {
 					$isPure = null;
 					foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) {
 						$variantIsPure = $variant->isPure();

Check warning on line 2596 in src/Analyser/TypeSpecifier.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

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

Check warning on line 2603 in src/Analyser/TypeSpecifier.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 						$isPure = $isPure === null ? $variantIsPure : $isPure->and($variantIsPure);
 					}
 					if ($isPure !== null) {
-						return $isPure->no()
+						return !$isPure->yes()
 							|| (!$this->rememberPossiblyImpureFunctionValues && !$isPure->yes());
 					}
 				}

Check warning on line 2603 in src/Analyser/TypeSpecifier.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 						$isPure = $isPure === null ? $variantIsPure : $isPure->and($variantIsPure);
 					}
 					if ($isPure !== null) {
-						return $isPure->no()
+						return !$isPure->yes()
 							|| (!$this->rememberPossiblyImpureFunctionValues && !$isPure->yes());
 					}
 				}
|| (!$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.
}

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
92 changes: 92 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,92 @@
<?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));
}
Loading