Skip to content

Commit cb215d3

Browse files
ondrejmirtesondrejmirtes
committed
Apply finally scope changes to try-catch exit points and check by-ref return types after finally
* `TryCatchHandler` now runs `MutatingScope::processFinallyScope()` on every exit point leaving a try or catch block, so the state a `return`/`break`/`continue`/`goto` carries out of the statement includes what the `finally` block did afterwards. This also makes nested try-finally blocks compose. * New `ReturnAfterFinallyNode` virtual node is emitted for each `return` exit point with that post-`finally` scope, collected by the function/method/closure/property-hook node gatherers and exposed as `ReturnStatementsNode::getReturnStatementsAfterFinally()`. * New `ReturnTypeAfterFinallyRule` (level 3, identifier `return.byRefFinally`) reports when a function returning by reference hands out a value that the `finally` block changes into something the declared return type does not accept. Return statements that already mismatch at the `return` itself are left to the regular return type rules. * Covers functions, methods, static methods, `__invoke`, closures and `&get` property hooks; returns from `catch` blocks; nested `finally` blocks; local/static variables, by-ref parameters, properties, static properties and array offsets; native and PHPDoc-only return types. Generators are skipped (their return value is not by-ref) and so is a `finally` that always terminates (already covered by `OverwrittenExitPointByFinallyRule`). `unset()` in `finally` is correctly not reported - the returned reference survives it.
1 parent 7a08648 commit cb215d3

17 files changed

Lines changed: 757 additions & 4 deletions

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
use PHPStan\Node\MethodCallExpressionNode;
5151
use PHPStan\Node\PropertyAssignNode;
5252
use PHPStan\Node\PropertyHookStatementNode;
53+
use PHPStan\Node\ReturnAfterFinallyNode;
5354
use PHPStan\Node\ReturnStatement;
5455
use PHPStan\Node\StaticMethodCallableNode;
5556
use PHPStan\Node\StaticMethodCallExpressionNode;
@@ -1231,12 +1232,13 @@ public function processClosureNode(
12311232

12321233
$executionEnds = [];
12331234
$gatheredReturnStatements = [];
1235+
$gatheredReturnStatementsAfterFinally = [];
12341236
$gatheredReturnStatementsWithScope = [];
12351237
$gatheredYieldStatements = [];
12361238
$gatheredYieldStatementsWithScope = [];
12371239
$closureImpurePoints = [];
12381240
$invalidateExpressions = [];
1239-
$closureStmtsGatherer = static function (Node $node, Scope $scope) use (&$executionEnds, &$gatheredReturnStatements, &$gatheredReturnStatementsWithScope, &$gatheredYieldStatements, &$gatheredYieldStatementsWithScope, &$closureScope, &$closureImpurePoints, &$invalidateExpressions): void {
1241+
$closureStmtsGatherer = static function (Node $node, Scope $scope) use (&$executionEnds, &$gatheredReturnStatements, &$gatheredReturnStatementsAfterFinally, &$gatheredReturnStatementsWithScope, &$gatheredYieldStatements, &$gatheredYieldStatementsWithScope, &$closureScope, &$closureImpurePoints, &$invalidateExpressions): void {
12401242
if ($scope->getAnonymousFunctionReflection() !== $closureScope->getAnonymousFunctionReflection()) {
12411243
return;
12421244
}
@@ -1255,6 +1257,10 @@ public function processClosureNode(
12551257
$executionEnds[] = $node;
12561258
return;
12571259
}
1260+
if ($node instanceof ReturnAfterFinallyNode) {
1261+
$gatheredReturnStatementsAfterFinally[] = new ReturnStatement($scope, $node->getReturnNode());
1262+
return;
1263+
}
12581264
if ($node instanceof InvalidateExprNode) {
12591265
$invalidateExpressions[] = $node;
12601266
return;
@@ -1283,6 +1289,7 @@ public function processClosureNode(
12831289
$this->callNodeCallback($nodeCallback, new ClosureReturnStatementsNode(
12841290
$expr,
12851291
$gatheredReturnStatements,
1292+
$gatheredReturnStatementsAfterFinally,
12861293
$gatheredYieldStatements,
12871294
$publicStatementResult,
12881295
$executionEnds,
@@ -1383,6 +1390,7 @@ public function processClosureNode(
13831390
$this->callNodeCallback($nodeCallback, new ClosureReturnStatementsNode(
13841391
$expr,
13851392
$gatheredReturnStatements,
1393+
$gatheredReturnStatementsAfterFinally,
13861394
$gatheredYieldStatements,
13871395
$publicStatementResult,
13881396
$executionEnds,

‎src/Analyser/PropertyHooksProcessor.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use PHPStan\Node\PropertyAssignNode;
1616
use PHPStan\Node\PropertyHookReturnStatementsNode;
1717
use PHPStan\Node\PropertyHookStatementNode;
18+
use PHPStan\Node\ReturnAfterFinallyNode;
1819
use PHPStan\Node\ReturnStatement;
1920
use PHPStan\Parser\LineAttributesVisitor;
2021
use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection;
@@ -111,9 +112,10 @@ public function processPropertyHooks(
111112
}
112113

113114
$gatheredReturnStatements = [];
115+
$gatheredReturnStatementsAfterFinally = [];
114116
$executionEnds = [];
115117
$methodImpurePoints = [];
116-
$nodeScopeResolver->pushNodeGatherer(static function (Node $node, Scope $scope) use ($hookScope, &$gatheredReturnStatements, &$executionEnds, &$hookImpurePoints): void {
118+
$nodeScopeResolver->pushNodeGatherer(static function (Node $node, Scope $scope) use ($hookScope, &$gatheredReturnStatements, &$gatheredReturnStatementsAfterFinally, &$executionEnds, &$hookImpurePoints): void {
117119
if ($scope->getFunction() !== $hookScope->getFunction()) {
118120
return;
119121
}
@@ -134,6 +136,10 @@ public function processPropertyHooks(
134136
$executionEnds[] = $node;
135137
return;
136138
}
139+
if ($node instanceof ReturnAfterFinallyNode) {
140+
$gatheredReturnStatementsAfterFinally[] = new ReturnStatement($scope, $node->getReturnNode());
141+
return;
142+
}
137143
if (!$node instanceof Return_) {
138144
return;
139145
}
@@ -149,6 +155,7 @@ public function processPropertyHooks(
149155
$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyHookReturnStatementsNode(
150156
$hook,
151157
$gatheredReturnStatements,
158+
$gatheredReturnStatementsAfterFinally,
152159
$statementResult,
153160
$executionEnds,
154161
array_merge($statementResult->getImpurePoints(), $methodImpurePoints),

‎src/Analyser/StmtHandler/ClassMethodHandler.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use PHPStan\Node\InClassMethodNode;
2727
use PHPStan\Node\MethodReturnStatementsNode;
2828
use PHPStan\Node\PropertyAssignNode;
29+
use PHPStan\Node\ReturnAfterFinallyNode;
2930
use PHPStan\Node\ReturnStatement;
3031
use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection;
3132
use PHPStan\ShouldNotHappenException;
@@ -165,10 +166,11 @@ public function processStmt(
165166

166167
if ($stmt->stmts !== null) {
167168
$gatheredReturnStatements = [];
169+
$gatheredReturnStatementsAfterFinally = [];
168170
$gatheredYieldStatements = [];
169171
$executionEnds = [];
170172
$methodImpurePoints = [];
171-
$nodeScopeResolver->pushNodeGatherer(static function (Node $node, Scope $scope) use ($methodScope, &$gatheredReturnStatements, &$gatheredYieldStatements, &$executionEnds, &$methodImpurePoints): void {
173+
$nodeScopeResolver->pushNodeGatherer(static function (Node $node, Scope $scope) use ($methodScope, &$gatheredReturnStatements, &$gatheredReturnStatementsAfterFinally, &$gatheredYieldStatements, &$executionEnds, &$methodImpurePoints): void {
172174
if ($scope->getFunction() !== $methodScope->getFunction()) {
173175
return;
174176
}
@@ -198,6 +200,10 @@ public function processStmt(
198200
$executionEnds[] = $node;
199201
return;
200202
}
203+
if ($node instanceof ReturnAfterFinallyNode) {
204+
$gatheredReturnStatementsAfterFinally[] = new ReturnStatement($scope, $node->getReturnNode());
205+
return;
206+
}
201207
if ($node instanceof Expr\Yield_ || $node instanceof Expr\YieldFrom) {
202208
$gatheredYieldStatements[] = $node;
203209
}
@@ -221,6 +227,7 @@ public function processStmt(
221227
$nodeScopeResolver->callNodeCallback($nodeCallback, new MethodReturnStatementsNode(
222228
$stmt,
223229
$gatheredReturnStatements,
230+
$gatheredReturnStatementsAfterFinally,
224231
$gatheredYieldStatements,
225232
$statementResult,
226233
$executionEnds,

‎src/Analyser/StmtHandler/FunctionHandler.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use PHPStan\Node\FunctionReturnStatementsNode;
2323
use PHPStan\Node\InFunctionNode;
2424
use PHPStan\Node\PropertyAssignNode;
25+
use PHPStan\Node\ReturnAfterFinallyNode;
2526
use PHPStan\Node\ReturnStatement;
2627
use PHPStan\Reflection\Php\PhpFunctionFromParserNodeReflection;
2728
use PHPStan\ShouldNotHappenException;
@@ -96,10 +97,11 @@ public function processStmt(
9697
$nodeScopeResolver->callNodeCallback($nodeCallback, new InFunctionNode($functionReflection, $stmt), $functionScope, $storage);
9798

9899
$gatheredReturnStatements = [];
100+
$gatheredReturnStatementsAfterFinally = [];
99101
$gatheredYieldStatements = [];
100102
$executionEnds = [];
101103
$functionImpurePoints = [];
102-
$nodeScopeResolver->pushNodeGatherer(static function (Node $node, Scope $scope) use ($functionScope, &$gatheredReturnStatements, &$gatheredYieldStatements, &$executionEnds, &$functionImpurePoints): void {
104+
$nodeScopeResolver->pushNodeGatherer(static function (Node $node, Scope $scope) use ($functionScope, &$gatheredReturnStatements, &$gatheredReturnStatementsAfterFinally, &$gatheredYieldStatements, &$executionEnds, &$functionImpurePoints): void {
103105
if ($scope->getFunction() !== $functionScope->getFunction()) {
104106
return;
105107
}
@@ -120,6 +122,10 @@ public function processStmt(
120122
$executionEnds[] = $node;
121123
return;
122124
}
125+
if ($node instanceof ReturnAfterFinallyNode) {
126+
$gatheredReturnStatementsAfterFinally[] = new ReturnStatement($scope, $node->getReturnNode());
127+
return;
128+
}
123129
if ($node instanceof Expr\Yield_ || $node instanceof Expr\YieldFrom) {
124130
$gatheredYieldStatements[] = $node;
125131
}
@@ -138,6 +144,7 @@ public function processStmt(
138144
$nodeScopeResolver->callNodeCallback($nodeCallback, new FunctionReturnStatementsNode(
139145
$stmt,
140146
$gatheredReturnStatements,
147+
$gatheredReturnStatementsAfterFinally,
141148
$gatheredYieldStatements,
142149
$statementResult,
143150
$executionEnds,

‎src/Analyser/StmtHandler/TryCatchHandler.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use PhpParser\Node\Stmt;
88
use PhpParser\Node\Stmt\TryCatch;
99
use PHPStan\Analyser\ExpressionResultStorage;
10+
use PHPStan\Analyser\InternalStatementExitPoint;
1011
use PHPStan\Analyser\InternalStatementResult;
1112
use PHPStan\Analyser\MutatingScope;
1213
use PHPStan\Analyser\NodeScopeResolver;
@@ -16,6 +17,7 @@
1617
use PHPStan\Node\CatchWithUnthrownExceptionNode;
1718
use PHPStan\Node\Expr\TypeExpr;
1819
use PHPStan\Node\FinallyExitPointsNode;
20+
use PHPStan\Node\ReturnAfterFinallyNode;
1921
use PHPStan\Node\VariableAssignNode;
2022
use PHPStan\ShouldNotHappenException;
2123
use PHPStan\Type\NeverType;
@@ -262,6 +264,28 @@ public function processStmt(
262264
$impurePoints = array_merge($impurePoints, $finallyResult->getImpurePoints());
263265
$finallyScope = $finallyResult->getScope();
264266
$finalScope = $finallyResult->isAlwaysTerminating() ? $finalScope : $finalScope->processFinallyScope($finallyScope, $originalFinallyScope);
267+
if (!$finallyResult->isAlwaysTerminating()) {
268+
// the finally block runs after the exit point, so its changes are
269+
// part of the state the exit point leaves the try-catch with
270+
$exitPointsAfterFinally = [];
271+
foreach ($exitPoints as $exitPoint) {
272+
$exitStatement = $exitPoint->getStatement();
273+
$exitPointScope = $exitPoint->getScope()->processFinallyScope($finallyScope, $originalFinallyScope);
274+
$exitPointsAfterFinally[] = new InternalStatementExitPoint($exitStatement, $exitPointScope);
275+
276+
if (!$exitStatement instanceof Node\Stmt\Return_ || $exitStatement->expr === null) {
277+
continue;
278+
}
279+
280+
$nodeScopeResolver->callNodeCallback(
281+
$nodeCallback,
282+
new ReturnAfterFinallyNode($exitStatement),
283+
$exitPointScope,
284+
$storage,
285+
);
286+
}
287+
$exitPoints = $exitPointsAfterFinally;
288+
}
265289
if (count($finallyResult->getExitPoints()) > 0 && $finallyResult->isAlwaysTerminating()) {
266290
$nodeScopeResolver->callNodeCallback($nodeCallback, new FinallyExitPointsNode(
267291
$finallyResult->toPublic()->getExitPoints(),

‎src/Node/ClosureReturnStatementsNode.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,15 @@ final class ClosureReturnStatementsNode extends NodeAbstract implements ReturnSt
2222

2323
/**
2424
* @param list<ReturnStatement> $returnStatements
25+
* @param list<ReturnStatement> $returnStatementsAfterFinally
2526
* @param list<Yield_|YieldFrom> $yieldStatements
2627
* @param list<ExecutionEndNode> $executionEnds
2728
* @param ImpurePoint[] $impurePoints
2829
*/
2930
public function __construct(
3031
Closure $closureExpr,
3132
private array $returnStatements,
33+
private array $returnStatementsAfterFinally,
3234
private array $yieldStatements,
3335
private StatementResult $statementResult,
3436
private array $executionEnds,
@@ -54,6 +56,11 @@ public function getReturnStatements(): array
5456
return $this->returnStatements;
5557
}
5658

59+
public function getReturnStatementsAfterFinally(): array
60+
{
61+
return $this->returnStatementsAfterFinally;
62+
}
63+
5764
public function getExecutionEnds(): array
5865
{
5966
return $this->executionEnds;

‎src/Node/FunctionReturnStatementsNode.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,15 @@ final class FunctionReturnStatementsNode extends NodeAbstract implements ReturnS
2121

2222
/**
2323
* @param list<ReturnStatement> $returnStatements
24+
* @param list<ReturnStatement> $returnStatementsAfterFinally
2425
* @param list<Yield_|YieldFrom> $yieldStatements
2526
* @param list<ExecutionEndNode> $executionEnds
2627
* @param ImpurePoint[] $impurePoints
2728
*/
2829
public function __construct(
2930
private Function_ $function,
3031
private array $returnStatements,
32+
private array $returnStatementsAfterFinally,
3133
private array $yieldStatements,
3234
private StatementResult $statementResult,
3335
private array $executionEnds,
@@ -43,6 +45,11 @@ public function getReturnStatements(): array
4345
return $this->returnStatements;
4446
}
4547

48+
public function getReturnStatementsAfterFinally(): array
49+
{
50+
return $this->returnStatementsAfterFinally;
51+
}
52+
4653
public function getStatementResult(): StatementResult
4754
{
4855
return $this->statementResult;

‎src/Node/MethodReturnStatementsNode.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,15 @@ final class MethodReturnStatementsNode extends NodeAbstract implements ReturnSta
2424

2525
/**
2626
* @param list<ReturnStatement> $returnStatements
27+
* @param list<ReturnStatement> $returnStatementsAfterFinally
2728
* @param list<Yield_|YieldFrom> $yieldStatements
2829
* @param list<ExecutionEndNode> $executionEnds
2930
* @param ImpurePoint[] $impurePoints
3031
*/
3132
public function __construct(
3233
ClassMethod $method,
3334
private array $returnStatements,
35+
private array $returnStatementsAfterFinally,
3436
private array $yieldStatements,
3537
private StatementResult $statementResult,
3638
private array $executionEnds,
@@ -48,6 +50,11 @@ public function getReturnStatements(): array
4850
return $this->returnStatements;
4951
}
5052

53+
public function getReturnStatementsAfterFinally(): array
54+
{
55+
return $this->returnStatementsAfterFinally;
56+
}
57+
5158
public function getStatementResult(): StatementResult
5259
{
5360
return $this->statementResult;

‎src/Node/PropertyHookReturnStatementsNode.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@ final class PropertyHookReturnStatementsNode extends NodeAbstract implements Ret
1919

2020
/**
2121
* @param list<ReturnStatement> $returnStatements
22+
* @param list<ReturnStatement> $returnStatementsAfterFinally
2223
* @param list<ExecutionEndNode> $executionEnds
2324
* @param ImpurePoint[] $impurePoints
2425
*/
2526
public function __construct(
2627
private PropertyHook $hook,
2728
private array $returnStatements,
29+
private array $returnStatementsAfterFinally,
2830
private StatementResult $statementResult,
2931
private array $executionEnds,
3032
private array $impurePoints,
@@ -66,6 +68,11 @@ public function getReturnStatements(): array
6668
return $this->returnStatements;
6769
}
6870

71+
public function getReturnStatementsAfterFinally(): array
72+
{
73+
return $this->returnStatementsAfterFinally;
74+
}
75+
6976
public function getStatementResult(): StatementResult
7077
{
7178
return $this->statementResult;

‎src/Node/ReturnAfterFinallyNode.php‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Node;
4+
5+
use Override;
6+
use PhpParser\Node\Stmt\Return_;
7+
use PhpParser\NodeAbstract;
8+
9+
/**
10+
* Emitted for a return statement in a try or catch block once the following
11+
* finally block has been analysed. The scope it's emitted with is the scope
12+
* at the return statement with the changes made by the finally block applied.
13+
*/
14+
final class ReturnAfterFinallyNode extends NodeAbstract implements VirtualNode
15+
{
16+
17+
public function __construct(private Return_ $returnNode)
18+
{
19+
parent::__construct($returnNode->getAttributes());
20+
}
21+
22+
public function getReturnNode(): Return_
23+
{
24+
return $this->returnNode;
25+
}
26+
27+
#[Override]
28+
public function getType(): string
29+
{
30+
return 'PHPStan_Node_ReturnAfterFinallyNode';
31+
}
32+
33+
/**
34+
* @return string[]
35+
*/
36+
#[Override]
37+
public function getSubNodeNames(): array
38+
{
39+
return [];
40+
}
41+
42+
}

0 commit comments

Comments
 (0)