Skip to content

Commit 6097052

Browse files
VincentLangletphpstan-bot
authored andcommitted
Fix phpstan/phpstan#14318: False positive variable might not be defined in catch block
- Moved method call throw point creation to after processArgs in MethodCallHandler - Applied the same fix to StaticCallHandler for consistency - The throw point scope now includes variables assigned in function arguments - New regression test in tests/PHPStan/Rules/Variables/data/bug-14318.php
1 parent 0e5233a commit 6097052

4 files changed

Lines changed: 83 additions & 9 deletions

File tree

src/Analyser/ExprHandler/MethodCallHandler.php

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,6 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
110110
$methodReflection->getNamedArgumentsVariants(),
111111
);
112112

113-
$methodThrowPoint = $this->getMethodThrowPoint($methodReflection, $parametersAcceptor, $expr, $scope);
114-
if ($methodThrowPoint !== null) {
115-
$throwPoints[] = $methodThrowPoint;
116-
}
117113
}
118114
} else {
119115
$methodNameResult = $nodeScopeResolver->processExprNode($stmt, $expr->name, $scope, $storage, $nodeCallback, $context->enterDeep());
@@ -157,6 +153,13 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
157153
$scope = $argsResult->getScope();
158154

159155
if ($methodReflection !== null) {
156+
if ($parametersAcceptor !== null) {
157+
$methodThrowPoint = $this->getMethodThrowPoint($methodReflection, $parametersAcceptor, $expr, $scope);
158+
if ($methodThrowPoint !== null) {
159+
$throwPoints[] = $methodThrowPoint;
160+
}
161+
}
162+
160163
if ($methodReflection->getName() === '__construct' || $methodReflection->hasSideEffects()->yes()) {
161164
$nodeScopeResolver->callNodeCallback($nodeCallback, new InvalidateExprNode($normalizedExpr->var), $scope, $storage);
162165
$scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass());

src/Analyser/ExprHandler/StaticCallHandler.php

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,6 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
103103
$methodReflection->getNamedArgumentsVariants(),
104104
);
105105

106-
$methodThrowPoint = $this->getStaticMethodThrowPoint($methodReflection, $parametersAcceptor, $expr, $scope);
107-
if ($methodThrowPoint !== null) {
108-
$throwPoints[] = $methodThrowPoint;
109-
}
110-
111106
$declaringClass = $methodReflection->getDeclaringClass();
112107
if (
113108
$declaringClass->getName() === 'Closure'
@@ -203,6 +198,13 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
203198
$scope = $argsResult->getScope();
204199
$scopeFunction = $scope->getFunction();
205200

201+
if ($methodReflection !== null && $parametersAcceptor !== null) {
202+
$methodThrowPoint = $this->getStaticMethodThrowPoint($methodReflection, $parametersAcceptor, $expr, $scope);
203+
if ($methodThrowPoint !== null) {
204+
$throwPoints[] = $methodThrowPoint;
205+
}
206+
}
207+
206208
if (
207209
$methodReflection !== null
208210
&& (

tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,16 @@ public function testBug9349(): void
14221422
]);
14231423
}
14241424

1425+
public function testBug14318(): void
1426+
{
1427+
$this->cliArgumentsVariablesRegistered = true;
1428+
$this->polluteScopeWithLoopInitialAssignments = false;
1429+
$this->checkMaybeUndefinedVariables = true;
1430+
$this->polluteScopeWithAlwaysIterableForeach = true;
1431+
1432+
$this->analyse([__DIR__ . '/data/bug-14318.php'], []);
1433+
}
1434+
14251435
#[RequiresPhp('>= 8.0')]
14261436
public function testBug14274(): void
14271437
{
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug14318;
4+
5+
class HelloWorld5
6+
{
7+
public function test5(): void
8+
{
9+
global $pdo;
10+
11+
try {
12+
$this->maybeThrows5($sql = "SELECT * FROM foo");
13+
$rs = $pdo->query($sql);
14+
if ($result = $rs->fetch(\PDO::FETCH_ASSOC)) {
15+
// do something
16+
}
17+
} catch (\PDOException $e) {
18+
var_dump($sql);
19+
}
20+
}
21+
22+
/**
23+
* @throws \RuntimeException
24+
*/
25+
public function maybeThrows5(string $s): void
26+
{
27+
if (random_int(0, 1) === 1) {
28+
throw new \RuntimeException();
29+
}
30+
}
31+
}
32+
33+
class HelloWorld6
34+
{
35+
public function test6(): void
36+
{
37+
global $pdo;
38+
39+
try {
40+
$this->maybeThrows6(strlen($sql = "SELECT * FROM foo"));
41+
$rs = $pdo->query($sql);
42+
if ($result = $rs->fetch(\PDO::FETCH_ASSOC)) {
43+
// do something
44+
}
45+
} catch (\PDOException $e) {
46+
var_dump($sql);
47+
}
48+
}
49+
50+
/**
51+
* @throws \RuntimeException
52+
*/
53+
public function maybeThrows6(int $s): void
54+
{
55+
if (random_int(0, 1) === 1) {
56+
throw new \RuntimeException();
57+
}
58+
}
59+
}

0 commit comments

Comments
 (0)