From 2790ea10438b04ae899577b499d69474a7bf7979 Mon Sep 17 00:00:00 2001 From: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Date: Sat, 23 May 2026 22:28:27 +0000 Subject: [PATCH 1/4] Only suppress implicit throw point for Throwable-returning calls inside `throw` expressions - Add `inThrow` flag to `ExpressionContext` so `ThrowHandler` can signal to child expression handlers that they are inside a `throw` expression - In `FuncCallHandler`, `MethodThrowPointHelper`, and `NewHandler`, only skip the implicit throw point when both conditions hold: the call is inside a `throw` expression AND the return type is Throwable - Previously, the Throwable return type check unconditionally suppressed implicit throw points, causing false "dead catch" reports for calls like `$this->returnThrowable()` that were not inside a `throw` statement - Fix the same pattern in `NewHandler` for constructor calls on Throwable classes (e.g. `new Exception()` outside `throw`) - Update existing test assertion in throw-points/try-catch.php for `$foo = new \InvalidArgumentException()` where variable certainty in the catch block is now correctly `No` instead of `Yes` --- src/Analyser/ExprHandler/FuncCallHandler.php | 7 ++- .../Helper/MethodThrowPointHelper.php | 6 ++ .../ExprHandler/MethodCallHandler.php | 2 +- src/Analyser/ExprHandler/NewHandler.php | 8 ++- .../ExprHandler/StaticCallHandler.php | 2 +- src/Analyser/ExprHandler/ThrowHandler.php | 2 +- src/Analyser/ExpressionContext.php | 15 ++++- .../Analyser/nsrt/throw-points/try-catch.php | 2 +- .../CatchWithUnthrownExceptionRuleTest.php | 5 ++ .../Rules/Exceptions/data/bug-9826.php | 61 +++++++++++++++++++ 10 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 tests/PHPStan/Rules/Exceptions/data/bug-9826.php diff --git a/src/Analyser/ExprHandler/FuncCallHandler.php b/src/Analyser/ExprHandler/FuncCallHandler.php index 2e308bb43ee..f59a6de9236 100644 --- a/src/Analyser/ExprHandler/FuncCallHandler.php +++ b/src/Analyser/ExprHandler/FuncCallHandler.php @@ -311,7 +311,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex } if ($functionReflection !== null) { - $functionThrowPoint = $this->getFunctionThrowPoint($functionReflection, $parametersAcceptor, $normalizedExpr, $scope); + $functionThrowPoint = $this->getFunctionThrowPoint($functionReflection, $parametersAcceptor, $normalizedExpr, $scope, $context); if ($functionThrowPoint !== null) { $throwPoints[] = $functionThrowPoint; } @@ -580,6 +580,7 @@ private function getFunctionThrowPoint( ?ParametersAcceptor $parametersAcceptor, FuncCall $normalizedFuncCall, MutatingScope $scope, + ExpressionContext $context, ): ?InternalThrowPoint { foreach ($this->dynamicThrowTypeExtensionProvider->getDynamicFunctionThrowTypeExtensions() as $extension) { @@ -625,6 +626,10 @@ private function getFunctionThrowPoint( || $requiredParameters > 0 || count($normalizedFuncCall->getArgs()) > 0 ) { + if (!$context->isInThrow()) { + return InternalThrowPoint::createImplicit($scope, $normalizedFuncCall); + } + $functionReturnedType = $scope->getType($normalizedFuncCall); if (!(new ObjectType(Throwable::class))->isSuperTypeOf($functionReturnedType)->yes()) { return InternalThrowPoint::createImplicit($scope, $normalizedFuncCall); diff --git a/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php b/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php index 5401642d4c8..50f3989d600 100644 --- a/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php +++ b/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php @@ -4,6 +4,7 @@ use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\StaticCall; +use PHPStan\Analyser\ExpressionContext; use PHPStan\Analyser\InternalThrowPoint; use PHPStan\Analyser\MutatingScope; use PHPStan\DependencyInjection\AutowiredParameter; @@ -35,6 +36,7 @@ public function getThrowPoint( ParametersAcceptor $parametersAcceptor, MethodCall|StaticCall $normalizedMethodCall, MutatingScope $scope, + ?ExpressionContext $context = null, ): ?InternalThrowPoint { if ($normalizedMethodCall instanceof MethodCall) { @@ -86,6 +88,10 @@ public function getThrowPoint( return InternalThrowPoint::createExplicit($scope, $throwType, $normalizedMethodCall, true); } } elseif ($this->implicitThrows) { + if ($context === null || !$context->isInThrow()) { + return InternalThrowPoint::createImplicit($scope, $normalizedMethodCall); + } + $methodReturnedType = $scope->getType($normalizedMethodCall); if (!(new ObjectType(Throwable::class))->isSuperTypeOf($methodReturnedType)->yes()) { return InternalThrowPoint::createImplicit($scope, $normalizedMethodCall); diff --git a/src/Analyser/ExprHandler/MethodCallHandler.php b/src/Analyser/ExprHandler/MethodCallHandler.php index f2dc9fd9af2..0ee068b195d 100644 --- a/src/Analyser/ExprHandler/MethodCallHandler.php +++ b/src/Analyser/ExprHandler/MethodCallHandler.php @@ -144,7 +144,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $scope = $argsResult->getScope(); if ($methodReflection !== null) { - $methodThrowPoint = $this->methodThrowPointHelper->getThrowPoint($methodReflection, $parametersAcceptor, $normalizedExpr, $scope); + $methodThrowPoint = $this->methodThrowPointHelper->getThrowPoint($methodReflection, $parametersAcceptor, $normalizedExpr, $scope, $context); if ($methodThrowPoint !== null) { $throwPoints[] = $methodThrowPoint; } diff --git a/src/Analyser/ExprHandler/NewHandler.php b/src/Analyser/ExprHandler/NewHandler.php index 57cc504a3a3..922ae12ce7a 100644 --- a/src/Analyser/ExprHandler/NewHandler.php +++ b/src/Analyser/ExprHandler/NewHandler.php @@ -200,7 +200,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex if ($constructorReflection !== null && $parametersAcceptor !== null) { $className ??= $constructorReflection->getDeclaringClass()->getName(); - $constructorThrowPoint = $this->getConstructorThrowPoint($constructorReflection, $parametersAcceptor, $expr, new Name\FullyQualified($className), $expr->getArgs(), $scope); + $constructorThrowPoint = $this->getConstructorThrowPoint($constructorReflection, $parametersAcceptor, $expr, new Name\FullyQualified($className), $expr->getArgs(), $scope, $context); if ($constructorThrowPoint !== null) { $throwPoints[] = $constructorThrowPoint; } @@ -275,7 +275,7 @@ private function processConstructorReflection(string $className, New_ $expr, Mut /** * @param list $args */ - private function getConstructorThrowPoint(MethodReflection $constructorReflection, ParametersAcceptor $parametersAcceptor, New_ $new, Name $className, array $args, MutatingScope $scope): ?InternalThrowPoint + private function getConstructorThrowPoint(MethodReflection $constructorReflection, ParametersAcceptor $parametersAcceptor, New_ $new, Name $className, array $args, MutatingScope $scope, ExpressionContext $context): ?InternalThrowPoint { $methodCall = new StaticCall($className, $constructorReflection->getName(), $args); $normalizedMethodCall = ArgumentsNormalizer::reorderStaticCallArguments($parametersAcceptor, $methodCall); @@ -300,6 +300,10 @@ private function getConstructorThrowPoint(MethodReflection $constructorReflectio return InternalThrowPoint::createExplicit($scope, $throwType, $new, true); } } elseif ($this->implicitThrows) { + if (!$context->isInThrow()) { + return InternalThrowPoint::createImplicit($scope, $methodCall); + } + if (!$constructorReflection->getDeclaringClass()->is(Throwable::class)) { return InternalThrowPoint::createImplicit($scope, $methodCall); } diff --git a/src/Analyser/ExprHandler/StaticCallHandler.php b/src/Analyser/ExprHandler/StaticCallHandler.php index 2869dcee52f..66814a6a003 100644 --- a/src/Analyser/ExprHandler/StaticCallHandler.php +++ b/src/Analyser/ExprHandler/StaticCallHandler.php @@ -207,7 +207,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $scopeFunction = $scope->getFunction(); if ($methodReflection !== null) { - $methodThrowPoint = $this->methodThrowPointHelper->getThrowPoint($methodReflection, $parametersAcceptor, $normalizedExpr, $scope); + $methodThrowPoint = $this->methodThrowPointHelper->getThrowPoint($methodReflection, $parametersAcceptor, $normalizedExpr, $scope, $context); if ($methodThrowPoint !== null) { $throwPoints[] = $methodThrowPoint; } diff --git a/src/Analyser/ExprHandler/ThrowHandler.php b/src/Analyser/ExprHandler/ThrowHandler.php index 3866bcfe57d..5590da256bf 100644 --- a/src/Analyser/ExprHandler/ThrowHandler.php +++ b/src/Analyser/ExprHandler/ThrowHandler.php @@ -31,7 +31,7 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { - $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, ExpressionContext::createDeep()); + $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, ExpressionContext::createDeep()->enterThrow()); return new ExpressionResult( $scope, diff --git a/src/Analyser/ExpressionContext.php b/src/Analyser/ExpressionContext.php index ad0d16618ee..e84a2f8cc78 100644 --- a/src/Analyser/ExpressionContext.php +++ b/src/Analyser/ExpressionContext.php @@ -11,6 +11,7 @@ private function __construct( private bool $isDeep, private ?string $inAssignRightSideVariableName, private ?Expr $inAssignRightSideExpr, + private bool $inThrow = false, ) { } @@ -31,7 +32,7 @@ public function enterDeep(): self return $this; } - return new self(true, $this->inAssignRightSideVariableName, $this->inAssignRightSideExpr); + return new self(true, $this->inAssignRightSideVariableName, $this->inAssignRightSideExpr, $this->inThrow); } public function isDeep(): bool @@ -39,9 +40,19 @@ public function isDeep(): bool return $this->isDeep; } + public function enterThrow(): self + { + return new self($this->isDeep, $this->inAssignRightSideVariableName, $this->inAssignRightSideExpr, true); + } + + public function isInThrow(): bool + { + return $this->inThrow; + } + public function enterRightSideAssign(string $variableName, Expr $expr): self { - return new self($this->isDeep, $variableName, $expr); + return new self($this->isDeep, $variableName, $expr, $this->inThrow); } public function getInAssignRightSideVariableName(): ?string diff --git a/tests/PHPStan/Analyser/nsrt/throw-points/try-catch.php b/tests/PHPStan/Analyser/nsrt/throw-points/try-catch.php index 87c0be66eb3..9950b1ccb13 100644 --- a/tests/PHPStan/Analyser/nsrt/throw-points/try-catch.php +++ b/tests/PHPStan/Analyser/nsrt/throw-points/try-catch.php @@ -115,7 +115,7 @@ function (): void { try { $foo = new \InvalidArgumentException(); } catch (Throwable $e) { - assertVariableCertainty(TrinaryLogic::createYes(), $foo); + assertVariableCertainty(TrinaryLogic::createNo(), $foo); } }; diff --git a/tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php b/tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php index 76ddadd6470..b72065feddf 100644 --- a/tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php +++ b/tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php @@ -821,4 +821,9 @@ public function testBug7799(): void $this->analyse([__DIR__ . '/data/bug-7799.php'], []); } + public function testBug9826(): void + { + $this->analyse([__DIR__ . '/data/bug-9826.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Exceptions/data/bug-9826.php b/tests/PHPStan/Rules/Exceptions/data/bug-9826.php new file mode 100644 index 00000000000..0356a328f00 --- /dev/null +++ b/tests/PHPStan/Rules/Exceptions/data/bug-9826.php @@ -0,0 +1,61 @@ +returnVoid(); + } catch (\Throwable $e) { + // ok - implicit throws enabled + } + + try { + $this->returnThrowable(); + } catch (\Throwable $e) { + // ok - method can implicitly throw even though it returns Throwable + } + + try { + self::staticReturnThrowable(); + } catch (\Throwable $e) { + // ok - static method can implicitly throw even though it returns Throwable + } + + try { + returnThrowable(); + } catch (\Throwable $e) { + // ok - function can implicitly throw even though it returns Throwable + } + + try { + new \Exception('test'); + } catch (\Throwable $e) { + // ok - new Exception() can implicitly throw even though it constructs Throwable + } + } + +} + +function returnThrowable(): \Exception +{ + return new \Exception(); +} From a07dc29c57ac1a779689130662f247e1472ee82a Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 23 May 2026 22:45:25 +0000 Subject: [PATCH 2/4] Remove default null from MethodThrowPointHelper::getThrowPoint $context parameter Since this is not API, the default value is not needed. Pass ExpressionContext explicitly from ImplicitToStringCallHelper. Co-Authored-By: Claude Opus 4.6 --- .../ExprHandler/Helper/ImplicitToStringCallHelper.php | 2 ++ src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Analyser/ExprHandler/Helper/ImplicitToStringCallHelper.php b/src/Analyser/ExprHandler/Helper/ImplicitToStringCallHelper.php index fc2d6246260..eab62846f88 100644 --- a/src/Analyser/ExprHandler/Helper/ImplicitToStringCallHelper.php +++ b/src/Analyser/ExprHandler/Helper/ImplicitToStringCallHelper.php @@ -4,6 +4,7 @@ use PhpParser\Node\Expr; use PhpParser\Node\Identifier; +use PHPStan\Analyser\ExpressionContext; use PHPStan\Analyser\ExpressionResult; use PHPStan\Analyser\ImpurePoint; use PHPStan\Analyser\MutatingScope; @@ -59,6 +60,7 @@ public function processImplicitToStringCall(Expr $expr, MutatingScope $scope): E $toStringMethod->getOnlyVariant(), new Expr\MethodCall($expr, new Identifier('__toString')), $scope, + ExpressionContext::createDeep(), ); if ($throwPoint !== null) { $throwPoints[] = $throwPoint; diff --git a/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php b/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php index 50f3989d600..6bcf6d397b4 100644 --- a/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php +++ b/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php @@ -36,7 +36,7 @@ public function getThrowPoint( ParametersAcceptor $parametersAcceptor, MethodCall|StaticCall $normalizedMethodCall, MutatingScope $scope, - ?ExpressionContext $context = null, + ExpressionContext $context, ): ?InternalThrowPoint { if ($normalizedMethodCall instanceof MethodCall) { @@ -88,7 +88,7 @@ public function getThrowPoint( return InternalThrowPoint::createExplicit($scope, $throwType, $normalizedMethodCall, true); } } elseif ($this->implicitThrows) { - if ($context === null || !$context->isInThrow()) { + if (!$context->isInThrow()) { return InternalThrowPoint::createImplicit($scope, $normalizedMethodCall); } From 70dde7c368c8d7aed5aa4e2f420880b9c945fceb Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 23 May 2026 23:09:34 +0000 Subject: [PATCH 3/4] Preserve never-type implicit throw point suppression for dynamically-typed calls The previous commit's early return when `!isInThrow()` bypassed the return type check entirely, creating false implicit throw points for never-returning calls like `trigger_error("hello", E_USER_ERROR)`. The original Throwable supertype check accidentally suppressed `never` (bottom type, subtype of everything). Separate the two concerns: - `never` return type: always suppress (function terminates, not throws) - Throwable return type: only suppress inside `throw` expressions Co-Authored-By: Claude Opus 4.6 --- src/Analyser/ExprHandler/FuncCallHandler.php | 6 +----- .../ExprHandler/Helper/MethodThrowPointHelper.php | 6 +----- tests/PHPStan/Rules/Exceptions/data/bug-9826.php | 9 +++++++++ 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/Analyser/ExprHandler/FuncCallHandler.php b/src/Analyser/ExprHandler/FuncCallHandler.php index f59a6de9236..3bc7f370b91 100644 --- a/src/Analyser/ExprHandler/FuncCallHandler.php +++ b/src/Analyser/ExprHandler/FuncCallHandler.php @@ -626,12 +626,8 @@ private function getFunctionThrowPoint( || $requiredParameters > 0 || count($normalizedFuncCall->getArgs()) > 0 ) { - if (!$context->isInThrow()) { - return InternalThrowPoint::createImplicit($scope, $normalizedFuncCall); - } - $functionReturnedType = $scope->getType($normalizedFuncCall); - if (!(new ObjectType(Throwable::class))->isSuperTypeOf($functionReturnedType)->yes()) { + if (!$context->isInThrow() || !(new ObjectType(Throwable::class))->isSuperTypeOf($functionReturnedType)->yes()) { return InternalThrowPoint::createImplicit($scope, $normalizedFuncCall); } } diff --git a/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php b/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php index 6bcf6d397b4..08ff8735587 100644 --- a/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php +++ b/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php @@ -88,12 +88,8 @@ public function getThrowPoint( return InternalThrowPoint::createExplicit($scope, $throwType, $normalizedMethodCall, true); } } elseif ($this->implicitThrows) { - if (!$context->isInThrow()) { - return InternalThrowPoint::createImplicit($scope, $normalizedMethodCall); - } - $methodReturnedType = $scope->getType($normalizedMethodCall); - if (!(new ObjectType(Throwable::class))->isSuperTypeOf($methodReturnedType)->yes()) { + if (!$context->isInThrow() || !(new ObjectType(Throwable::class))->isSuperTypeOf($methodReturnedType)->yes()) { return InternalThrowPoint::createImplicit($scope, $normalizedMethodCall); } } diff --git a/tests/PHPStan/Rules/Exceptions/data/bug-9826.php b/tests/PHPStan/Rules/Exceptions/data/bug-9826.php index 0356a328f00..2979cfe9837 100644 --- a/tests/PHPStan/Rules/Exceptions/data/bug-9826.php +++ b/tests/PHPStan/Rules/Exceptions/data/bug-9826.php @@ -59,3 +59,12 @@ function returnThrowable(): \Exception { return new \Exception(); } + +function triggerErrorNeverReturns(): void +{ + try { + $a = trigger_error("hello", E_USER_ERROR); + } catch (\Exception $e) { + // ok - trigger_error returns never (explicit), gets throw point like any never-returning function + } +} From b4bb14ebe8f06ca692ade764f2c9244967ebb9b9 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sun, 24 May 2026 19:25:28 +0000 Subject: [PATCH 4/4] Address review: merge conditions in NewHandler, remove triggerError test Co-Authored-By: Claude Opus 4.6 --- src/Analyser/ExprHandler/NewHandler.php | 6 +----- tests/PHPStan/Rules/Exceptions/data/bug-9826.php | 9 --------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/src/Analyser/ExprHandler/NewHandler.php b/src/Analyser/ExprHandler/NewHandler.php index 922ae12ce7a..41c6210da74 100644 --- a/src/Analyser/ExprHandler/NewHandler.php +++ b/src/Analyser/ExprHandler/NewHandler.php @@ -300,11 +300,7 @@ private function getConstructorThrowPoint(MethodReflection $constructorReflectio return InternalThrowPoint::createExplicit($scope, $throwType, $new, true); } } elseif ($this->implicitThrows) { - if (!$context->isInThrow()) { - return InternalThrowPoint::createImplicit($scope, $methodCall); - } - - if (!$constructorReflection->getDeclaringClass()->is(Throwable::class)) { + if (!$context->isInThrow() || !$constructorReflection->getDeclaringClass()->is(Throwable::class)) { return InternalThrowPoint::createImplicit($scope, $methodCall); } } diff --git a/tests/PHPStan/Rules/Exceptions/data/bug-9826.php b/tests/PHPStan/Rules/Exceptions/data/bug-9826.php index 2979cfe9837..0356a328f00 100644 --- a/tests/PHPStan/Rules/Exceptions/data/bug-9826.php +++ b/tests/PHPStan/Rules/Exceptions/data/bug-9826.php @@ -59,12 +59,3 @@ function returnThrowable(): \Exception { return new \Exception(); } - -function triggerErrorNeverReturns(): void -{ - try { - $a = trigger_error("hello", E_USER_ERROR); - } catch (\Exception $e) { - // ok - trigger_error returns never (explicit), gets throw point like any never-returning function - } -}