Skip to content

Commit a5aa2ce

Browse files
authored
Fix list type lost in for loop
1 parent 32d4998 commit a5aa2ce

File tree

3 files changed

+119
-10
lines changed

3 files changed

+119
-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 @@ private function getFilteringExprForMatchArm(Expr\Match_ $expr, array $condition
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: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
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+
function getItemsWithForLoop(array $items): array
14+
{
15+
for ($i = 0; $i < count($items); $i++) {
16+
$items[$i] = 1;
17+
}
18+
19+
assertType('non-empty-list<int>', $items);
20+
return $items;
21+
}
22+
23+
/**
24+
* @param list<string> $items
25+
*
26+
* @return list<string>
27+
*/
28+
function getItemsWithForLoopInvertLastCond(array $items): array
29+
{
30+
for ($i = 0; count($items) > $i; ++$i) {
31+
$items[$i] = 'hello';
32+
}
33+
34+
assertType('list<string>', $items);
35+
return $items;
36+
}
37+
38+
39+
/**
40+
* @param array<string> $items
41+
*
42+
* @return array<string>
43+
*/
44+
function getItemsArray(array $items): array
45+
{
46+
for ($i = 0; count($items) > $i; ++$i) {
47+
$items[$i] = 'hello';
48+
}
49+
50+
assertType('array<string>', $items);
51+
return $items;
52+
}

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)