Skip to content

Commit b3f4336

Browse files
committed
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 fa4e7d2 commit b3f4336

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
@@ -347,6 +347,7 @@ public function processAssignVar(
347347
$var === $originalVar
348348
&& $var->dim !== null
349349
&& $scope->hasExpressionType($var)->yes()
350+
&& !$scope->isExpressionTrackingOnly($var)
350351
) {
351352
$assignedPropertyExpr = new SetExistingOffsetValueTypeExpr(
352353
$varForSetOffsetValue,
@@ -1067,7 +1068,7 @@ private function processArrayByRefItems(MutatingScope $scope, string $rootVarNam
10671068
*
10681069
* @return array{Type, list<array{Expr, Type}>}
10691070
*/
1070-
private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, array $offsetTypes, Type $offsetValueType, Type $valueToWrite, Scope $scope): array
1071+
private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, array $offsetTypes, Type $offsetValueType, Type $valueToWrite, MutatingScope $scope): array
10711072
{
10721073
$originalValueToWrite = $valueToWrite;
10731074

@@ -1080,13 +1081,13 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar
10801081
} else {
10811082
$has = $offsetValueType->hasOffsetValueType($offsetType);
10821083
if ($has->yes()) {
1083-
if ($scope->hasExpressionType($dimFetch)->yes()) {
1084+
if ($scope->hasExpressionType($dimFetch)->yes() && !$scope->isExpressionTrackingOnly($dimFetch)) {
10841085
$offsetValueType = $scope->getType($dimFetch);
10851086
} else {
10861087
$offsetValueType = $offsetValueType->getOffsetValueType($offsetType);
10871088
}
10881089
} elseif ($has->maybe()) {
1089-
if ($scope->hasExpressionType($dimFetch)->yes()) {
1090+
if ($scope->hasExpressionType($dimFetch)->yes() && !$scope->isExpressionTrackingOnly($dimFetch)) {
10901091
$generalizeOnWrite = false;
10911092
$offsetValueType = $scope->getType($dimFetch);
10921093
} else {
@@ -1122,6 +1123,7 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar
11221123
$offsetType !== null
11231124
&& $arrayDimFetch !== null
11241125
&& $scope->hasExpressionType($arrayDimFetch)->yes()
1126+
&& !$scope->isExpressionTrackingOnly($arrayDimFetch)
11251127
&& !$offsetValueType->hasOffsetValueType($offsetType)->no()
11261128
) {
11271129
$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
@@ -972,6 +972,7 @@ private function resolveType(string $exprString, Expr $node): Type
972972
&& !$node instanceof Expr\Closure
973973
&& !$node instanceof Expr\ArrowFunction
974974
&& $this->hasExpressionType($node)->yes()
975+
&& !$this->expressionTypes[$exprString]->isTrackingOnly()
975976
) {
976977
return $this->expressionTypes[$exprString]->getType();
977978
}
@@ -1302,6 +1303,15 @@ public function hasExpressionType(Expr $node): TrinaryLogic
13021303
return $this->expressionTypes[$exprString]->getCertainty();
13031304
}
13041305

1306+
public function isExpressionTrackingOnly(Expr $node): bool
1307+
{
1308+
$exprString = $this->getNodeKey($node);
1309+
if (!isset($this->expressionTypes[$exprString])) {
1310+
return false;
1311+
}
1312+
return $this->expressionTypes[$exprString]->isTrackingOnly();
1313+
}
1314+
13051315
/**
13061316
* @param MethodReflection|FunctionReflection|null $reflection
13071317
*/
@@ -4087,12 +4097,35 @@ private function generalizeVariableTypeHolders(
40874097
$generalizedExpressions = [];
40884098
$newVariableTypeHolders = [];
40894099
foreach ($variableTypeHolders as $variableExprString => $variableTypeHolder) {
4100+
$invalidatedByGeneralization = false;
40904101
foreach ($generalizedExpressions as $generalizedExprString => $generalizedExpr) {
40914102
if (!$this->shouldInvalidateExpression($generalizedExprString, $generalizedExpr, $variableTypeHolder->getExpr(), $variableExprString)) {
40924103
continue;
40934104
}
40944105

4095-
continue 2;
4106+
$invalidatedByGeneralization = true;
4107+
break;
4108+
}
4109+
4110+
if ($invalidatedByGeneralization) {
4111+
$expr = $variableTypeHolder->getExpr();
4112+
if (
4113+
$expr instanceof Expr\ArrayDimFetch
4114+
&& $expr->dim !== null
4115+
&& $variableTypeHolder->getCertainty()->yes()
4116+
&& isset($otherVariableTypeHolders[$variableExprString])
4117+
&& $otherVariableTypeHolders[$variableExprString]->getCertainty()->yes()
4118+
) {
4119+
$newVariableTypeHolders[$variableExprString] = new ExpressionTypeHolder(
4120+
$expr,
4121+
$variableTypeHolder->getType(),
4122+
$variableTypeHolder->getCertainty(),
4123+
true,
4124+
);
4125+
continue;
4126+
}
4127+
4128+
continue;
40964129
}
40974130
if (!isset($otherVariableTypeHolders[$variableExprString])) {
40984131
$newVariableTypeHolders[$variableExprString] = $variableTypeHolder;
@@ -4109,6 +4142,7 @@ private function generalizeVariableTypeHolders(
41094142
$variableTypeHolder->getExpr(),
41104143
$generalizedType,
41114144
$variableTypeHolder->getCertainty(),
4145+
$variableTypeHolder->isTrackingOnly(),
41124146
);
41134147
}
41144148

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1343,4 +1343,11 @@ public function testArrayFindKeyExisting(): void
13431343
]);
13441344
}
13451345

1346+
public function testBug14598(): void
1347+
{
1348+
$this->reportPossiblyNonexistentConstantArrayOffset = true;
1349+
1350+
$this->analyse([__DIR__ . '/data/bug-14598.php'], []);
1351+
}
1352+
13461353
}
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)