Skip to content

Commit 41f6d1c

Browse files
committed
Fix "list type lost in for loop"
1 parent 806f1a1 commit 41f6d1c

File tree

3 files changed

+104
-10
lines changed

3 files changed

+104
-10
lines changed

src/Analyser/NodeScopeResolver.php

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1487,6 +1487,8 @@ private function processStmtNode(
14871487
$initScope = $condResult->getScope();
14881488
$condResultScope = $condResult->getScope();
14891489

1490+
// only the last condition expression is relevant whether the loop continues
1491+
// see https://www.php.net/manual/en/control-structures.for.php
14901492
if ($condExpr === $lastCondExpr) {
14911493
$condTruthiness = ($this->treatPhpDocTypesAsCertain ? $condResultScope->getType($condExpr) : $condResultScope->getNativeType($condExpr))->toBoolean();
14921494
$isIterableAtLeastOnce = $isIterableAtLeastOnce->and($condTruthiness->isTrue());
@@ -1513,6 +1515,7 @@ private function processStmtNode(
15131515
foreach ($bodyScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
15141516
$bodyScope = $bodyScope->mergeWith($continueExitPoint->getScope());
15151517
}
1518+
15161519
foreach ($stmt->loop as $loopExpr) {
15171520
$exprResult = $this->processExprNode($stmt, $loopExpr, $bodyScope, static function (): void {
15181521
}, ExpressionContext::createTopLevel());
@@ -1539,6 +1542,7 @@ private function processStmtNode(
15391542
if ($lastCondExpr !== null) {
15401543
$alwaysIterates = $alwaysIterates->and($bodyScope->getType($lastCondExpr)->toBoolean()->isTrue());
15411544
$bodyScope = $this->processExprNode($stmt, $lastCondExpr, $bodyScope, $nodeCallback, ExpressionContext::createDeep())->getTruthyScope();
1545+
$bodyScope = $this->inferForLoopExpressions($stmt, $lastCondExpr, $bodyScope);
15421546
}
15431547

15441548
$finalScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $bodyScope, $nodeCallback, $context)->filterOutLoopExitPoints();
@@ -7116,4 +7120,66 @@ public function getFilteringExprForMatchArm(Expr\Match_ $expr, array $conditions
71167120
);
71177121
}
71187122

7123+
private function inferForLoopExpressions(For_ $stmt, Expr $lastCondExpr, MutatingScope $bodyScope): MutatingScope
7124+
{
7125+
// infer $items[$i] type from for ($i = 0; $i < count($items); $i++) {...}
7126+
7127+
if (
7128+
// $i = 0
7129+
count($stmt->init) === 1
7130+
&& $stmt->init[0] instanceof Assign
7131+
&& $stmt->init[0]->var instanceof Variable
7132+
&& $stmt->init[0]->expr instanceof Node\Scalar\Int_
7133+
&& $stmt->init[0]->expr->value === 0
7134+
// $i++ or ++$i
7135+
&& count($stmt->loop) === 1
7136+
&& ($stmt->loop[0] instanceof Expr\PreInc || $stmt->loop[0] instanceof Expr\PostInc)
7137+
&& $stmt->loop[0]->var instanceof Variable
7138+
) {
7139+
// $i < count($items)
7140+
if (
7141+
$lastCondExpr instanceof BinaryOp\Smaller
7142+
&& $lastCondExpr->left instanceof Variable
7143+
&& $lastCondExpr->right instanceof FuncCall
7144+
&& $lastCondExpr->right->name instanceof Name
7145+
&& $lastCondExpr->right->name->toLowerString() === 'count'
7146+
&& count($lastCondExpr->right->getArgs()) > 0
7147+
&& $lastCondExpr->right->getArgs()[0]->value instanceof Variable
7148+
&& is_string($stmt->init[0]->var->name)
7149+
&& $stmt->init[0]->var->name === $stmt->loop[0]->var->name
7150+
&& $stmt->init[0]->var->name === $lastCondExpr->left->name
7151+
) {
7152+
$arrayArg = $lastCondExpr->right->getArgs()[0]->value;
7153+
$bodyScope = $bodyScope->assignExpression(
7154+
new ArrayDimFetch($lastCondExpr->right->getArgs()[0]->value, $lastCondExpr->left),
7155+
$bodyScope->getType($arrayArg)->getIterableValueType(),
7156+
$bodyScope->getNativeType($arrayArg)->getIterableValueType(),
7157+
);
7158+
}
7159+
7160+
// count($items) > $i
7161+
if (
7162+
$lastCondExpr instanceof BinaryOp\Greater
7163+
&& $lastCondExpr->right instanceof Variable
7164+
&& $lastCondExpr->left instanceof FuncCall
7165+
&& $lastCondExpr->left->name instanceof Name
7166+
&& $lastCondExpr->left->name->toLowerString() === 'count'
7167+
&& count($lastCondExpr->left->getArgs()) > 0
7168+
&& $lastCondExpr->left->getArgs()[0]->value instanceof Variable
7169+
&& is_string($stmt->init[0]->var->name)
7170+
&& $stmt->init[0]->var->name === $stmt->loop[0]->var->name
7171+
&& $stmt->init[0]->var->name === $lastCondExpr->right->name
7172+
) {
7173+
$arrayArg = $lastCondExpr->left->getArgs()[0]->value;
7174+
$bodyScope = $bodyScope->assignExpression(
7175+
new ArrayDimFetch($lastCondExpr->left->getArgs()[0]->value, $lastCondExpr->right),
7176+
$bodyScope->getType($arrayArg)->getIterableValueType(),
7177+
$bodyScope->getNativeType($arrayArg)->getIterableValueType(),
7178+
);
7179+
}
7180+
}
7181+
7182+
return $bodyScope;
7183+
}
7184+
71197185
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace Bug12807;
4+
5+
use function PHPStan\debugScope;
6+
use function PHPStan\Testing\assertType;
7+
8+
/**
9+
* @param non-empty-list<int> $items
10+
*
11+
* @return non-empty-list<int>
12+
*/
13+
/*
14+
function getItemsWithForLoop(array $items): array
15+
{
16+
for ($i = 0; $i < count($items); $i++) {
17+
$items[$i] = 1;
18+
}
19+
20+
assertType('non-empty-list<int>', $items);
21+
return $items;
22+
}*/
23+
24+
/**
25+
* @param non-empty-list<string> $items
26+
*
27+
* @return non-empty-list<string>
28+
*/
29+
function getItemsWithForLoopInvertLastCond(array $items): array
30+
{
31+
for ($i = 0; count($items) > $i; ++$i) {
32+
$items[$i] = 'hello';
33+
}
34+
35+
assertType('non-empty-list<string>', $items);
36+
return $items;
37+
}

tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -867,16 +867,7 @@ public function testBug12406b(): void
867867
{
868868
$this->reportPossiblyNonexistentGeneralArrayOffset = true;
869869

870-
$this->analyse([__DIR__ . '/data/bug-12406b.php'], [
871-
[
872-
'Offset int<0, max> might not exist on non-empty-list<array{0: string, 1: non-empty-string, 2: non-falsy-string, 3: numeric-string, 4?: numeric-string, 5?: numeric-string}>.',
873-
22,
874-
],
875-
[
876-
'Offset int<0, max> might not exist on non-empty-list<array{0: string, 1: non-empty-string, 2: non-falsy-string, 3: numeric-string, 4?: numeric-string, 5?: numeric-string}>.',
877-
23,
878-
],
879-
]);
870+
$this->analyse([__DIR__ . '/data/bug-12406b.php'], []);
880871
}
881872

882873
public function testBug11679(): void

0 commit comments

Comments
 (0)