Skip to content

Commit e0182ce

Browse files
ondrejmirtesondrejmirtes
committed
Better try-catch logic
1 parent e53474b commit e0182ce

11 files changed

Lines changed: 171 additions & 42 deletions

File tree

‎build/phpstan.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ parameters:
3131
- ../tests/e2e/magic-setter/*
3232
- ../tests/PHPStan/Rules/Properties/UninitializedPropertyRuleTest.php
3333
- ../tests/PHPStan/Command/IgnoredRegexValidatorTest.php
34+
- ../src/Command/IgnoredRegexValidator.php
3435
featureToggles:
3536
readComposerPhpVersion: false
3637
ignoreErrors:

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 58 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ private function processStmtNode(
689689

690690
if (!$conditionType instanceof ConstantBooleanType || $conditionType->getValue()) {
691691
$exitPoints = $branchScopeStatementResult->getExitPoints();
692-
$throwPoints = $branchScopeStatementResult->getThrowPoints();
692+
$throwPoints = array_merge($throwPoints, $branchScopeStatementResult->getThrowPoints());
693693
$branchScope = $branchScopeStatementResult->getScope();
694694
$finalScope = $branchScopeStatementResult->isAlwaysTerminating() ? null : $branchScope;
695695
$alwaysTerminating = $branchScopeStatementResult->isAlwaysTerminating();
@@ -1126,23 +1126,8 @@ private function processStmtNode(
11261126
$branchScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $scope, $nodeCallback);
11271127
$branchScope = $branchScopeResult->getScope();
11281128
$finalScope = $branchScopeResult->isAlwaysTerminating() ? null : $branchScope;
1129-
if (count($branchScopeResult->getThrowPoints()) > 0) {
1130-
$allCaught = false;
1131-
foreach ($stmt->catches as $catch) {
1132-
foreach ($catch->types as $type) {
1133-
if ($type->toString() === \Throwable::class) {
1134-
continue;
1135-
}
1136-
$allCaught = true;
1137-
}
1138-
}
1139-
if (!$allCaught) {
1140-
$branchScope = $branchScope->mergeWith($branchScopeResult->getThrowPoints()[0]->getScope());
1141-
}
1142-
}
1143-
$tryScope = $branchScope;
1129+
11441130
$exitPoints = [];
1145-
$throwPoints = [];
11461131
$alwaysTerminating = $branchScopeResult->isAlwaysTerminating();
11471132
$hasYield = $branchScopeResult->hasYield();
11481133

@@ -1158,34 +1143,78 @@ private function processStmtNode(
11581143
$exitPoints[] = $exitPoint;
11591144
}
11601145

1161-
// todo filter caught
1162-
$throwPoints = array_merge($throwPoints, $branchScopeResult->getThrowPoints());
1146+
$throwPoints = $branchScopeResult->getThrowPoints();
1147+
foreach ($throwPoints as $throwPoint) {
1148+
if ($finallyScope === null) {
1149+
continue;
1150+
}
1151+
$finallyScope = $finallyScope->mergeWith($throwPoint->getScope());
1152+
}
1153+
1154+
$throwPointsForLater = [];
11631155

11641156
foreach ($stmt->catches as $catchNode) {
11651157
$nodeCallback($catchNode, $scope);
1166-
if (!$this->polluteCatchScopeWithTryAssignments) {
1167-
$catchScopeResult = $this->processCatchNode($catchNode, $scope->mergeWith($tryScope), $nodeCallback);
1168-
$catchScopeForFinally = $catchScopeResult->getScope();
1158+
$catchType = TypeCombinator::union(...array_map(static function (Name $name): Type {
1159+
return new ObjectType($name->toString());
1160+
}, $catchNode->types));
1161+
$matchingThrowPoints = [];
1162+
$newThrowPoints = [];
1163+
foreach ($throwPoints as $throwPoint) {
1164+
$isSuperType = $catchType->isSuperTypeOf($throwPoint->getType());
1165+
if (!$isSuperType->no()) {
1166+
$matchingThrowPoints[] = $throwPoint;
1167+
}
1168+
if ($isSuperType->yes()) {
1169+
continue;
1170+
}
1171+
$newThrowPoints[] = $throwPoint;
1172+
}
1173+
$throwPoints = $newThrowPoints;
1174+
if (count($matchingThrowPoints) === 0) {
1175+
if (!$this->polluteCatchScopeWithTryAssignments) {
1176+
$catchScope = $scope->mergeWith($finalScope);
1177+
} else {
1178+
$catchScope = $branchScope;
1179+
}
11691180
} else {
1170-
$catchScopeForFinally = $this->processCatchNode($catchNode, $tryScope, $nodeCallback)->getScope();
1171-
$catchScopeResult = $this->processCatchNode($catchNode, $scope->mergeWith($tryScope), static function (): void {
1172-
});
1181+
$catchScope = null;
1182+
foreach ($matchingThrowPoints as $matchingThrowPoint) {
1183+
if ($catchScope === null) {
1184+
$catchScope = $matchingThrowPoint->getScope();
1185+
} else {
1186+
$catchScope = $catchScope->mergeWith($matchingThrowPoint->getScope());
1187+
}
1188+
}
1189+
}
1190+
1191+
$catchScopeResult = $this->processCatchNode($catchNode, $catchScope, $nodeCallback);
1192+
if (count($matchingThrowPoints) === 0) {
1193+
continue;
11731194
}
11741195

11751196
$finalScope = $catchScopeResult->isAlwaysTerminating() ? $finalScope : $catchScopeResult->getScope()->mergeWith($finalScope);
11761197
$alwaysTerminating = $alwaysTerminating && $catchScopeResult->isAlwaysTerminating();
11771198
$hasYield = $hasYield || $catchScopeResult->hasYield();
1199+
$catchThrowPoints = $catchScopeResult->getThrowPoints();
1200+
$throwPointsForLater = array_merge($throwPointsForLater, $catchThrowPoints);
11781201

11791202
if ($finallyScope !== null) {
1180-
$finallyScope = $finallyScope->mergeWith($catchScopeForFinally);
1203+
$finallyScope = $finallyScope->mergeWith($catchScopeResult->getScope());
11811204
}
11821205
foreach ($catchScopeResult->getExitPoints() as $exitPoint) {
11831206
if ($finallyScope !== null) {
11841207
$finallyScope = $finallyScope->mergeWith($exitPoint->getScope());
11851208
}
11861209
$exitPoints[] = $exitPoint;
11871210
}
1188-
$throwPoints = array_merge($throwPoints, $catchScopeResult->getThrowPoints());
1211+
1212+
foreach ($catchThrowPoints as $catchThrowPoint) {
1213+
if ($finallyScope === null) {
1214+
continue;
1215+
}
1216+
$finallyScope = $finallyScope->mergeWith($catchThrowPoint->getScope());
1217+
}
11891218
}
11901219

11911220
if ($finalScope === null) {
@@ -1197,13 +1226,13 @@ private function processStmtNode(
11971226
$finallyResult = $this->processStmtNodes($stmt->finally, $stmt->finally->stmts, $finallyScope, $nodeCallback);
11981227
$alwaysTerminating = $alwaysTerminating || $finallyResult->isAlwaysTerminating();
11991228
$hasYield = $hasYield || $finallyResult->hasYield();
1229+
$throwPointsForLater = array_merge($throwPointsForLater, $finallyResult->getThrowPoints());
12001230
$finallyScope = $finallyResult->getScope();
12011231
$finalScope = $finallyResult->isAlwaysTerminating() ? $finalScope : $finalScope->processFinallyScope($finallyScope, $originalFinallyScope);
12021232
$exitPoints = array_merge($exitPoints, $finallyResult->getExitPoints());
1203-
$throwPoints = array_merge($throwPoints, $finallyResult->getThrowPoints());
12041233
}
12051234

1206-
return new StatementResult($finalScope, $hasYield, $alwaysTerminating, $exitPoints, $throwPoints);
1235+
return new StatementResult($finalScope, $hasYield, $alwaysTerminating, $exitPoints, array_merge($throwPoints, $throwPointsForLater));
12071236
} elseif ($stmt instanceof Unset_) {
12081237
$hasYield = false;
12091238
$throwPoints = [];

‎tests/PHPStan/Analyser/NodeScopeResolverTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5809,6 +5809,7 @@ public function dataThrowPoints(): iterable
58095809
yield from $this->gatherAssertTypes(__DIR__ . '/data/throw-points/try-catch-finally.php');
58105810
yield from $this->gatherAssertTypes(__DIR__ . '/data/throw-points/variable.php');
58115811
yield from $this->gatherAssertTypes(__DIR__ . '/data/throw-points/while.php');
5812+
yield from $this->gatherAssertTypes(__DIR__ . '/data/throw-points/try-catch.php');
58125813
}
58135814

58145815
/**

‎tests/PHPStan/Analyser/StatementResultTest.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public function dataIsAlwaysTerminating(): array
144144
true,
145145
],
146146
[
147-
'try { return; } catch (Exception $e) { }',
147+
'try { maybeThrow(); return; } catch (Exception $e) { }',
148148
false,
149149
],
150150
[
@@ -156,7 +156,7 @@ public function dataIsAlwaysTerminating(): array
156156
true,
157157
],
158158
[
159-
'try { break; } catch (Exception $e) { break; } catch (OtherException $e) { }',
159+
'try { maybeThrow(); break; } catch (Exception $e) { break; } catch (OtherException $e) { }',
160160
false,
161161
],
162162
[
@@ -324,15 +324,15 @@ public function dataIsAlwaysTerminating(): array
324324
true,
325325
],
326326
[
327-
'while ($string !== null) { $string = null; try { return true; } catch (\Exception $e) { doFoo(); } }',
327+
'while ($string !== null) { $string = null; try { maybeThrow(); return true; } catch (\Exception $e) { doFoo(); } }',
328328
false,
329329
],
330330
[
331-
'while ($string !== null) { $string = null; try { return true; } catch (\Exception $e) { doFoo(); } }',
331+
'while ($string !== null) { $string = null; try { maybeThrow(); return true; } catch (\Exception $e) { doFoo(); } }',
332332
false,
333333
],
334334
[
335-
'try { return true; } catch (\Exception $e) { doFoo(); }',
335+
'try { maybeThrow(); return true; } catch (\Exception $e) { doFoo(); }',
336336
false,
337337
],
338338
[

‎tests/PHPStan/Analyser/data/finally-with-early-termination.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
try {
66
$integerOrString = 1;
77
$fooOrBarException = null;
8+
maybeThrows();
89
return 1;
910
} catch (FooException $e) {
1011
$integerOrString = 1;

‎tests/PHPStan/Analyser/data/finally.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ function () {
1616
try {
1717
$integerOrString = 1;
1818
$fooOrBarException = null;
19+
maybeThrows();
1920
} catch (FooException $e) {
2021
$integerOrString = 1;
2122
$fooOrBarException = $e;

‎tests/PHPStan/Analyser/data/if.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ function () {
3838
$exceptionFromTry = null;
3939
try {
4040
$inTry = 1;
41-
$inTryNotInCatch = 1;
4241
$fooObjectFromTryCatch = new InTryCatchFoo();
42+
$inTryNotInCatch = 1;
4343
$mixedVarFromTryCatch = 1;
4444
$nullableIntegerFromTryCatch = 1;
4545
$anotherNullableIntegerFromTryCatch = null;
@@ -59,7 +59,7 @@ function () {
5959

6060
$exceptionFromTryCatch = null;
6161
try {
62-
62+
maybeThrows();
6363
} catch (\SomeConcreteException $exceptionFromTryCatch) {
6464
return;
6565
} catch (\AnotherException $exceptionFromTryCatch) {

‎tests/PHPStan/Analyser/data/throw-points/try-catch.php‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
<?php
2+
3+
namespace ThrowPoints\TryCatch;
4+
5+
use PHPStan\TrinaryLogic;
6+
use function PHPStan\Analyser\assertType;
7+
use function PHPStan\Analyser\assertVariableCertainty;
8+
9+
class MyInvalidArgumentException extends \InvalidArgumentException
10+
{
11+
12+
}
13+
14+
class MyRuntimeException extends \RuntimeException
15+
{
16+
17+
}
18+
19+
class Foo
20+
{
21+
22+
/** @throws void */
23+
public static function createInvalidArgumentException(): \InvalidArgumentException
24+
{
25+
26+
}
27+
28+
/** @throws void */
29+
public static function createMyInvalidArgumentException(): MyInvalidArgumentException
30+
{
31+
32+
}
33+
34+
/** @throws void */
35+
public static function createRuntimeException(): \RuntimeException
36+
{
37+
38+
}
39+
40+
/** @throws void */
41+
public static function createMyRuntimeException(): MyRuntimeException
42+
{
43+
44+
}
45+
46+
/** @throws void */
47+
public static function myRand(): int
48+
{
49+
50+
}
51+
52+
}
53+
54+
function (): void {
55+
try {
56+
if (Foo::myRand() === 0) {
57+
$foo = 1;
58+
throw Foo::createInvalidArgumentException();
59+
}
60+
if (Foo::myRand() === 1) {
61+
$foo = 2;
62+
throw Foo::createMyInvalidArgumentException();
63+
}
64+
65+
if (Foo::myRand() === 2) {
66+
$baz = 1;
67+
throw Foo::createRuntimeException();
68+
}
69+
if (Foo::myRand() === 3) {
70+
$baz = 2;
71+
throw Foo::createMyRuntimeException();
72+
}
73+
74+
$bar = 1;
75+
} catch (\InvalidArgumentException $e) {
76+
assertVariableCertainty(TrinaryLogic::createYes(), $foo);
77+
assertType('1|2', $foo);
78+
79+
assertVariableCertainty(TrinaryLogic::createNo(), $bar);
80+
assertVariableCertainty(TrinaryLogic::createNo(), $baz);
81+
} catch (\RuntimeException $e) {
82+
assertVariableCertainty(TrinaryLogic::createNo(), $foo);
83+
assertVariableCertainty(TrinaryLogic::createNo(), $bar);
84+
assertVariableCertainty(TrinaryLogic::createYes(), $baz);
85+
assertType('1|2', $baz);
86+
} catch (\Throwable $e) {
87+
assertVariableCertainty(TrinaryLogic::createNo(), $foo);
88+
assertVariableCertainty(TrinaryLogic::createYes(), $bar);
89+
assertVariableCertainty(TrinaryLogic::createNo(), $baz);
90+
} finally {
91+
assertVariableCertainty(TrinaryLogic::createMaybe(), $foo);
92+
assertType('1|2', $foo);
93+
94+
assertVariableCertainty(TrinaryLogic::createMaybe(), $bar);
95+
assertType('1', $bar);
96+
97+
assertVariableCertainty(TrinaryLogic::createMaybe(), $baz);
98+
assertType('1|2', $baz);
99+
}
100+
};

‎tests/PHPStan/Rules/Missing/data/missing-return.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ public function doBar(): int
232232
public function doBaz(): int
233233
{
234234
try {
235-
return 1;
235+
maybeThrow(); return 1;
236236
} catch (\Exception $e) {
237237
return 1;
238238
} catch (\Throwable $e) {

‎tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,6 @@ public function testDefinedVariables(): void
159159
'Variable $forJ might not be defined.',
160160
251,
161161
],
162-
[
163-
'Variable $variableDefinedInTry might not be defined.',
164-
260,
165-
],
166162
[
167163
'Variable $variableAvailableInAllCatches might not be defined.',
168164
266,

0 commit comments

Comments
 (0)