Skip to content

Commit ed4ebed

Browse files
phpstan-botVincentLanglet
authored andcommitted
Preserve ArrayDimFetch expression tracking through loop generalization
When a parent expression is generalized during loop fixpoint iteration, child ArrayDimFetch expressions are now preserved with a trackingOnly flag instead of being removed. This prevents false positive "nonexistent offset" errors on array dim fetches that were explicitly assigned earlier in the same code path. The trackingOnly flag decouples expression tracking (hasExpressionType returns Yes, so the NonexistentOffsetInArrayDimFetchRule skips the check) from type resolution (resolveType falls through to the ExprHandler to compute the type fresh from the parent, avoiding stale stored types). Closes phpstan/phpstan#14598
1 parent f4ce2af commit ed4ebed

5 files changed

Lines changed: 129 additions & 7 deletions

File tree

‎src/Analyser/ExprHandler/AssignHandler.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,7 @@ public function processAssignVar(
540540
$var === $originalVar
541541
&& $var->dim !== null
542542
&& $scope->hasExpressionType($var)->yes()
543+
&& !$scope->isExpressionTrackingOnly($var)
543544
) {
544545
$assignedPropertyExpr = new SetExistingOffsetValueTypeExpr(
545546
$varForSetOffsetValue,
@@ -1408,7 +1409,7 @@ private function processArrayByRefItems(MutatingScope $scope, string $rootVarNam
14081409
*
14091410
* @return array{Type, list<array{Expr, Type}>}
14101411
*/
1411-
private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, array $offsetTypes, Type $offsetValueType, Type $valueToWrite, Scope $scope): array
1412+
private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, array $offsetTypes, Type $offsetValueType, Type $valueToWrite, MutatingScope $scope): array
14121413
{
14131414
$originalValueToWrite = $valueToWrite;
14141415

@@ -1426,13 +1427,13 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar
14261427
} else {
14271428
$has = $offsetValueType->hasOffsetValueType($offsetType);
14281429
if ($has->yes()) {
1429-
if ($scope->hasExpressionType($dimFetch)->yes()) {
1430+
if ($scope->hasExpressionType($dimFetch)->yes() && !$scope->isExpressionTrackingOnly($dimFetch)) {
14301431
$offsetValueType = $scope->getType($dimFetch);
14311432
} else {
14321433
$offsetValueType = $offsetValueType->getOffsetValueType($offsetType);
14331434
}
14341435
} elseif ($has->maybe()) {
1435-
if ($scope->hasExpressionType($dimFetch)->yes()) {
1436+
if ($scope->hasExpressionType($dimFetch)->yes() && !$scope->isExpressionTrackingOnly($dimFetch)) {
14361437
$generalizeOnWrite = false;
14371438
$offsetValueType = $scope->getType($dimFetch);
14381439
} else {
@@ -1469,6 +1470,7 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar
14691470
$offsetType !== null
14701471
&& $arrayDimFetch !== null
14711472
&& $scope->hasExpressionType($arrayDimFetch)->yes()
1473+
&& !$scope->isExpressionTrackingOnly($arrayDimFetch)
14721474
&& !$offsetValueType->hasOffsetValueType($offsetType)->no()
14731475
) {
14741476
$hasOffsetType = null;

‎src/Analyser/ExpressionTypeHolder.php‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ public function __construct(
1414
private readonly Expr $expr,
1515
private readonly Type $type,
1616
private readonly TrinaryLogic $certainty,
17+
private readonly bool $trackingOnly = false,
1718
)
1819
{
1920
}
@@ -52,22 +53,31 @@ public function equals(self $other): bool
5253

5354
public function and(self $other): self
5455
{
56+
$newTrackingOnly = $this->trackingOnly || $other->trackingOnly;
57+
5558
if ($this->type === $other->type || $this->type->equals($other->type)) {
56-
if ($this->certainty->and($other->certainty)->yes()) {
59+
$newCertainty = $this->certainty->and($other->certainty);
60+
if ($newCertainty->yes() && !$newTrackingOnly) {
5761
return $this;
5862
}
5963

60-
if ($this->certainty->maybe()) {
64+
if ($this->certainty->maybe() && $this->trackingOnly === $newTrackingOnly) {
6165
return $this;
6266
}
6367

64-
return $other;
68+
return new self(
69+
$this->expr,
70+
$this->type,
71+
$newCertainty,
72+
$newTrackingOnly,
73+
);
6574
}
6675

6776
return new self(
6877
$this->expr,
6978
TypeCombinator::union($this->type, $other->type),
7079
$this->certainty->and($other->certainty),
80+
$newTrackingOnly,
7181
);
7282
}
7383

@@ -86,4 +96,9 @@ public function getCertainty(): TrinaryLogic
8696
return $this->certainty;
8797
}
8898

99+
public function isTrackingOnly(): bool
100+
{
101+
return $this->trackingOnly;
102+
}
103+
89104
}

‎src/Analyser/MutatingScope.php‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,7 @@ private function resolveType(string $exprString, Expr $node): Type
980980
&& !$node instanceof Expr\Closure
981981
&& !$node instanceof Expr\ArrowFunction
982982
&& $this->hasExpressionType($node)->yes()
983+
&& !$this->expressionTypes[$exprString]->isTrackingOnly()
983984
) {
984985
return $this->expressionTypes[$exprString]->getType();
985986
}
@@ -1310,6 +1311,15 @@ public function hasExpressionType(Expr $node): TrinaryLogic
13101311
return $this->expressionTypes[$exprString]->getCertainty();
13111312
}
13121313

1314+
public function isExpressionTrackingOnly(Expr $node): bool
1315+
{
1316+
$exprString = $this->getNodeKey($node);
1317+
if (!isset($this->expressionTypes[$exprString])) {
1318+
return false;
1319+
}
1320+
return $this->expressionTypes[$exprString]->isTrackingOnly();
1321+
}
1322+
13131323
/**
13141324
* @param MethodReflection|FunctionReflection|null $reflection
13151325
*/
@@ -4205,12 +4215,35 @@ private function generalizeVariableTypeHolders(
42054215
$generalizedExpressions = [];
42064216
$newVariableTypeHolders = [];
42074217
foreach ($variableTypeHolders as $variableExprString => $variableTypeHolder) {
4218+
$invalidatedByGeneralization = false;
42084219
foreach ($generalizedExpressions as $generalizedExprString => $generalizedExpr) {
42094220
if (!$this->shouldInvalidateExpression($generalizedExprString, $generalizedExpr, $variableTypeHolder->getExpr(), $variableExprString)) {
42104221
continue;
42114222
}
42124223

4213-
continue 2;
4224+
$invalidatedByGeneralization = true;
4225+
break;
4226+
}
4227+
4228+
if ($invalidatedByGeneralization) {
4229+
$expr = $variableTypeHolder->getExpr();
4230+
if (
4231+
$expr instanceof Expr\ArrayDimFetch
4232+
&& $expr->dim !== null
4233+
&& $variableTypeHolder->getCertainty()->yes()
4234+
&& isset($otherVariableTypeHolders[$variableExprString])
4235+
&& $otherVariableTypeHolders[$variableExprString]->getCertainty()->yes()
4236+
) {
4237+
$newVariableTypeHolders[$variableExprString] = new ExpressionTypeHolder(
4238+
$expr,
4239+
$variableTypeHolder->getType(),
4240+
$variableTypeHolder->getCertainty(),
4241+
true,
4242+
);
4243+
continue;
4244+
}
4245+
4246+
continue;
42144247
}
42154248
if (!isset($otherVariableTypeHolders[$variableExprString])) {
42164249
$newVariableTypeHolders[$variableExprString] = $variableTypeHolder;
@@ -4227,6 +4260,7 @@ private function generalizeVariableTypeHolders(
42274260
$variableTypeHolder->getExpr(),
42284261
$generalizedType,
42294262
$variableTypeHolder->getCertainty(),
4263+
$variableTypeHolder->isTrackingOnly(),
42304264
);
42314265
}
42324266

‎tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,6 +1348,13 @@ public function testArrayFindKeyExisting(): void
13481348
]);
13491349
}
13501350

1351+
public function testBug14598(): void
1352+
{
1353+
$this->reportPossiblyNonexistentConstantArrayOffset = true;
1354+
1355+
$this->analyse([__DIR__ . '/data/bug-14598.php'], []);
1356+
}
1357+
13511358
public function testBug13688(): void
13521359
{
13531360
$this->analyse([__DIR__ . '/data/bug-13688.php'], []);
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug14598;
4+
5+
/**
6+
* @param array<'A'|'B'|'C', array<int<0, 3>, 'off'|'on'>> $raw
7+
* @return array<'A'|'B'|'C', string>
8+
*/
9+
function buildData(array $raw): array
10+
{
11+
$return = [];
12+
foreach ($raw as $id => $sensors) {
13+
$tmp[$id] = [];
14+
$last = "off";
15+
foreach ($sensors as $i => $stat) {
16+
if ($last !== $stat) {
17+
$tmp[$id][] = sprintf("%02d", $i);
18+
$last = $stat;
19+
}
20+
}
21+
$return[$id] = count($tmp[$id])
22+
? implode(",", $tmp[$id])
23+
: "invalid";
24+
}
25+
26+
return $return;
27+
}
28+
29+
/**
30+
* @param array<'A'|'B'|'C', array<int, int>> $raw
31+
*/
32+
function simpleNestedForeach(array $raw): void
33+
{
34+
$tmp = [];
35+
foreach ($raw as $id => $sensors) {
36+
$tmp[$id] = [];
37+
foreach ($sensors as $i => $stat) {
38+
if ($i > 0) {
39+
$tmp[$id][] = $stat;
40+
}
41+
}
42+
echo count($tmp[$id]);
43+
}
44+
}
45+
46+
/**
47+
* @param list<'A'|'B'|'C'> $keys
48+
* @param array<int, int> $values
49+
*/
50+
function nestedWhileLoop(array $keys, array $values): void
51+
{
52+
$tmp = [];
53+
foreach ($keys as $id) {
54+
$tmp[$id] = [];
55+
$i = 0;
56+
while ($i < count($values)) {
57+
if ($values[$i] > 0) {
58+
$tmp[$id][] = $values[$i];
59+
}
60+
$i++;
61+
}
62+
echo count($tmp[$id]);
63+
}
64+
}

0 commit comments

Comments
 (0)