From 609705252c0a2fde8673c78cef5b128fb465be4a Mon Sep 17 00:00:00 2001 From: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Date: Tue, 17 Mar 2026 17:25:01 +0000 Subject: [PATCH 1/8] 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 --- .../ExprHandler/MethodCallHandler.php | 11 ++-- .../ExprHandler/StaticCallHandler.php | 12 ++-- .../Variables/DefinedVariableRuleTest.php | 10 ++++ .../Rules/Variables/data/bug-14318.php | 59 +++++++++++++++++++ 4 files changed, 83 insertions(+), 9 deletions(-) create mode 100644 tests/PHPStan/Rules/Variables/data/bug-14318.php diff --git a/src/Analyser/ExprHandler/MethodCallHandler.php b/src/Analyser/ExprHandler/MethodCallHandler.php index bf40faf5468..35c5455da5c 100644 --- a/src/Analyser/ExprHandler/MethodCallHandler.php +++ b/src/Analyser/ExprHandler/MethodCallHandler.php @@ -110,10 +110,6 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $methodReflection->getNamedArgumentsVariants(), ); - $methodThrowPoint = $this->getMethodThrowPoint($methodReflection, $parametersAcceptor, $expr, $scope); - if ($methodThrowPoint !== null) { - $throwPoints[] = $methodThrowPoint; - } } } else { $methodNameResult = $nodeScopeResolver->processExprNode($stmt, $expr->name, $scope, $storage, $nodeCallback, $context->enterDeep()); @@ -157,6 +153,13 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $scope = $argsResult->getScope(); if ($methodReflection !== null) { + if ($parametersAcceptor !== null) { + $methodThrowPoint = $this->getMethodThrowPoint($methodReflection, $parametersAcceptor, $expr, $scope); + if ($methodThrowPoint !== null) { + $throwPoints[] = $methodThrowPoint; + } + } + if ($methodReflection->getName() === '__construct' || $methodReflection->hasSideEffects()->yes()) { $nodeScopeResolver->callNodeCallback($nodeCallback, new InvalidateExprNode($normalizedExpr->var), $scope, $storage); $scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass()); diff --git a/src/Analyser/ExprHandler/StaticCallHandler.php b/src/Analyser/ExprHandler/StaticCallHandler.php index 1438af86fad..16eceaefbc6 100644 --- a/src/Analyser/ExprHandler/StaticCallHandler.php +++ b/src/Analyser/ExprHandler/StaticCallHandler.php @@ -103,11 +103,6 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $methodReflection->getNamedArgumentsVariants(), ); - $methodThrowPoint = $this->getStaticMethodThrowPoint($methodReflection, $parametersAcceptor, $expr, $scope); - if ($methodThrowPoint !== null) { - $throwPoints[] = $methodThrowPoint; - } - $declaringClass = $methodReflection->getDeclaringClass(); if ( $declaringClass->getName() === 'Closure' @@ -203,6 +198,13 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $scope = $argsResult->getScope(); $scopeFunction = $scope->getFunction(); + if ($methodReflection !== null && $parametersAcceptor !== null) { + $methodThrowPoint = $this->getStaticMethodThrowPoint($methodReflection, $parametersAcceptor, $expr, $scope); + if ($methodThrowPoint !== null) { + $throwPoints[] = $methodThrowPoint; + } + } + if ( $methodReflection !== null && ( diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index 0bf14086718..b7b547ca165 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1422,6 +1422,16 @@ public function testBug9349(): void ]); } + public function testBug14318(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = false; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + + $this->analyse([__DIR__ . '/data/bug-14318.php'], []); + } + #[RequiresPhp('>= 8.0')] public function testBug14274(): void { diff --git a/tests/PHPStan/Rules/Variables/data/bug-14318.php b/tests/PHPStan/Rules/Variables/data/bug-14318.php new file mode 100644 index 00000000000..c48db6da7de --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-14318.php @@ -0,0 +1,59 @@ +maybeThrows5($sql = "SELECT * FROM foo"); + $rs = $pdo->query($sql); + if ($result = $rs->fetch(\PDO::FETCH_ASSOC)) { + // do something + } + } catch (\PDOException $e) { + var_dump($sql); + } + } + + /** + * @throws \RuntimeException + */ + public function maybeThrows5(string $s): void + { + if (random_int(0, 1) === 1) { + throw new \RuntimeException(); + } + } +} + +class HelloWorld6 +{ + public function test6(): void + { + global $pdo; + + try { + $this->maybeThrows6(strlen($sql = "SELECT * FROM foo")); + $rs = $pdo->query($sql); + if ($result = $rs->fetch(\PDO::FETCH_ASSOC)) { + // do something + } + } catch (\PDOException $e) { + var_dump($sql); + } + } + + /** + * @throws \RuntimeException + */ + public function maybeThrows6(int $s): void + { + if (random_int(0, 1) === 1) { + throw new \RuntimeException(); + } + } +} From fe05863391b4fcda1e89b35cfbedf200597dc413 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Wed, 18 Mar 2026 06:05:05 +0000 Subject: [PATCH 2/8] Add regression tests for phpstan/phpstan#11284 and phpstan/phpstan#7806 Both issues involve variables in try/catch blocks not being properly tracked - #11284 for by-reference parameters and #7806 for preg_match matches. These are also fixed by the throw point scope fix in #5240. Co-Authored-By: Claude Opus 4.6 --- .../Variables/DefinedVariableRuleTest.php | 20 ++++++++++++ .../Rules/Variables/data/bug-11284.php | 32 +++++++++++++++++++ .../PHPStan/Rules/Variables/data/bug-7806.php | 19 +++++++++++ 3 files changed, 71 insertions(+) create mode 100644 tests/PHPStan/Rules/Variables/data/bug-11284.php create mode 100644 tests/PHPStan/Rules/Variables/data/bug-7806.php diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index b7b547ca165..33f9f709a15 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1432,6 +1432,26 @@ public function testBug14318(): void $this->analyse([__DIR__ . '/data/bug-14318.php'], []); } + public function testBug11284(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = false; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + + $this->analyse([__DIR__ . '/data/bug-11284.php'], []); + } + + public function testBug7806(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = false; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + + $this->analyse([__DIR__ . '/data/bug-7806.php'], []); + } + #[RequiresPhp('>= 8.0')] public function testBug14274(): void { diff --git a/tests/PHPStan/Rules/Variables/data/bug-11284.php b/tests/PHPStan/Rules/Variables/data/bug-11284.php new file mode 100644 index 00000000000..879eb1e5983 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-11284.php @@ -0,0 +1,32 @@ + $err + * @throws \RuntimeException + */ + public function maybeThrows(array &$err): void + { + $err[] = 'error'; + if (random_int(0, 1) === 1) { + throw new \RuntimeException(); + } + } + + public function test(): void + { + $err = []; + try { + $this->maybeThrows($err); + } catch (\RuntimeException $e) { + if (!empty($err)) { + echo implode(', ', $err); + } + } + } + +} diff --git a/tests/PHPStan/Rules/Variables/data/bug-7806.php b/tests/PHPStan/Rules/Variables/data/bug-7806.php new file mode 100644 index 00000000000..141b437ce9e --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-7806.php @@ -0,0 +1,19 @@ + Date: Wed, 18 Mar 2026 07:26:13 +0100 Subject: [PATCH 3/8] fix regression tests --- .../Variables/DefinedVariableRuleTest.php | 20 -------------- .../PHPStan/Rules/Variables/EmptyRuleTest.php | 14 ++++++++++ .../PHPStan/Rules/Variables/data/bug-7806.php | 26 +++++++++++++------ 3 files changed, 32 insertions(+), 28 deletions(-) diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index 33f9f709a15..b7b547ca165 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1432,26 +1432,6 @@ public function testBug14318(): void $this->analyse([__DIR__ . '/data/bug-14318.php'], []); } - public function testBug11284(): void - { - $this->cliArgumentsVariablesRegistered = true; - $this->polluteScopeWithLoopInitialAssignments = false; - $this->checkMaybeUndefinedVariables = true; - $this->polluteScopeWithAlwaysIterableForeach = true; - - $this->analyse([__DIR__ . '/data/bug-11284.php'], []); - } - - public function testBug7806(): void - { - $this->cliArgumentsVariablesRegistered = true; - $this->polluteScopeWithLoopInitialAssignments = false; - $this->checkMaybeUndefinedVariables = true; - $this->polluteScopeWithAlwaysIterableForeach = true; - - $this->analyse([__DIR__ . '/data/bug-7806.php'], []); - } - #[RequiresPhp('>= 8.0')] public function testBug14274(): void { diff --git a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php index 1af3618c5b8..873c89fb869 100644 --- a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php +++ b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php @@ -209,6 +209,20 @@ public function testBug10367(): void $this->analyse([__DIR__ . '/data/bug-10367.php'], []); } + public function testBug11284(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/bug-11284.php'], []); + } + + public function testBug7806(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/bug-7806.php'], []); + } + #[RequiresPhp('>= 8.0')] public function testIssetAfterRememberedConstructor(): void { diff --git a/tests/PHPStan/Rules/Variables/data/bug-7806.php b/tests/PHPStan/Rules/Variables/data/bug-7806.php index 141b437ce9e..e9bf61aeab7 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-7806.php +++ b/tests/PHPStan/Rules/Variables/data/bug-7806.php @@ -2,18 +2,28 @@ namespace Bug7806; -class HelloWorld -{ +class Test { + /** + * @param array|null $reasons + * @throws \Exception + */ + function check(array &$reasons = null): void { + $fileName = time() % 2 ? "abc":null; + if (!$fileName) { + $reasons[] = sprintf("Dependency check fail"); + throw new \Exception("check failed"); + } + } - public function test(): void - { + function test():void { try { - preg_match('/pattern/', 'subject', $reasons); - } catch (\Throwable $e) { + $this->check($reasons); + printf("ok\n"); + } catch (\Exception $e) { if (!empty($reasons)) { - echo implode(', ', $reasons); + $e = new \Exception("Dependency check failed: " . implode(', ', $reasons), 0, $e); } + throw new \Exception("Failed", 0, $e); } } - } From 770316b42305641d3d393e04a56a939dbaa4f83a Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Wed, 18 Mar 2026 06:31:06 +0000 Subject: [PATCH 4/8] Add static method call test cases for bug-14318 Cover the StaticCallHandler code path with regression tests using static method calls (self::maybeThrows()) alongside the existing instance method call tests. Co-Authored-By: Claude Opus 4.6 --- .../Rules/Variables/data/bug-14318.php | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/PHPStan/Rules/Variables/data/bug-14318.php b/tests/PHPStan/Rules/Variables/data/bug-14318.php index c48db6da7de..c8a1896e66c 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-14318.php +++ b/tests/PHPStan/Rules/Variables/data/bug-14318.php @@ -57,3 +57,59 @@ public function maybeThrows6(int $s): void } } } + +class HelloWorld7 +{ + public function test7(): void + { + global $pdo; + + try { + self::maybeThrows7($sql = "SELECT * FROM foo"); + $rs = $pdo->query($sql); + if ($result = $rs->fetch(\PDO::FETCH_ASSOC)) { + // do something + } + } catch (\PDOException $e) { + var_dump($sql); + } + } + + /** + * @throws \RuntimeException + */ + public static function maybeThrows7(string $s): void + { + if (random_int(0, 1) === 1) { + throw new \RuntimeException(); + } + } +} + +class HelloWorld8 +{ + public function test8(): void + { + global $pdo; + + try { + self::maybeThrows8(strlen($sql = "SELECT * FROM foo")); + $rs = $pdo->query($sql); + if ($result = $rs->fetch(\PDO::FETCH_ASSOC)) { + // do something + } + } catch (\PDOException $e) { + var_dump($sql); + } + } + + /** + * @throws \RuntimeException + */ + public static function maybeThrows8(int $s): void + { + if (random_int(0, 1) === 1) { + throw new \RuntimeException(); + } + } +} From 5f4200ae61eca0216da22dec1ed23d9b67b69c17 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Wed, 18 Mar 2026 07:02:06 +0000 Subject: [PATCH 5/8] Add function call test cases for bug-14318 Adds regression tests for regular function calls (FuncCallHandler) with variable assignment in arguments, proving this handler already had the correct throw point order. Co-Authored-By: Claude Opus 4.6 --- .../Rules/Variables/data/bug-14318.php | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/PHPStan/Rules/Variables/data/bug-14318.php b/tests/PHPStan/Rules/Variables/data/bug-14318.php index c8a1896e66c..edeb432cc7a 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-14318.php +++ b/tests/PHPStan/Rules/Variables/data/bug-14318.php @@ -113,3 +113,59 @@ public static function maybeThrows8(int $s): void } } } + +/** + * @throws \RuntimeException + */ +function maybeThrows9(string $s): void +{ + if (random_int(0, 1) === 1) { + throw new \RuntimeException(); + } +} + +/** + * @throws \RuntimeException + */ +function maybeThrows10(int $s): void +{ + if (random_int(0, 1) === 1) { + throw new \RuntimeException(); + } +} + +class HelloWorld9 +{ + public function test9(): void + { + global $pdo; + + try { + maybeThrows9($sql = "SELECT * FROM foo"); + $rs = $pdo->query($sql); + if ($result = $rs->fetch(\PDO::FETCH_ASSOC)) { + // do something + } + } catch (\PDOException $e) { + var_dump($sql); + } + } +} + +class HelloWorld10 +{ + public function test10(): void + { + global $pdo; + + try { + maybeThrows10(strlen($sql = "SELECT * FROM foo")); + $rs = $pdo->query($sql); + if ($result = $rs->fetch(\PDO::FETCH_ASSOC)) { + // do something + } + } catch (\PDOException $e) { + var_dump($sql); + } + } +} From f99e3b397c3d4e1dd43e1aef73f52730a38439e3 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 18 Mar 2026 08:05:05 +0100 Subject: [PATCH 6/8] simplify test --- .../Rules/Variables/data/bug-14318.php | 62 +++++-------------- 1 file changed, 15 insertions(+), 47 deletions(-) diff --git a/tests/PHPStan/Rules/Variables/data/bug-14318.php b/tests/PHPStan/Rules/Variables/data/bug-14318.php index edeb432cc7a..767b77e8f38 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-14318.php +++ b/tests/PHPStan/Rules/Variables/data/bug-14318.php @@ -115,57 +115,25 @@ public static function maybeThrows8(int $s): void } /** - * @throws \RuntimeException + * @param array|null $reasons + * @throws \Exception */ -function maybeThrows9(string $s): void -{ - if (random_int(0, 1) === 1) { - throw new \RuntimeException(); +function check1(array &$reasons = null): void { + $fileName = time() % 2 ? "abc":null; + if (!$fileName) { + $reasons[] = sprintf("Dependency check fail"); + throw new \Exception("check failed"); } } -/** - * @throws \RuntimeException - */ -function maybeThrows10(int $s): void -{ - if (random_int(0, 1) === 1) { - throw new \RuntimeException(); - } -} - -class HelloWorld9 -{ - public function test9(): void - { - global $pdo; - - try { - maybeThrows9($sql = "SELECT * FROM foo"); - $rs = $pdo->query($sql); - if ($result = $rs->fetch(\PDO::FETCH_ASSOC)) { - // do something - } - } catch (\PDOException $e) { - var_dump($sql); - } - } -} - -class HelloWorld10 -{ - public function test10(): void - { - global $pdo; - - try { - maybeThrows10(strlen($sql = "SELECT * FROM foo")); - $rs = $pdo->query($sql); - if ($result = $rs->fetch(\PDO::FETCH_ASSOC)) { - // do something - } - } catch (\PDOException $e) { - var_dump($sql); +function test1():void { + try { + check1($reasons); + printf("ok\n"); + } catch (\Exception $e) { + if (!empty($reasons)) { + $e = new \Exception("Dependency check failed: " . implode(', ', $reasons), 0, $e); } + throw new \Exception("Failed", 0, $e); } } From b15cf90e93037f09e2b25aef2b2dc43c4cb6d6de Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 18 Mar 2026 08:06:56 +0100 Subject: [PATCH 7/8] move test --- .../Rules/Variables/data/bug-14318.php | 24 ----------------- .../PHPStan/Rules/Variables/data/bug-7806.php | 27 ++++++++++++++++++- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/tests/PHPStan/Rules/Variables/data/bug-14318.php b/tests/PHPStan/Rules/Variables/data/bug-14318.php index 767b77e8f38..c8a1896e66c 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-14318.php +++ b/tests/PHPStan/Rules/Variables/data/bug-14318.php @@ -113,27 +113,3 @@ public static function maybeThrows8(int $s): void } } } - -/** - * @param array|null $reasons - * @throws \Exception - */ -function check1(array &$reasons = null): void { - $fileName = time() % 2 ? "abc":null; - if (!$fileName) { - $reasons[] = sprintf("Dependency check fail"); - throw new \Exception("check failed"); - } -} - -function test1():void { - try { - check1($reasons); - printf("ok\n"); - } catch (\Exception $e) { - if (!empty($reasons)) { - $e = new \Exception("Dependency check failed: " . implode(', ', $reasons), 0, $e); - } - throw new \Exception("Failed", 0, $e); - } -} diff --git a/tests/PHPStan/Rules/Variables/data/bug-7806.php b/tests/PHPStan/Rules/Variables/data/bug-7806.php index e9bf61aeab7..b029f23a25c 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-7806.php +++ b/tests/PHPStan/Rules/Variables/data/bug-7806.php @@ -2,7 +2,7 @@ namespace Bug7806; -class Test { +class TestMethod { /** * @param array|null $reasons * @throws \Exception @@ -27,3 +27,28 @@ function test():void { } } } + +/** + * @param array|null $reasons + * @throws \Exception + */ +function check1(array &$reasons = null): void { + $fileName = time() % 2 ? "abc":null; + if (!$fileName) { + $reasons[] = sprintf("Dependency check fail"); + throw new \Exception("check failed"); + } +} + +function test1():void { + try { + check1($reasons); + printf("ok\n"); + } catch (\Exception $e) { + if (!empty($reasons)) { + $e = new \Exception("Dependency check failed: " . implode(', ', $reasons), 0, $e); + } + throw new \Exception("Failed", 0, $e); + } +} + From 4d6050774ee4f06c10a4ae31683ce6d256fcf458 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 18 Mar 2026 08:09:37 +0100 Subject: [PATCH 8/8] clearer names --- tests/PHPStan/Rules/Variables/data/bug-14318.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/PHPStan/Rules/Variables/data/bug-14318.php b/tests/PHPStan/Rules/Variables/data/bug-14318.php index c8a1896e66c..68d25809562 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-14318.php +++ b/tests/PHPStan/Rules/Variables/data/bug-14318.php @@ -58,7 +58,7 @@ public function maybeThrows6(int $s): void } } -class HelloWorld7 +class HelloWorld7Static { public function test7(): void { @@ -86,7 +86,7 @@ public static function maybeThrows7(string $s): void } } -class HelloWorld8 +class HelloWorld8Static { public function test8(): void {