Skip to content

Commit f75b4f6

Browse files
ondrejmirtesondrejmirtes
committed
Walk catch blocks that no throw point reaches
Amp-Thread-ID: https://ampcode.com/threads/T-01a179f2-995f-7745-8d8c-4c174ba5c0ce
1 parent 9b24ee5 commit f75b4f6

8 files changed

Lines changed: 248 additions & 2 deletions

File tree

‎src/Analyser/StmtHandler/TryCatchHandler.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,17 @@ public function processStmt(
215215

216216
if (count($matchingThrowPoints) === 0) {
217217
$catchFlows[] = [$originalCatchType, $this->statementsHandler->getVariableMentionFlow($catchNode)];
218+
219+
// the catch block is not reachable, but rules still need to see its statements;
220+
// it is entered from where the try block can throw anything, and the result
221+
// of the walk does not flow anywhere
222+
$deadCatchScope = null;
223+
foreach ($branchScopeResult->getThrowPoints() as $throwPoint) {
224+
$deadCatchScope = $deadCatchScope === null ? $throwPoint->getScope() : $deadCatchScope->mergeWith($throwPoint->getScope());
225+
}
226+
$deadCatchScope = ($deadCatchScope ?? $scope->mergeWith($branchScope))->enterCatchType($catchType instanceof NeverType ? $originalCatchType : $catchType, $catchNode->var !== null && is_string($catchNode->var->name) ? $catchNode->var->name : null);
227+
$deadCatchResult = $nodeScopeResolver->processStmtNodesInternal($catchNode, $catchNode->stmts, $deadCatchScope, $storage, $nodeCallback, $context);
228+
$dependencies[] = $deadCatchResult->getDependencies();
218229
continue;
219230
}
220231

‎tests/PHPStan/Analyser/nsrt/bug-14397.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ function test2(mixed $mixed): void
2121
filter_var($mixed, FILTER_VALIDATE_INT, FILTER_FLAG_GLOBAL_RANGE);
2222
$foo = 1;
2323
} catch (\Filter\FilterFailedException $e) {
24-
assertVariableCertainty(TrinaryLogic::createYes(), $foo);
24+
assertVariableCertainty(TrinaryLogic::createMaybe(), $foo);
2525
}
2626

2727
assertType('int|false', filter_var($mixed, FILTER_VALIDATE_INT, FILTER_FLAG_GLOBAL_RANGE));

‎tests/PHPStan/Analyser/nsrt/dead-catch.php‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
<?php
2+
3+
namespace DeadCatchTypes;
4+
5+
use function PHPStan\Testing\assertNativeType;
6+
use function PHPStan\Testing\assertType;
7+
8+
class FooException extends \Exception
9+
{
10+
11+
}
12+
13+
class BarException extends \Exception
14+
{
15+
16+
}
17+
18+
interface MarkerException
19+
{
20+
21+
}
22+
23+
/** @throws \RuntimeException */
24+
function throwsRuntime(): void
25+
{
26+
}
27+
28+
function noThrowPoints(): void
29+
{
30+
try {
31+
$a = 1;
32+
} catch (\InvalidArgumentException $e) {
33+
assertType('InvalidArgumentException', $e);
34+
assertNativeType('InvalidArgumentException', $e);
35+
}
36+
}
37+
38+
function union(): void
39+
{
40+
try {
41+
$a = 1;
42+
} catch (FooException | BarException $e) {
43+
assertType('DeadCatchTypes\BarException|DeadCatchTypes\FooException', $e);
44+
assertNativeType('DeadCatchTypes\BarException|DeadCatchTypes\FooException', $e);
45+
}
46+
}
47+
48+
function interfaceCatch(): void
49+
{
50+
try {
51+
$a = 1;
52+
} catch (MarkerException $e) {
53+
assertType('DeadCatchTypes\MarkerException&Throwable', $e);
54+
assertNativeType('DeadCatchTypes\MarkerException&Throwable', $e);
55+
}
56+
}
57+
58+
function afterEarlierCatch(): void
59+
{
60+
try {
61+
$a = 1;
62+
} catch (FooException $e) {
63+
assertType('DeadCatchTypes\FooException', $e);
64+
} catch (FooException | BarException $e) {
65+
assertType('DeadCatchTypes\BarException', $e);
66+
assertNativeType('DeadCatchTypes\BarException', $e);
67+
}
68+
}
69+
70+
function afterThrowableCatch(): void
71+
{
72+
try {
73+
throwsRuntime();
74+
} catch (\Throwable $e) {
75+
assertType('Throwable', $e);
76+
} catch (\InvalidArgumentException $e) {
77+
assertType('InvalidArgumentException', $e);
78+
assertNativeType('InvalidArgumentException', $e);
79+
}
80+
}
81+
82+
function scopeOfThrowPoints(): void
83+
{
84+
try {
85+
$a = 1;
86+
throwsRuntime();
87+
$a = 2;
88+
throwsRuntime();
89+
$a = 3;
90+
} catch (\InvalidArgumentException $e) {
91+
assertType('1|2', $a);
92+
assertNativeType('1|2', $a);
93+
assertType('InvalidArgumentException', $e);
94+
}
95+
}

‎tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4595,4 +4595,25 @@ public function testClassConstantWildcardArguments(): void
45954595
]);
45964596
}
45974597

4598+
public function testDeadCatch(): void
4599+
{
4600+
$this->checkThisOnly = false;
4601+
$this->checkNullables = true;
4602+
$this->checkUnionTypes = true;
4603+
$this->analyse([__DIR__ . '/data/dead-catch.php'], [
4604+
[
4605+
'Call to an undefined method InvalidArgumentException::nonExistentMethod().',
4606+
14,
4607+
],
4608+
[
4609+
'Call to an undefined method CallMethodsDeadCatch\\Foo::nonExistentMethod().',
4610+
15,
4611+
],
4612+
[
4613+
'Call to an undefined method InvalidArgumentException::nonExistentMethod().',
4614+
26,
4615+
],
4616+
]);
4617+
}
4618+
45984619
}

‎tests/PHPStan/Rules/Methods/data/dead-catch.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
namespace CallMethodsDeadCatch;
4+
5+
class Foo
6+
{
7+
8+
public function doFoo(): void
9+
{
10+
try {
11+
$a = 1;
12+
} catch (\InvalidArgumentException $e) {
13+
$e->getMessage();
14+
$e->nonExistentMethod();
15+
$this->nonExistentMethod();
16+
}
17+
}
18+
19+
public function doBar(): void
20+
{
21+
try {
22+
$this->doFoo();
23+
} catch (\Throwable $e) {
24+
25+
} catch (\InvalidArgumentException $e) {
26+
$e->nonExistentMethod();
27+
}
28+
}
29+
30+
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,4 +527,16 @@ public function testVariableVariableName(): void
527527
]);
528528
}
529529

530+
public function testDeadCatch(): void
531+
{
532+
$this->cliArgumentsVariablesRegistered = true;
533+
$this->checkMaybeUndefinedVariables = true;
534+
$this->analyse([__DIR__ . '/data/dead-catch-defined-variable.php'], [
535+
[
536+
'Undefined variable: $undefined',
537+
22,
538+
],
539+
]);
540+
}
541+
530542
}

‎tests/PHPStan/Rules/Variables/data/dead-catch-defined-variable.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
3+
namespace DeadCatchDefinedVariable;
4+
5+
class Foo
6+
{
7+
8+
/** @throws \RuntimeException */
9+
private function process(string $name): void
10+
{
11+
}
12+
13+
/** @param list<string> $names */
14+
public function doFoo(array $names): void
15+
{
16+
try {
17+
foreach ($names as $name) {
18+
$this->process($name);
19+
}
20+
} catch (\InvalidArgumentException $e) {
21+
echo $name;
22+
echo $undefined;
23+
}
24+
}
25+
26+
}

‎turbo-ext/src/TryCatchHandler.cpp‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,9 +441,60 @@ class TryCatchHandler
441441
zv::Val mentionFlow = pt_statements_handler_get_variable_mention_flow(statementsHandler, catchNode);
442442
if (UNEXPECTED(mentionFlow.isUndef())) return zv::Val();
443443
zv::Arr catchFlow = zv::Arr::create(2);
444-
catchFlow.push(std::move(originalCatchType));
444+
catchFlow.push(zv::Ref(originalCatchType.raw()));
445445
catchFlow.push(std::move(mentionFlow));
446446
catchFlows.push(zv::Val(std::move(catchFlow)));
447+
448+
// the catch block is not reachable, but rules still need to see its statements;
449+
// it is entered from where the try block can throw anything, and the result
450+
// of the walk does not flow anywhere
451+
zend_string *deadVariableName = NULL;
452+
{
453+
zval *catchVar = ptsh::readNodeProperty(pt_tch_catch_var_site, catchNode, PT_LC("var"));
454+
if (UNEXPECTED(catchVar == NULL)) return zv::Val();
455+
if (Z_TYPE_P(catchVar) != IS_NULL) {
456+
zval *name = ptsh::readNodeProperty(pt_tch_variable_name_site, catchVar, PT_LC("name"));
457+
if (UNEXPECTED(name == NULL)) return zv::Val();
458+
if (Z_TYPE_P(name) == IS_STRING) {
459+
deadVariableName = Z_STR_P(name);
460+
}
461+
}
462+
}
463+
zv::Val deadVariableNameHold = deadVariableName != NULL ? zv::Val::string(deadVariableName) : zv::Val::null();
464+
zv::Val mergedScope = zv::Val::null();
465+
{
466+
zv::Val hold;
467+
zval *branchThrowPoints = pt_internal_statement_result_throw_points(branch, hold);
468+
if (UNEXPECTED(branchThrowPoints == NULL)) return zv::Val();
469+
zv::Val branchThrowPointsHold = zv::Val::copyOf(zv::Ref(branchThrowPoints));
470+
if (Z_TYPE_P(branchThrowPointsHold.raw()) == IS_ARRAY) {
471+
for (auto throwEntry : zv::ArrRef(branchThrowPointsHold.raw())) {
472+
zv::Val scopeHold;
473+
zval *throwScope = pt_internal_throw_point_scope(throwEntry.value().deref().raw(), scopeHold);
474+
if (UNEXPECTED(throwScope == NULL)) return zv::Val();
475+
if (mergedScope.isNull()) {
476+
mergedScope = zv::Val::copyOf(zv::Ref(throwScope));
477+
continue;
478+
}
479+
zv::Val merged = pt_mutating_scope_merge_with(Z_OBJ_P(mergedScope.raw()), throwScope);
480+
if (UNEXPECTED(merged.isUndef())) return zv::Val();
481+
mergedScope = std::move(merged);
482+
}
483+
}
484+
}
485+
if (mergedScope.isNull()) {
486+
mergedScope = pt_mutating_scope_merge_with(Z_OBJ_P(scope), branchScope.raw());
487+
if (UNEXPECTED(mergedScope.isUndef())) return zv::Val();
488+
}
489+
zval *deadCatchType = isNeverType(catchType.raw()) ? originalCatchType.raw() : catchType.raw();
490+
zv::Val deadCatchScope = pt_mutating_scope_enter_catch_type(Z_OBJ_P(mergedScope.raw()), deadCatchType, deadVariableName != NULL ? Z_STR_P(deadVariableNameHold.raw()) : NULL);
491+
if (UNEXPECTED(deadCatchScope.isUndef())) return zv::Val();
492+
zval *deadCatchStmts = ptsh::readNodeProperty(pt_tch_catch_stmts_site, catchNode, PT_LC("stmts"));
493+
if (UNEXPECTED(deadCatchStmts == NULL)) return zv::Val();
494+
zv::Val deadCatchStmtsHold = zv::Val::copyOf(zv::Ref(deadCatchStmts));
495+
zv::Val deadCatchResult = pt_node_scope_resolver_process_stmt_nodes_internal(nodeScopeResolver, catchNode, deadCatchStmtsHold.raw(), deadCatchScope.raw(), storage, nodeCallback, context);
496+
if (UNEXPECTED(deadCatchResult.isUndef())) return zv::Val();
497+
if (UNEXPECTED(!pushStatementDependencies(dependencies, deadCatchResult.raw()))) return zv::Val();
447498
continue;
448499
}
449500

0 commit comments

Comments
 (0)