Skip to content

Commit deaafc7

Browse files
committed
Fix phpstan/phpstan#14001: Type loss in foreach when using StatementResult::getScope()
- Changed StatementResult, StatementExitPoint, and ReturnStatement to store and return MutatingScope instead of Scope, since mergeWith() is only on MutatingScope and calling it on Scope produced ErrorType, poisoning the variable type across foreach loop iterations - Added toMutatingScope() calls in NodeScopeResolver where ReturnStatement is constructed from a Scope-typed callback parameter - Fixed a null check on getClassReflection() in NodeScopeResolver that was exposed by the more precise type tracking - Removed now-redundant toMutatingScope() calls on ReturnStatement::getScope() - New regression test in tests/PHPStan/Analyser/nsrt/bug-14001.php
1 parent 8b96391 commit deaafc7

File tree

5 files changed

+63
-14
lines changed

5 files changed

+63
-14
lines changed

src/Analyser/NodeScopeResolver.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ public function processStmtNode(
646646
return;
647647
}
648648

649-
$gatheredReturnStatements[] = new ReturnStatement($scope, $node);
649+
$gatheredReturnStatements[] = new ReturnStatement($scope->toMutatingScope(), $node);
650650
}, StatementContext::createTopLevel())->toPublic();
651651

652652
$this->callNodeCallback($nodeCallback, new FunctionReturnStatementsNode(
@@ -804,7 +804,7 @@ public function processStmtNode(
804804
return;
805805
}
806806

807-
$gatheredReturnStatements[] = new ReturnStatement($scope, $node);
807+
$gatheredReturnStatements[] = new ReturnStatement($scope->toMutatingScope(), $node);
808808
}, StatementContext::createTopLevel())->toPublic();
809809

810810
$methodReflection = $methodScope->getFunction();
@@ -842,11 +842,11 @@ public function processStmtNode(
842842

843843
foreach ($gatheredReturnStatements as $statement) {
844844
if ($finalScope === null) {
845-
$finalScope = $statement->getScope()->toMutatingScope();
845+
$finalScope = $statement->getScope();
846846
continue;
847847
}
848848

849-
$finalScope = $finalScope->mergeWith($statement->getScope()->toMutatingScope());
849+
$finalScope = $finalScope->mergeWith($statement->getScope());
850850
}
851851

852852
if ($finalScope !== null) {
@@ -855,7 +855,7 @@ public function processStmtNode(
855855

856856
}
857857
}
858-
if (!$scope->getClassReflection()->isAnonymous() && !$scope->isInAnonymousFunction()) {
858+
if ($scope->getClassReflection() !== null && !$scope->getClassReflection()->isAnonymous() && !$scope->isInAnonymousFunction()) {
859859
$this->processPendingFibers($storage);
860860
}
861861
} elseif ($stmt instanceof Echo_) {
@@ -2763,7 +2763,7 @@ public function processClosureNode(
27632763
return;
27642764
}
27652765

2766-
$gatheredReturnStatements[] = new ReturnStatement($scope, $node);
2766+
$gatheredReturnStatements[] = new ReturnStatement($scope->toMutatingScope(), $node);
27672767
};
27682768

27692769
if (count($byRefUses) === 0) {
@@ -3142,7 +3142,7 @@ private function processPropertyHooks(
31423142
return;
31433143
}
31443144

3145-
$gatheredReturnStatements[] = new ReturnStatement($scope, $node);
3145+
$gatheredReturnStatements[] = new ReturnStatement($scope->toMutatingScope(), $node);
31463146
}, StatementContext::createTopLevel())->toPublic();
31473147

31483148
$this->callNodeCallback($nodeCallback, new PropertyHookReturnStatementsNode(

src/Analyser/StatementExitPoint.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
final class StatementExitPoint
1111
{
1212

13-
public function __construct(private Stmt $statement, private Scope $scope)
13+
public function __construct(private Stmt $statement, private MutatingScope $scope)
1414
{
1515
}
1616

@@ -19,7 +19,7 @@ public function getStatement(): Stmt
1919
return $this->statement;
2020
}
2121

22-
public function getScope(): Scope
22+
public function getScope(): MutatingScope
2323
{
2424
return $this->scope;
2525
}

src/Analyser/StatementResult.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ final class StatementResult
1818
* @param EndStatementResult[] $endStatements
1919
*/
2020
public function __construct(
21-
private Scope $scope,
21+
private MutatingScope $scope,
2222
private bool $hasYield,
2323
private bool $isAlwaysTerminating,
2424
private array $exitPoints,
@@ -29,7 +29,7 @@ public function __construct(
2929
{
3030
}
3131

32-
public function getScope(): Scope
32+
public function getScope(): MutatingScope
3333
{
3434
return $this->scope;
3535
}

src/Node/ReturnStatement.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
use PhpParser\Node;
66
use PhpParser\Node\Stmt\Return_;
7-
use PHPStan\Analyser\Scope;
7+
use PHPStan\Analyser\MutatingScope;
88

99
/**
1010
* @api
@@ -14,12 +14,12 @@ final class ReturnStatement
1414

1515
private Node\Stmt\Return_ $returnNode;
1616

17-
public function __construct(private Scope $scope, Return_ $returnNode)
17+
public function __construct(private MutatingScope $scope, Return_ $returnNode)
1818
{
1919
$this->returnNode = $returnNode;
2020
}
2121

22-
public function getScope(): Scope
22+
public function getScope(): MutatingScope
2323
{
2424
return $this->scope;
2525
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug14001;
4+
5+
use PHPStan\Analyser\StatementResult;
6+
use PHPStan\Node\ExecutionEndNode;
7+
use function PHPStan\Testing\assertType;
8+
9+
class Foo
10+
{
11+
/**
12+
* @param list<ExecutionEndNode> $executionEnds
13+
*/
14+
public function testWithEarlyTermination(array $executionEnds): void
15+
{
16+
$finalScope = null;
17+
foreach ($executionEnds as $executionEnd) {
18+
$statementResult = $executionEnd->getStatementResult();
19+
if ($statementResult->isAlwaysTerminating()) {
20+
continue;
21+
}
22+
if ($finalScope === null) {
23+
$finalScope = $statementResult->getScope();
24+
continue;
25+
}
26+
27+
$finalScope = $finalScope->mergeWith($statementResult->getScope());
28+
}
29+
assertType('PHPStan\Analyser\MutatingScope|null', $finalScope);
30+
}
31+
32+
/**
33+
* @param list<ExecutionEndNode> $executionEnds
34+
*/
35+
public function testWithoutEarlyTermination(array $executionEnds): void
36+
{
37+
$finalScope = null;
38+
foreach ($executionEnds as $executionEnd) {
39+
$endScope = $executionEnd->getStatementResult()->getScope();
40+
if ($finalScope === null) {
41+
$finalScope = $endScope;
42+
continue;
43+
}
44+
45+
$finalScope = $finalScope->mergeWith($endScope);
46+
}
47+
assertType('PHPStan\Analyser\MutatingScope|null', $finalScope);
48+
}
49+
}

0 commit comments

Comments
 (0)