From 853cc0d4f1b13ede391048bbf2b78a6f914c9260 Mon Sep 17 00:00:00 2001 From: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Date: Wed, 6 May 2026 19:10:58 +0000 Subject: [PATCH] Do not propagate `isAlwaysTerminating` from immediately-invoked callable arguments to the outer call - Remove `isAlwaysTerminating` propagation from closure, arrow function, and other callable arguments in `NodeScopeResolver::processArgs()` when the parameter is marked as immediately-invoked - Remove now-unused `ProcessClosureResult::isAlwaysTerminating()` method and its backing field - Update `ExpressionResultTest` expectations for `call_user_func` with never-returning callbacks (minor false negative trade-off) - "Immediately invoked" means the callback runs during the function's execution, not that it is unconditionally called on every code path - Throw points and impure points still propagate correctly --- src/Analyser/NodeScopeResolver.php | 11 +-- src/Analyser/ProcessClosureResult.php | 6 -- .../PHPStan/Analyser/ExpressionResultTest.php | 4 +- tests/PHPStan/Analyser/nsrt/bug-14582.php | 19 +++++ .../DeadCode/UnreachableStatementRuleTest.php | 7 ++ .../PHPStan/Rules/DeadCode/data/bug-14582.php | 77 +++++++++++++++++++ 6 files changed, 107 insertions(+), 17 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-14582.php create mode 100644 tests/PHPStan/Rules/DeadCode/data/bug-14582.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 0bad74200b7..3cf5075cb14 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -2788,9 +2788,6 @@ public function processClosureNode( throw new ShouldNotHappenException(); } - $returnType = $closureType->getReturnType(); - $isAlwaysTerminating = ($returnType instanceof NeverType && $returnType->isExplicit()); - $this->callNodeCallback($nodeCallback, new InClosureNode($closureType, $expr), $closureScope, $storage); $executionEnds = []; @@ -2844,7 +2841,7 @@ public function processClosureNode( array_merge($publicStatementResult->getImpurePoints(), $closureImpurePoints), ), $closureScope, $storage); - return new ProcessClosureResult($scope, $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions, $isAlwaysTerminating); + return new ProcessClosureResult($scope, $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions); } $originalStorage = $storage; @@ -2894,7 +2891,7 @@ public function processClosureNode( array_merge($publicStatementResult->getImpurePoints(), $closureImpurePoints), ), $closureScope, $storage); - return new ProcessClosureResult($scope, $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions, $isAlwaysTerminating, $closureResultScope, $byRefUses); + return new ProcessClosureResult($scope, $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions, $closureResultScope, $byRefUses); } /** @@ -3402,7 +3399,6 @@ public function processArgs( if ($this->callCallbackImmediately($parameter, $parameterType, $calleeReflection)) { $throwPoints = array_merge($throwPoints, array_map(static fn (InternalThrowPoint $throwPoint) => $throwPoint->isExplicit() ? InternalThrowPoint::createExplicit($scope, $throwPoint->getType(), $arg->value, $throwPoint->canContainAnyThrowable()) : InternalThrowPoint::createImplicit($scope, $arg->value), $closureResult->getThrowPoints())); $impurePoints = array_merge($impurePoints, $closureResult->getImpurePoints()); - $isAlwaysTerminating = $isAlwaysTerminating || $closureResult->isAlwaysTerminating(); } $this->storeBeforeScope($storage, $arg->value, $scopeToPass); @@ -3462,7 +3458,6 @@ public function processArgs( if ($this->callCallbackImmediately($parameter, $parameterType, $calleeReflection)) { $throwPoints = array_merge($throwPoints, array_map(static fn (InternalThrowPoint $throwPoint) => $throwPoint->isExplicit() ? InternalThrowPoint::createExplicit($scope, $throwPoint->getType(), $arg->value, $throwPoint->canContainAnyThrowable()) : InternalThrowPoint::createImplicit($scope, $arg->value), $arrowFunctionResult->getThrowPoints())); $impurePoints = array_merge($impurePoints, $arrowFunctionResult->getImpurePoints()); - $isAlwaysTerminating = $isAlwaysTerminating || $arrowFunctionResult->isAlwaysTerminating(); } $this->storeBeforeScope($storage, $arg->value, $scopeToPass); } else { @@ -3492,8 +3487,6 @@ public function processArgs( } $throwPoints = array_merge($throwPoints, $callableThrowPoints); $impurePoints = array_merge($impurePoints, array_map(static fn (SimpleImpurePoint $impurePoint) => new ImpurePoint($scope, $arg->value, $impurePoint->getIdentifier(), $impurePoint->getDescription(), $impurePoint->isCertain()), $acceptors[0]->getImpurePoints())); - $returnType = $acceptors[0]->getReturnType(); - $isAlwaysTerminating = $isAlwaysTerminating || ($returnType instanceof NeverType && $returnType->isExplicit()); } } } diff --git a/src/Analyser/ProcessClosureResult.php b/src/Analyser/ProcessClosureResult.php index dd6cfa5d263..cb4b609e1f4 100644 --- a/src/Analyser/ProcessClosureResult.php +++ b/src/Analyser/ProcessClosureResult.php @@ -19,7 +19,6 @@ public function __construct( private array $throwPoints, private array $impurePoints, private array $invalidateExpressions, - private bool $isAlwaysTerminating, private ?MutatingScope $byRefClosureResultScope = null, private array $byRefUses = [], ) @@ -64,9 +63,4 @@ public function getInvalidateExpressions(): array return $this->invalidateExpressions; } - public function isAlwaysTerminating(): bool - { - return $this->isAlwaysTerminating; - } - } diff --git a/tests/PHPStan/Analyser/ExpressionResultTest.php b/tests/PHPStan/Analyser/ExpressionResultTest.php index 38b8b5d3685..4da555b362f 100644 --- a/tests/PHPStan/Analyser/ExpressionResultTest.php +++ b/tests/PHPStan/Analyser/ExpressionResultTest.php @@ -111,7 +111,7 @@ public static function dataIsAlwaysTerminating(): array ], [ 'call_user_func(fn() => exit());', - true, + false, ], [ '(function() { exit(); })();', @@ -123,7 +123,7 @@ public static function dataIsAlwaysTerminating(): array ], [ 'call_user_func(function() { exit(); });', - true, + false, ], [ 'usort($arr, static function($a, $b):int { return $a <=> $b; });', diff --git a/tests/PHPStan/Analyser/nsrt/bug-14582.php b/tests/PHPStan/Analyser/nsrt/bug-14582.php new file mode 100644 index 00000000000..023a650adaf --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14582.php @@ -0,0 +1,19 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug14582; + +use function PHPStan\Testing\assertType; + +function testArrayFilter(): void +{ + $b = array_filter([], fn() => throw new \Error()); + assertType('array{}', $b); +} + +function testArrayMap(): void +{ + $result = array_map(fn() => throw new \Error(), []); + assertType('array{}', $result); +} diff --git a/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php b/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php index 3b1bea93a44..963ae40e143 100644 --- a/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php +++ b/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php @@ -412,4 +412,11 @@ public function testBug14369(): void ]); } + #[RequiresPhp('>= 8.1.0')] + public function testBug14582(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-14582.php'], []); + } + } diff --git a/tests/PHPStan/Rules/DeadCode/data/bug-14582.php b/tests/PHPStan/Rules/DeadCode/data/bug-14582.php new file mode 100644 index 00000000000..38ec6940b67 --- /dev/null +++ b/tests/PHPStan/Rules/DeadCode/data/bug-14582.php @@ -0,0 +1,77 @@ += 8.1 + +declare(strict_types=1); + +namespace Bug14582; + +use Closure; +use RuntimeException; + +/** + * @template TLeft + * @template TRight + */ +interface Choice +{ + /** + * @template TResult + * + * @param (Closure(TRight): TResult) $right + * @param-immediately-invoked-callable $right + * @param (Closure(TLeft): TResult) $left + * @param-immediately-invoked-callable $left + * + * @return TResult + */ + public function proceed(Closure $right, Closure $left): mixed; +} + +/** @return Choice */ +function getChoice(bool $right): Choice +{ + throw new RuntimeException('stub'); +} + +function noop(): void +{ +} + +function doSomethingAfterwards(): void +{ +} + +function test(bool $right): void +{ + getChoice($right)->proceed( + right: noop(...), + left: fn (string $message) => throw new RuntimeException($message), + ); + doSomethingAfterwards(); +} + +function testArrayFilter(): void +{ + $b = array_filter([], fn() => throw new \Error()); + echo $b; +} + +function testArrayMap(): void +{ + array_map(fn() => throw new \Error(), []); + echo 'reachable'; +} + +function testUsort(): void +{ + $a = [1, 2, 3]; + usort($a, fn($a, $b) => throw new \Error()); + echo 'reachable'; +} + +function testClosureArgument(): void +{ + array_filter([], function () { + throw new \Error(); + }); + echo 'reachable'; +}