Skip to content

Commit 966bf93

Browse files
github-actions[bot]phpstan-bot
authored andcommitted
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 phpstan/phpstan#9349
1 parent ba4fe14 commit 966bf93

File tree

5 files changed

+69
-30
lines changed

5 files changed

+69
-30
lines changed

src/Analyser/InternalThrowPoint.php

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,6 @@ public function getType(): Type
7070
return $this->type;
7171
}
7272

73-
/**
74-
* @return Node\Expr|Node\Stmt
75-
*/
76-
public function getNode()
77-
{
78-
return $this->node;
79-
}
80-
8173
public function isExplicit(): bool
8274
{
8375
return $this->explicit;

src/Analyser/NodeScopeResolver.php

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1915,7 +1915,6 @@ private function processStmtNode(
19151915
}
19161916

19171917
// explicit only
1918-
$onlyExplicitIsThrow = true;
19191918
if (count($matchingThrowPoints) === 0) {
19201919
foreach ($throwPoints as $throwPointIndex => $throwPoint) {
19211920
foreach ($catchTypes as $catchTypeIndex => $catchTypeItem) {
@@ -1927,32 +1926,23 @@ private function processStmtNode(
19271926
if (!$throwPoint->isExplicit()) {
19281927
continue;
19291928
}
1930-
$throwNode = $throwPoint->getNode();
1931-
if (
1932-
!$throwNode instanceof Expr\Throw_
1933-
&& !($throwNode instanceof Node\Stmt\Expression && $throwNode->expr instanceof Expr\Throw_)
1934-
) {
1935-
$onlyExplicitIsThrow = false;
1936-
}
19371929
$matchingThrowPoints[$throwPointIndex] = $throwPoint;
19381930
}
19391931
}
19401932
}
19411933

1942-
// implicit only
1943-
if (count($matchingThrowPoints) === 0 || $onlyExplicitIsThrow) {
1944-
foreach ($throwPoints as $throwPointIndex => $throwPoint) {
1945-
if ($throwPoint->isExplicit()) {
1934+
// implicit
1935+
foreach ($throwPoints as $throwPointIndex => $throwPoint) {
1936+
if ($throwPoint->isExplicit()) {
1937+
continue;
1938+
}
1939+
1940+
foreach ($catchTypes as $catchTypeItem) {
1941+
if ($catchTypeItem->isSuperTypeOf($throwPoint->getType())->no()) {
19461942
continue;
19471943
}
19481944

1949-
foreach ($catchTypes as $catchTypeItem) {
1950-
if ($catchTypeItem->isSuperTypeOf($throwPoint->getType())->no()) {
1951-
continue;
1952-
}
1953-
1954-
$matchingThrowPoints[$throwPointIndex] = $throwPoint;
1955-
}
1945+
$matchingThrowPoints[$throwPointIndex] = $throwPoint;
19561946
}
19571947
}
19581948

tests/PHPStan/Analyser/nsrt/bug-4821.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public function sayHello(): void
1616
return;
1717
} catch (\ReflectionException $e) {
1818
assertVariableCertainty(TrinaryLogic::createYes(), $object);
19-
assertVariableCertainty(TrinaryLogic::createNo(), $method);
19+
assertVariableCertainty(TrinaryLogic::createMaybe(), $method);
2020
}
2121
}
2222

tests/PHPStan/Analyser/nsrt/explicit-throws.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public function doBar(): void
2626
$a = 1;
2727
$this->throwInvalidArgument();
2828
} catch (\InvalidArgumentException $e) {
29-
assertVariableCertainty(TrinaryLogic::createYes(), $a);
29+
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
3030
}
3131
}
3232

@@ -38,7 +38,7 @@ public function doBaz(): void
3838
$this->throwInvalidArgument();
3939
throw new \InvalidArgumentException();
4040
} catch (\InvalidArgumentException $e) {
41-
assertVariableCertainty(TrinaryLogic::createYes(), $a);
41+
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
4242
}
4343
}
4444

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<?php
2+
3+
namespace ThrowPoints\Bug9349;
4+
5+
use PHPStan\TrinaryLogic;
6+
use function PHPStan\Testing\assertType;
7+
use function PHPStan\Testing\assertVariableCertainty;
8+
9+
class Foo
10+
{
11+
/**
12+
* @throws \RuntimeException
13+
*/
14+
public function throwsRuntime(): void
15+
{
16+
}
17+
18+
/**
19+
* @throws \LogicException
20+
*/
21+
public function throwsLogic(): void
22+
{
23+
}
24+
25+
/** @return mixed */
26+
public function doSomething(): void
27+
{
28+
}
29+
}
30+
31+
function (Foo $foo): void {
32+
try {
33+
$foo->throwsRuntime();
34+
$sql = 'SELECT * FROM foo';
35+
$foo->doSomething();
36+
} catch (\PDOException $e) {
37+
// throwsRuntime() declares @throws RuntimeException
38+
// PDOException extends RuntimeException, so throwsRuntime()
39+
// might throw a PDOException before $sql is assigned.
40+
// But doSomething() also has implicit throws after $sql is assigned.
41+
// So $sql might or might not be defined.
42+
assertVariableCertainty(TrinaryLogic::createMaybe(), $sql);
43+
}
44+
};
45+
46+
function (Foo $foo): void {
47+
try {
48+
$foo->throwsLogic();
49+
$sql = 'SELECT * FROM foo';
50+
$foo->doSomething();
51+
} catch (\PDOException $e) {
52+
// LogicException and PDOException are unrelated
53+
// Only implicit throws from doSomething() can reach here
54+
// $sql is assigned before doSomething()
55+
assertVariableCertainty(TrinaryLogic::createYes(), $sql);
56+
}
57+
};

0 commit comments

Comments
 (0)