Skip to content

Commit

Permalink
Optimization - do not pass along implicit throw points outside of try…
Browse files Browse the repository at this point in the history 
…-catch
  • Loading branch information
@ondrejmirtes
ondrejmirtes committed Apr 16, 2021
1 parent 78a2333 commit 2911f68
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 18 deletions.
33 changes: 19 additions & 14 deletions src/Analyser/NodeScopeResolver.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ public function processNodes(
}

/**
* @param \PhpParser\Node $parentNode
* @param \PhpParser\Node\Stmt|\PhpParser\Node\Expr $parentNode
* @param \PhpParser\Node\Stmt[] $stmts
* @param \PHPStan\Analyser\MutatingScope $scope
* @param callable(\PhpParser\Node $node, Scope $scope): void $nodeCallback
Expand Down Expand Up @@ -298,6 +298,7 @@ public function processStmtNodes(
$nodeCallback(new ExecutionEndNode(
$stmt,
new StatementResult(
$stmt,
$scope,
$hasYield,
$statementResult->isAlwaysTerminating(),
Expand All @@ -323,7 +324,7 @@ public function processStmtNodes(
break;
}

$statementResult = new StatementResult($scope, $hasYield, $alreadyTerminated, $exitPoints, $throwPoints);
$statementResult = new StatementResult($parentNode instanceof Node\Stmt ? $parentNode : new Node\Stmt\Expression($parentNode), $scope, $hasYield, $alreadyTerminated, $exitPoints, $throwPoints);
if ($stmtCount === 0 && $shouldCheckLastStatement) {
/** @var Node\Stmt\Function_|Node\Stmt\ClassMethod|Expr\Closure $parentNode */
$parentNode = $parentNode;
Expand Down Expand Up @@ -372,12 +373,12 @@ private function processStmtNode(
) {
$methodReflection = $scope->getClassReflection()->getNativeMethod($stmt->name->toString());
if ($methodReflection instanceof NativeMethodReflection) {
return new StatementResult($scope, false, false, [], []);
return new StatementResult($stmt, $scope, false, false, [], []);
}
if ($methodReflection instanceof PhpMethodReflection) {
$declaringTrait = $methodReflection->getDeclaringTrait();
if ($declaringTrait === null || $declaringTrait->getName() !== $scope->getTraitReflection()->getName()) {
return new StatementResult($scope, false, false, [], []);
return new StatementResult($stmt, $scope, false, false, [], []);
}
}
}
Expand Down Expand Up @@ -560,7 +561,7 @@ private function processStmtNode(
$throwPoints = [];
}

return new StatementResult($scope, $hasYield, true, [
return new StatementResult($stmt, $scope, $hasYield, true, [
new StatementExitPoint($stmt, $scope),
], $throwPoints);
} elseif ($stmt instanceof Continue_ || $stmt instanceof Break_) {
Expand All @@ -574,7 +575,7 @@ private function processStmtNode(
$throwPoints = [];
}

return new StatementResult($scope, $hasYield, true, [
return new StatementResult($stmt, $scope, $hasYield, true, [
new StatementExitPoint($stmt, $scope),
], $throwPoints);
} elseif ($stmt instanceof Node\Stmt\Expression) {
Expand All @@ -589,11 +590,11 @@ private function processStmtNode(
$hasYield = $result->hasYield();
$throwPoints = $result->getThrowPoints();
if ($earlyTerminationExpr !== null) {
return new StatementResult($scope, $hasYield, true, [
return new StatementResult($stmt, $scope, $hasYield, true, [
new StatementExitPoint($stmt, $scope),
], $throwPoints);
}
return new StatementResult($scope, $hasYield, false, [], $throwPoints);
return new StatementResult($stmt, $scope, $hasYield, false, [], $throwPoints);
} elseif ($stmt instanceof Node\Stmt\Namespace_) {
if ($stmt->name !== null) {
$scope = $scope->enterNamespace($stmt->name->toString());
Expand All @@ -603,7 +604,7 @@ private function processStmtNode(
$hasYield = false;
$throwPoints = [];
} elseif ($stmt instanceof Node\Stmt\Trait_) {
return new StatementResult($scope, false, false, [], []);
return new StatementResult($stmt, $scope, false, false, [], []);
} elseif ($stmt instanceof Node\Stmt\ClassLike) {
$hasYield = false;
$throwPoints = [];
Expand Down Expand Up @@ -679,7 +680,7 @@ private function processStmtNode(
$result = $this->processExprNode($stmt->expr, $scope, $nodeCallback, ExpressionContext::createDeep());
$throwPoints = $result->getThrowPoints();
$throwPoints[] = ThrowPoint::createExplicit($result->getScope(), $scope->getType($stmt->expr), false);
return new StatementResult($result->getScope(), $result->hasYield(), true, [
return new StatementResult($stmt, $result->getScope(), $result->hasYield(), true, [
new StatementExitPoint($stmt, $scope),
], $throwPoints);
} elseif ($stmt instanceof If_) {
Expand Down Expand Up @@ -767,7 +768,7 @@ private function processStmtNode(
$finalScope = $scope;
}

return new StatementResult($finalScope, $hasYield, $alwaysTerminating, $exitPoints, $throwPoints);
return new StatementResult($stmt, $finalScope, $hasYield, $alwaysTerminating, $exitPoints, $throwPoints);
} elseif ($stmt instanceof Node\Stmt\TraitUse) {
$hasYield = false;
$throwPoints = [];
Expand Down Expand Up @@ -833,6 +834,7 @@ private function processStmtNode(
}

return new StatementResult(
$stmt,
$finalScope,
$finalScopeResult->hasYield() || $condResult->hasYield(),
$isIterableAtLeastOnce->yes() && $finalScopeResult->isAlwaysTerminating(),
Expand Down Expand Up @@ -906,6 +908,7 @@ private function processStmtNode(
}

return new StatementResult(
$stmt,
$finalScope,
$finalScopeResult->hasYield() || $condResult->hasYield(),
$isAlwaysTerminating,
Expand Down Expand Up @@ -974,6 +977,7 @@ private function processStmtNode(
}

return new StatementResult(
$stmt,
$finalScope,
$bodyScopeResult->hasYield() || $hasYield,
$alwaysTerminating,
Expand Down Expand Up @@ -1057,6 +1061,7 @@ private function processStmtNode(
$finalScope = $finalScope->mergeWith($scope);

return new StatementResult(
$stmt,
$finalScope,
$finalScopeResult->hasYield() || $hasYield,
false/* $finalScopeResult->isAlwaysTerminating() && $isAlwaysIterable*/,
Expand Down Expand Up @@ -1128,7 +1133,7 @@ private function processStmtNode(
$finalScope = $scope->mergeWith($finalScope);
}

return new StatementResult($finalScope, $hasYield, $alwaysTerminating, $exitPointsForOuterLoop, $throwPoints);
return new StatementResult($stmt, $finalScope, $hasYield, $alwaysTerminating, $exitPointsForOuterLoop, $throwPoints);
} elseif ($stmt instanceof TryCatch) {
$branchScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $scope, $nodeCallback);
$branchScope = $branchScopeResult->getScope();
Expand Down Expand Up @@ -1298,7 +1303,7 @@ private function processStmtNode(
$exitPoints = array_merge($exitPoints, $finallyResult->getExitPoints());
}

return new StatementResult($finalScope, $hasYield, $alwaysTerminating, $exitPoints, array_merge($throwPoints, $throwPointsForLater));
return new StatementResult($stmt, $finalScope, $hasYield, $alwaysTerminating, $exitPoints, array_merge($throwPoints, $throwPointsForLater));
} elseif ($stmt instanceof Unset_) {
$hasYield = false;
$throwPoints = [];
Expand Down Expand Up @@ -1393,7 +1398,7 @@ private function processStmtNode(
$throwPoints = [];
}

return new StatementResult($scope, $hasYield, false, [], $throwPoints);
return new StatementResult($stmt, $scope, $hasYield, false, [], $throwPoints);
}

private function getCurrentClassReflection(Node\Stmt\ClassLike $stmt, Scope $scope): ClassReflection
Expand Down
62 changes: 59 additions & 3 deletions src/Analyser/StatementResult.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@

namespace PHPStan\Analyser;

use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Scalar\LNumber;
use PhpParser\Node\Stmt;

class StatementResult
{

private Stmt $statement;

private MutatingScope $scope;

private bool $hasYield;
Expand All @@ -21,25 +24,78 @@ class StatementResult
private array $throwPoints;

/**
* @param Stmt $statement
* @param MutatingScope $scope
* @param bool $hasYield
* @param bool $isAlwaysTerminating
* @param StatementExitPoint[] $exitPoints
* @param ThrowPoint[] $throwPoints
*/
public function __construct(
Stmt $statement,
MutatingScope $scope,
bool $hasYield,
bool $isAlwaysTerminating,
array $exitPoints,
array $throwPoints
)
{
$this->statement = $statement;
$this->scope = $scope;
$this->hasYield = $hasYield;
$this->isAlwaysTerminating = $isAlwaysTerminating;
$this->exitPoints = $exitPoints;
$this->throwPoints = $throwPoints;
$this->throwPoints = $this->filterThrowPoints($statement, $throwPoints);
}

/**
* @param Stmt $statement
* @param ThrowPoint[] $throwPoints
* @return ThrowPoint[]
*/
private function filterThrowPoints(Stmt $statement, array $throwPoints): array
{
if ($this->isInTry($statement)) {
return $throwPoints;
}

return array_values(array_filter($throwPoints, static function (ThrowPoint $throwPoint): bool {
return $throwPoint->isExplicit();
}));
}

private function isInTry(\PhpParser\Node $node): bool
{
if ($node instanceof Stmt\TryCatch) {
return true;
}

if ($node instanceof Stmt\Catch_ || $node instanceof Stmt\Finally_) {
$parent = $node->getAttribute('parent');
if ($parent === null) {
return false;
}
$parent2 = $parent->getAttribute('parent');
if (!$parent2 instanceof Stmt) {
return false;
}
return $this->isInTry($parent2);
}

if (
$node instanceof Stmt\ClassMethod
|| $node instanceof Stmt\Function_
|| $node instanceof Closure
) {
return false;
}

$parent = $node->getAttribute('parent');
if ($parent === null) {
return false;
}

return $this->isInTry($parent);
}

public function getScope(): MutatingScope
Expand Down Expand Up @@ -71,14 +127,14 @@ public function filterOutLoopExitPoints(): self

$num = $statement->num;
if (!$num instanceof LNumber) {
return new self($this->scope, $this->hasYield, false, $this->exitPoints, $this->throwPoints);
return new self($this->statement, $this->scope, $this->hasYield, false, $this->exitPoints, $this->throwPoints);
}

if ($num->value !== 1) {
continue;
}

return new self($this->scope, $this->hasYield, false, $this->exitPoints, $this->throwPoints);
return new self($this->statement, $this->scope, $this->hasYield, false, $this->exitPoints, $this->throwPoints);
}

return $this;
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/StatementResultTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ public function testIsAlwaysTerminating(
): void
{
/** @var Parser $parser */
$parser = self::getContainer()->getByType(Parser::class);
$parser = self::getContainer()->getService('currentPhpVersionRichParser');

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

0 comments on commit 2911f68

Please sign in to comment.