Skip to content

Commit 2911f68

Browse files
ondrejmirtesondrejmirtes
committed
Optimization - do not pass along implicit throw points outside of try-catch
1 parent 78a2333 commit 2911f68

File tree

3 files changed

+79
-18
lines changed

3 files changed

+79
-18
lines changed

‎src/Analyser/NodeScopeResolver.php

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ public function processNodes(
261261
}
262262

263263
/**
264-
* @param \PhpParser\Node $parentNode
264+
* @param \PhpParser\Node\Stmt|\PhpParser\Node\Expr $parentNode
265265
* @param \PhpParser\Node\Stmt[] $stmts
266266
* @param \PHPStan\Analyser\MutatingScope $scope
267267
* @param callable(\PhpParser\Node $node, Scope $scope): void $nodeCallback
@@ -298,6 +298,7 @@ public function processStmtNodes(
298298
$nodeCallback(new ExecutionEndNode(
299299
$stmt,
300300
new StatementResult(
301+
$stmt,
301302
$scope,
302303
$hasYield,
303304
$statementResult->isAlwaysTerminating(),
@@ -323,7 +324,7 @@ public function processStmtNodes(
323324
break;
324325
}
325326

326-
$statementResult = new StatementResult($scope, $hasYield, $alreadyTerminated, $exitPoints, $throwPoints);
327+
$statementResult = new StatementResult($parentNode instanceof Node\Stmt ? $parentNode : new Node\Stmt\Expression($parentNode), $scope, $hasYield, $alreadyTerminated, $exitPoints, $throwPoints);
327328
if ($stmtCount === 0 && $shouldCheckLastStatement) {
328329
/** @var Node\Stmt\Function_|Node\Stmt\ClassMethod|Expr\Closure $parentNode */
329330
$parentNode = $parentNode;
@@ -372,12 +373,12 @@ private function processStmtNode(
372373
) {
373374
$methodReflection = $scope->getClassReflection()->getNativeMethod($stmt->name->toString());
374375
if ($methodReflection instanceof NativeMethodReflection) {
375-
return new StatementResult($scope, false, false, [], []);
376+
return new StatementResult($stmt, $scope, false, false, [], []);
376377
}
377378
if ($methodReflection instanceof PhpMethodReflection) {
378379
$declaringTrait = $methodReflection->getDeclaringTrait();
379380
if ($declaringTrait === null || $declaringTrait->getName() !== $scope->getTraitReflection()->getName()) {
380-
return new StatementResult($scope, false, false, [], []);
381+
return new StatementResult($stmt, $scope, false, false, [], []);
381382
}
382383
}
383384
}
@@ -560,7 +561,7 @@ private function processStmtNode(
560561
$throwPoints = [];
561562
}
562563

563-
return new StatementResult($scope, $hasYield, true, [
564+
return new StatementResult($stmt, $scope, $hasYield, true, [
564565
new StatementExitPoint($stmt, $scope),
565566
], $throwPoints);
566567
} elseif ($stmt instanceof Continue_ || $stmt instanceof Break_) {
@@ -574,7 +575,7 @@ private function processStmtNode(
574575
$throwPoints = [];
575576
}
576577

577-
return new StatementResult($scope, $hasYield, true, [
578+
return new StatementResult($stmt, $scope, $hasYield, true, [
578579
new StatementExitPoint($stmt, $scope),
579580
], $throwPoints);
580581
} elseif ($stmt instanceof Node\Stmt\Expression) {
@@ -589,11 +590,11 @@ private function processStmtNode(
589590
$hasYield = $result->hasYield();
590591
$throwPoints = $result->getThrowPoints();
591592
if ($earlyTerminationExpr !== null) {
592-
return new StatementResult($scope, $hasYield, true, [
593+
return new StatementResult($stmt, $scope, $hasYield, true, [
593594
new StatementExitPoint($stmt, $scope),
594595
], $throwPoints);
595596
}
596-
return new StatementResult($scope, $hasYield, false, [], $throwPoints);
597+
return new StatementResult($stmt, $scope, $hasYield, false, [], $throwPoints);
597598
} elseif ($stmt instanceof Node\Stmt\Namespace_) {
598599
if ($stmt->name !== null) {
599600
$scope = $scope->enterNamespace($stmt->name->toString());
@@ -603,7 +604,7 @@ private function processStmtNode(
603604
$hasYield = false;
604605
$throwPoints = [];
605606
} elseif ($stmt instanceof Node\Stmt\Trait_) {
606-
return new StatementResult($scope, false, false, [], []);
607+
return new StatementResult($stmt, $scope, false, false, [], []);
607608
} elseif ($stmt instanceof Node\Stmt\ClassLike) {
608609
$hasYield = false;
609610
$throwPoints = [];
@@ -679,7 +680,7 @@ private function processStmtNode(
679680
$result = $this->processExprNode($stmt->expr, $scope, $nodeCallback, ExpressionContext::createDeep());
680681
$throwPoints = $result->getThrowPoints();
681682
$throwPoints[] = ThrowPoint::createExplicit($result->getScope(), $scope->getType($stmt->expr), false);
682-
return new StatementResult($result->getScope(), $result->hasYield(), true, [
683+
return new StatementResult($stmt, $result->getScope(), $result->hasYield(), true, [
683684
new StatementExitPoint($stmt, $scope),
684685
], $throwPoints);
685686
} elseif ($stmt instanceof If_) {
@@ -767,7 +768,7 @@ private function processStmtNode(
767768
$finalScope = $scope;
768769
}
769770

770-
return new StatementResult($finalScope, $hasYield, $alwaysTerminating, $exitPoints, $throwPoints);
771+
return new StatementResult($stmt, $finalScope, $hasYield, $alwaysTerminating, $exitPoints, $throwPoints);
771772
} elseif ($stmt instanceof Node\Stmt\TraitUse) {
772773
$hasYield = false;
773774
$throwPoints = [];
@@ -833,6 +834,7 @@ private function processStmtNode(
833834
}
834835

835836
return new StatementResult(
837+
$stmt,
836838
$finalScope,
837839
$finalScopeResult->hasYield() || $condResult->hasYield(),
838840
$isIterableAtLeastOnce->yes() && $finalScopeResult->isAlwaysTerminating(),
@@ -906,6 +908,7 @@ private function processStmtNode(
906908
}
907909

908910
return new StatementResult(
911+
$stmt,
909912
$finalScope,
910913
$finalScopeResult->hasYield() || $condResult->hasYield(),
911914
$isAlwaysTerminating,
@@ -974,6 +977,7 @@ private function processStmtNode(
974977
}
975978

976979
return new StatementResult(
980+
$stmt,
977981
$finalScope,
978982
$bodyScopeResult->hasYield() || $hasYield,
979983
$alwaysTerminating,
@@ -1057,6 +1061,7 @@ private function processStmtNode(
10571061
$finalScope = $finalScope->mergeWith($scope);
10581062

10591063
return new StatementResult(
1064+
$stmt,
10601065
$finalScope,
10611066
$finalScopeResult->hasYield() || $hasYield,
10621067
false/* $finalScopeResult->isAlwaysTerminating() && $isAlwaysIterable*/,
@@ -1128,7 +1133,7 @@ private function processStmtNode(
11281133
$finalScope = $scope->mergeWith($finalScope);
11291134
}
11301135

1131-
return new StatementResult($finalScope, $hasYield, $alwaysTerminating, $exitPointsForOuterLoop, $throwPoints);
1136+
return new StatementResult($stmt, $finalScope, $hasYield, $alwaysTerminating, $exitPointsForOuterLoop, $throwPoints);
11321137
} elseif ($stmt instanceof TryCatch) {
11331138
$branchScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $scope, $nodeCallback);
11341139
$branchScope = $branchScopeResult->getScope();
@@ -1298,7 +1303,7 @@ private function processStmtNode(
12981303
$exitPoints = array_merge($exitPoints, $finallyResult->getExitPoints());
12991304
}
13001305

1301-
return new StatementResult($finalScope, $hasYield, $alwaysTerminating, $exitPoints, array_merge($throwPoints, $throwPointsForLater));
1306+
return new StatementResult($stmt, $finalScope, $hasYield, $alwaysTerminating, $exitPoints, array_merge($throwPoints, $throwPointsForLater));
13021307
} elseif ($stmt instanceof Unset_) {
13031308
$hasYield = false;
13041309
$throwPoints = [];
@@ -1393,7 +1398,7 @@ private function processStmtNode(
13931398
$throwPoints = [];
13941399
}
13951400

1396-
return new StatementResult($scope, $hasYield, false, [], $throwPoints);
1401+
return new StatementResult($stmt, $scope, $hasYield, false, [], $throwPoints);
13971402
}
13981403

13991404
private function getCurrentClassReflection(Node\Stmt\ClassLike $stmt, Scope $scope): ClassReflection

‎src/Analyser/StatementResult.php

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,15 @@
22

33
namespace PHPStan\Analyser;
44

5+
use PhpParser\Node\Expr\Closure;
56
use PhpParser\Node\Scalar\LNumber;
67
use PhpParser\Node\Stmt;
78

89
class StatementResult
910
{
1011

12+
private Stmt $statement;
13+
1114
private MutatingScope $scope;
1215

1316
private bool $hasYield;
@@ -21,25 +24,78 @@ class StatementResult
2124
private array $throwPoints;
2225

2326
/**
27+
* @param Stmt $statement
2428
* @param MutatingScope $scope
2529
* @param bool $hasYield
2630
* @param bool $isAlwaysTerminating
2731
* @param StatementExitPoint[] $exitPoints
2832
* @param ThrowPoint[] $throwPoints
2933
*/
3034
public function __construct(
35+
Stmt $statement,
3136
MutatingScope $scope,
3237
bool $hasYield,
3338
bool $isAlwaysTerminating,
3439
array $exitPoints,
3540
array $throwPoints
3641
)
3742
{
43+
$this->statement = $statement;
3844
$this->scope = $scope;
3945
$this->hasYield = $hasYield;
4046
$this->isAlwaysTerminating = $isAlwaysTerminating;
4147
$this->exitPoints = $exitPoints;
42-
$this->throwPoints = $throwPoints;
48+
$this->throwPoints = $this->filterThrowPoints($statement, $throwPoints);
49+
}
50+
51+
/**
52+
* @param Stmt $statement
53+
* @param ThrowPoint[] $throwPoints
54+
* @return ThrowPoint[]
55+
*/
56+
private function filterThrowPoints(Stmt $statement, array $throwPoints): array
57+
{
58+
if ($this->isInTry($statement)) {
59+
return $throwPoints;
60+
}
61+
62+
return array_values(array_filter($throwPoints, static function (ThrowPoint $throwPoint): bool {
63+
return $throwPoint->isExplicit();
64+
}));
65+
}
66+
67+
private function isInTry(\PhpParser\Node $node): bool
68+
{
69+
if ($node instanceof Stmt\TryCatch) {
70+
return true;
71+
}
72+
73+
if ($node instanceof Stmt\Catch_ || $node instanceof Stmt\Finally_) {
74+
$parent = $node->getAttribute('parent');
75+
if ($parent === null) {
76+
return false;
77+
}
78+
$parent2 = $parent->getAttribute('parent');
79+
if (!$parent2 instanceof Stmt) {
80+
return false;
81+
}
82+
return $this->isInTry($parent2);
83+
}
84+
85+
if (
86+
$node instanceof Stmt\ClassMethod
87+
|| $node instanceof Stmt\Function_
88+
|| $node instanceof Closure
89+
) {
90+
return false;
91+
}
92+
93+
$parent = $node->getAttribute('parent');
94+
if ($parent === null) {
95+
return false;
96+
}
97+
98+
return $this->isInTry($parent);
4399
}
44100

45101
public function getScope(): MutatingScope
@@ -71,14 +127,14 @@ public function filterOutLoopExitPoints(): self
71127

72128
$num = $statement->num;
73129
if (!$num instanceof LNumber) {
74-
return new self($this->scope, $this->hasYield, false, $this->exitPoints, $this->throwPoints);
130+
return new self($this->statement, $this->scope, $this->hasYield, false, $this->exitPoints, $this->throwPoints);
75131
}
76132

77133
if ($num->value !== 1) {
78134
continue;
79135
}
80136

81-
return new self($this->scope, $this->hasYield, false, $this->exitPoints, $this->throwPoints);
137+
return new self($this->statement, $this->scope, $this->hasYield, false, $this->exitPoints, $this->throwPoints);
82138
}
83139

84140
return $this;

‎tests/PHPStan/Analyser/StatementResultTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ public function testIsAlwaysTerminating(
385385
): void
386386
{
387387
/** @var Parser $parser */
388-
$parser = self::getContainer()->getByType(Parser::class);
388+
$parser = self::getContainer()->getService('currentPhpVersionRichParser');
389389

390390
/** @var Stmt[] $stmts */
391391
$stmts = $parser->parseString(sprintf('<?php %s', $code));

0 commit comments

Comments
 (0)