From 966bf938aca79f243c4371d7123ab53885e35c1d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 27 Feb 2026 14:26:10 +0000 Subject: [PATCH] Fix throw points not properly matched to catch clauses - Always include implicit throw points when matching to catch clauses, regardless of whether explicit non-throw-statement matches exist - Previously, when a method with @throws matched a catch clause, implicit throw points from other method calls were excluded from the catch scope - This caused variables assigned between the two calls to be incorrectly reported as "Undefined variable" instead of "might not be defined" - Removed unused $onlyExplicitIsThrow flag and InternalThrowPoint::getNode() - Updated test expectations in bug-4821.php and explicit-throws.php to reflect the corrected (more sound) behavior Closes https://github.com/phpstan/phpstan/issues/9349 --- src/Analyser/InternalThrowPoint.php | 8 --- src/Analyser/NodeScopeResolver.php | 28 +++------ tests/PHPStan/Analyser/nsrt/bug-4821.php | 2 +- .../PHPStan/Analyser/nsrt/explicit-throws.php | 4 +- .../Analyser/nsrt/throw-points/bug-9349.php | 57 +++++++++++++++++++ 5 files changed, 69 insertions(+), 30 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/throw-points/bug-9349.php diff --git a/src/Analyser/InternalThrowPoint.php b/src/Analyser/InternalThrowPoint.php index 92291bd5ac..605d791bf7 100644 --- a/src/Analyser/InternalThrowPoint.php +++ b/src/Analyser/InternalThrowPoint.php @@ -70,14 +70,6 @@ public function getType(): Type return $this->type; } - /** - * @return Node\Expr|Node\Stmt - */ - public function getNode() - { - return $this->node; - } - public function isExplicit(): bool { return $this->explicit; diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 86f616953b..5a90910c51 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1915,7 +1915,6 @@ private function processStmtNode( } // explicit only - $onlyExplicitIsThrow = true; if (count($matchingThrowPoints) === 0) { foreach ($throwPoints as $throwPointIndex => $throwPoint) { foreach ($catchTypes as $catchTypeIndex => $catchTypeItem) { @@ -1927,32 +1926,23 @@ private function processStmtNode( if (!$throwPoint->isExplicit()) { continue; } - $throwNode = $throwPoint->getNode(); - if ( - !$throwNode instanceof Expr\Throw_ - && !($throwNode instanceof Node\Stmt\Expression && $throwNode->expr instanceof Expr\Throw_) - ) { - $onlyExplicitIsThrow = false; - } $matchingThrowPoints[$throwPointIndex] = $throwPoint; } } } - // implicit only - if (count($matchingThrowPoints) === 0 || $onlyExplicitIsThrow) { - foreach ($throwPoints as $throwPointIndex => $throwPoint) { - if ($throwPoint->isExplicit()) { + // implicit + foreach ($throwPoints as $throwPointIndex => $throwPoint) { + if ($throwPoint->isExplicit()) { + continue; + } + + foreach ($catchTypes as $catchTypeItem) { + if ($catchTypeItem->isSuperTypeOf($throwPoint->getType())->no()) { continue; } - foreach ($catchTypes as $catchTypeItem) { - if ($catchTypeItem->isSuperTypeOf($throwPoint->getType())->no()) { - continue; - } - - $matchingThrowPoints[$throwPointIndex] = $throwPoint; - } + $matchingThrowPoints[$throwPointIndex] = $throwPoint; } } diff --git a/tests/PHPStan/Analyser/nsrt/bug-4821.php b/tests/PHPStan/Analyser/nsrt/bug-4821.php index 340fd88ab2..af6c493ccc 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-4821.php +++ b/tests/PHPStan/Analyser/nsrt/bug-4821.php @@ -16,7 +16,7 @@ public function sayHello(): void return; } catch (\ReflectionException $e) { assertVariableCertainty(TrinaryLogic::createYes(), $object); - assertVariableCertainty(TrinaryLogic::createNo(), $method); + assertVariableCertainty(TrinaryLogic::createMaybe(), $method); } } diff --git a/tests/PHPStan/Analyser/nsrt/explicit-throws.php b/tests/PHPStan/Analyser/nsrt/explicit-throws.php index e37d6be94a..e734c0401b 100644 --- a/tests/PHPStan/Analyser/nsrt/explicit-throws.php +++ b/tests/PHPStan/Analyser/nsrt/explicit-throws.php @@ -26,7 +26,7 @@ public function doBar(): void $a = 1; $this->throwInvalidArgument(); } catch (\InvalidArgumentException $e) { - assertVariableCertainty(TrinaryLogic::createYes(), $a); + assertVariableCertainty(TrinaryLogic::createMaybe(), $a); } } @@ -38,7 +38,7 @@ public function doBaz(): void $this->throwInvalidArgument(); throw new \InvalidArgumentException(); } catch (\InvalidArgumentException $e) { - assertVariableCertainty(TrinaryLogic::createYes(), $a); + assertVariableCertainty(TrinaryLogic::createMaybe(), $a); } } diff --git a/tests/PHPStan/Analyser/nsrt/throw-points/bug-9349.php b/tests/PHPStan/Analyser/nsrt/throw-points/bug-9349.php new file mode 100644 index 0000000000..8d40e06473 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/throw-points/bug-9349.php @@ -0,0 +1,57 @@ +throwsRuntime(); + $sql = 'SELECT * FROM foo'; + $foo->doSomething(); + } catch (\PDOException $e) { + // throwsRuntime() declares @throws RuntimeException + // PDOException extends RuntimeException, so throwsRuntime() + // might throw a PDOException before $sql is assigned. + // But doSomething() also has implicit throws after $sql is assigned. + // So $sql might or might not be defined. + assertVariableCertainty(TrinaryLogic::createMaybe(), $sql); + } +}; + +function (Foo $foo): void { + try { + $foo->throwsLogic(); + $sql = 'SELECT * FROM foo'; + $foo->doSomething(); + } catch (\PDOException $e) { + // LogicException and PDOException are unrelated + // Only implicit throws from doSomething() can reach here + // $sql is assigned before doSomething() + assertVariableCertainty(TrinaryLogic::createYes(), $sql); + } +};