Skip to content

Commit 4159444

Browse files
committed
Revert Optimization - do not pass along implicit throw points outside of try-catch
This reverts commit 2911f68.
1 parent 7c0146c commit 4159444

File tree

2 files changed

+17
-78
lines changed

2 files changed

+17
-78
lines changed

src/Analyser/NodeScopeResolver.php

+14-19
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ public function processNodes(
261261
}
262262

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

327-
$statementResult = new StatementResult($parentNode instanceof Node\Stmt ? $parentNode : new Node\Stmt\Expression($parentNode), $scope, $hasYield, $alreadyTerminated, $exitPoints, $throwPoints);
326+
$statementResult = new StatementResult($scope, $hasYield, $alreadyTerminated, $exitPoints, $throwPoints);
328327
if ($stmtCount === 0 && $shouldCheckLastStatement) {
329328
/** @var Node\Stmt\Function_|Node\Stmt\ClassMethod|Expr\Closure $parentNode */
330329
$parentNode = $parentNode;
@@ -373,12 +372,12 @@ private function processStmtNode(
373372
) {
374373
$methodReflection = $scope->getClassReflection()->getNativeMethod($stmt->name->toString());
375374
if ($methodReflection instanceof NativeMethodReflection) {
376-
return new StatementResult($stmt, $scope, false, false, [], []);
375+
return new StatementResult($scope, false, false, [], []);
377376
}
378377
if ($methodReflection instanceof PhpMethodReflection) {
379378
$declaringTrait = $methodReflection->getDeclaringTrait();
380379
if ($declaringTrait === null || $declaringTrait->getName() !== $scope->getTraitReflection()->getName()) {
381-
return new StatementResult($stmt, $scope, false, false, [], []);
380+
return new StatementResult($scope, false, false, [], []);
382381
}
383382
}
384383
}
@@ -561,7 +560,7 @@ private function processStmtNode(
561560
$throwPoints = [];
562561
}
563562

564-
return new StatementResult($stmt, $scope, $hasYield, true, [
563+
return new StatementResult($scope, $hasYield, true, [
565564
new StatementExitPoint($stmt, $scope),
566565
], $throwPoints);
567566
} elseif ($stmt instanceof Continue_ || $stmt instanceof Break_) {
@@ -575,7 +574,7 @@ private function processStmtNode(
575574
$throwPoints = [];
576575
}
577576

578-
return new StatementResult($stmt, $scope, $hasYield, true, [
577+
return new StatementResult($scope, $hasYield, true, [
579578
new StatementExitPoint($stmt, $scope),
580579
], $throwPoints);
581580
} elseif ($stmt instanceof Node\Stmt\Expression) {
@@ -590,11 +589,11 @@ private function processStmtNode(
590589
$hasYield = $result->hasYield();
591590
$throwPoints = $result->getThrowPoints();
592591
if ($earlyTerminationExpr !== null) {
593-
return new StatementResult($stmt, $scope, $hasYield, true, [
592+
return new StatementResult($scope, $hasYield, true, [
594593
new StatementExitPoint($stmt, $scope),
595594
], $throwPoints);
596595
}
597-
return new StatementResult($stmt, $scope, $hasYield, false, [], $throwPoints);
596+
return new StatementResult($scope, $hasYield, false, [], $throwPoints);
598597
} elseif ($stmt instanceof Node\Stmt\Namespace_) {
599598
if ($stmt->name !== null) {
600599
$scope = $scope->enterNamespace($stmt->name->toString());
@@ -604,7 +603,7 @@ private function processStmtNode(
604603
$hasYield = false;
605604
$throwPoints = [];
606605
} elseif ($stmt instanceof Node\Stmt\Trait_) {
607-
return new StatementResult($stmt, $scope, false, false, [], []);
606+
return new StatementResult($scope, false, false, [], []);
608607
} elseif ($stmt instanceof Node\Stmt\ClassLike) {
609608
$hasYield = false;
610609
$throwPoints = [];
@@ -680,7 +679,7 @@ private function processStmtNode(
680679
$result = $this->processExprNode($stmt->expr, $scope, $nodeCallback, ExpressionContext::createDeep());
681680
$throwPoints = $result->getThrowPoints();
682681
$throwPoints[] = ThrowPoint::createExplicit($result->getScope(), $scope->getType($stmt->expr), false);
683-
return new StatementResult($stmt, $result->getScope(), $result->hasYield(), true, [
682+
return new StatementResult($result->getScope(), $result->hasYield(), true, [
684683
new StatementExitPoint($stmt, $scope),
685684
], $throwPoints);
686685
} elseif ($stmt instanceof If_) {
@@ -768,7 +767,7 @@ private function processStmtNode(
768767
$finalScope = $scope;
769768
}
770769

771-
return new StatementResult($stmt, $finalScope, $hasYield, $alwaysTerminating, $exitPoints, $throwPoints);
770+
return new StatementResult($finalScope, $hasYield, $alwaysTerminating, $exitPoints, $throwPoints);
772771
} elseif ($stmt instanceof Node\Stmt\TraitUse) {
773772
$hasYield = false;
774773
$throwPoints = [];
@@ -834,7 +833,6 @@ private function processStmtNode(
834833
}
835834

836835
return new StatementResult(
837-
$stmt,
838836
$finalScope,
839837
$finalScopeResult->hasYield() || $condResult->hasYield(),
840838
$isIterableAtLeastOnce->yes() && $finalScopeResult->isAlwaysTerminating(),
@@ -908,7 +906,6 @@ private function processStmtNode(
908906
}
909907

910908
return new StatementResult(
911-
$stmt,
912909
$finalScope,
913910
$finalScopeResult->hasYield() || $condResult->hasYield(),
914911
$isAlwaysTerminating,
@@ -977,7 +974,6 @@ private function processStmtNode(
977974
}
978975

979976
return new StatementResult(
980-
$stmt,
981977
$finalScope,
982978
$bodyScopeResult->hasYield() || $hasYield,
983979
$alwaysTerminating,
@@ -1061,7 +1057,6 @@ private function processStmtNode(
10611057
$finalScope = $finalScope->mergeWith($scope);
10621058

10631059
return new StatementResult(
1064-
$stmt,
10651060
$finalScope,
10661061
$finalScopeResult->hasYield() || $hasYield,
10671062
false/* $finalScopeResult->isAlwaysTerminating() && $isAlwaysIterable*/,
@@ -1133,7 +1128,7 @@ private function processStmtNode(
11331128
$finalScope = $scope->mergeWith($finalScope);
11341129
}
11351130

1136-
return new StatementResult($stmt, $finalScope, $hasYield, $alwaysTerminating, $exitPointsForOuterLoop, $throwPoints);
1131+
return new StatementResult($finalScope, $hasYield, $alwaysTerminating, $exitPointsForOuterLoop, $throwPoints);
11371132
} elseif ($stmt instanceof TryCatch) {
11381133
$branchScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $scope, $nodeCallback);
11391134
$branchScope = $branchScopeResult->getScope();
@@ -1303,7 +1298,7 @@ private function processStmtNode(
13031298
$exitPoints = array_merge($exitPoints, $finallyResult->getExitPoints());
13041299
}
13051300

1306-
return new StatementResult($stmt, $finalScope, $hasYield, $alwaysTerminating, $exitPoints, array_merge($throwPoints, $throwPointsForLater));
1301+
return new StatementResult($finalScope, $hasYield, $alwaysTerminating, $exitPoints, array_merge($throwPoints, $throwPointsForLater));
13071302
} elseif ($stmt instanceof Unset_) {
13081303
$hasYield = false;
13091304
$throwPoints = [];
@@ -1398,7 +1393,7 @@ private function processStmtNode(
13981393
$throwPoints = [];
13991394
}
14001395

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

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

src/Analyser/StatementResult.php

+3-59
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,12 @@
22

33
namespace PHPStan\Analyser;
44

5-
use PhpParser\Node\Expr\Closure;
65
use PhpParser\Node\Scalar\LNumber;
76
use PhpParser\Node\Stmt;
87

98
class StatementResult
109
{
1110

12-
private Stmt $statement;
13-
1411
private MutatingScope $scope;
1512

1613
private bool $hasYield;
@@ -24,78 +21,25 @@ class StatementResult
2421
private array $throwPoints;
2522

2623
/**
27-
* @param Stmt $statement
2824
* @param MutatingScope $scope
2925
* @param bool $hasYield
3026
* @param bool $isAlwaysTerminating
3127
* @param StatementExitPoint[] $exitPoints
3228
* @param ThrowPoint[] $throwPoints
3329
*/
3430
public function __construct(
35-
Stmt $statement,
3631
MutatingScope $scope,
3732
bool $hasYield,
3833
bool $isAlwaysTerminating,
3934
array $exitPoints,
4035
array $throwPoints
4136
)
4237
{
43-
$this->statement = $statement;
4438
$this->scope = $scope;
4539
$this->hasYield = $hasYield;
4640
$this->isAlwaysTerminating = $isAlwaysTerminating;
4741
$this->exitPoints = $exitPoints;
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);
42+
$this->throwPoints = $throwPoints;
9943
}
10044

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

12872
$num = $statement->num;
12973
if (!$num instanceof LNumber) {
130-
return new self($this->statement, $this->scope, $this->hasYield, false, $this->exitPoints, $this->throwPoints);
74+
return new self($this->scope, $this->hasYield, false, $this->exitPoints, $this->throwPoints);
13175
}
13276

13377
if ($num->value !== 1) {
13478
continue;
13579
}
13680

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

14084
return $this;

0 commit comments

Comments
 (0)