diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 5d2a084ec04..a6e1401ce7b 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -347,6 +347,7 @@ public function processAssignVar( $var === $originalVar && $var->dim !== null && $scope->hasExpressionType($var)->yes() + && !$scope->isExpressionTrackingOnly($var) ) { $assignedPropertyExpr = new SetExistingOffsetValueTypeExpr( $varForSetOffsetValue, @@ -1067,7 +1068,7 @@ private function processArrayByRefItems(MutatingScope $scope, string $rootVarNam * * @return array{Type, list} */ - private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, array $offsetTypes, Type $offsetValueType, Type $valueToWrite, Scope $scope): array + private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, array $offsetTypes, Type $offsetValueType, Type $valueToWrite, MutatingScope $scope): array { $originalValueToWrite = $valueToWrite; @@ -1080,13 +1081,13 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar } else { $has = $offsetValueType->hasOffsetValueType($offsetType); if ($has->yes()) { - if ($scope->hasExpressionType($dimFetch)->yes()) { + if ($scope->hasExpressionType($dimFetch)->yes() && !$scope->isExpressionTrackingOnly($dimFetch)) { $offsetValueType = $scope->getType($dimFetch); } else { $offsetValueType = $offsetValueType->getOffsetValueType($offsetType); } } elseif ($has->maybe()) { - if ($scope->hasExpressionType($dimFetch)->yes()) { + if ($scope->hasExpressionType($dimFetch)->yes() && !$scope->isExpressionTrackingOnly($dimFetch)) { $generalizeOnWrite = false; $offsetValueType = $scope->getType($dimFetch); } else { @@ -1122,6 +1123,7 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar $offsetType !== null && $arrayDimFetch !== null && $scope->hasExpressionType($arrayDimFetch)->yes() + && !$scope->isExpressionTrackingOnly($arrayDimFetch) && !$offsetValueType->hasOffsetValueType($offsetType)->no() ) { $hasOffsetType = null; diff --git a/src/Analyser/ExpressionTypeHolder.php b/src/Analyser/ExpressionTypeHolder.php index 7477dbf3dcd..9d44db8621a 100644 --- a/src/Analyser/ExpressionTypeHolder.php +++ b/src/Analyser/ExpressionTypeHolder.php @@ -14,6 +14,7 @@ public function __construct( private readonly Expr $expr, private readonly Type $type, private readonly TrinaryLogic $certainty, + private readonly bool $trackingOnly = false, ) { } @@ -52,22 +53,31 @@ public function equals(self $other): bool public function and(self $other): self { + $newTrackingOnly = $this->trackingOnly || $other->trackingOnly; + if ($this->type === $other->type || $this->type->equals($other->type)) { - if ($this->certainty->and($other->certainty)->yes()) { + $newCertainty = $this->certainty->and($other->certainty); + if ($newCertainty->yes() && !$newTrackingOnly) { return $this; } - if ($this->certainty->maybe()) { + if ($this->certainty->maybe() && $this->trackingOnly === $newTrackingOnly) { return $this; } - return $other; + return new self( + $this->expr, + $this->type, + $newCertainty, + $newTrackingOnly, + ); } return new self( $this->expr, TypeCombinator::union($this->type, $other->type), $this->certainty->and($other->certainty), + $newTrackingOnly, ); } @@ -86,4 +96,9 @@ public function getCertainty(): TrinaryLogic return $this->certainty; } + public function isTrackingOnly(): bool + { + return $this->trackingOnly; + } + } diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 12cf17a9bf4..ef86d9dbc93 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -972,6 +972,7 @@ private function resolveType(string $exprString, Expr $node): Type && !$node instanceof Expr\Closure && !$node instanceof Expr\ArrowFunction && $this->hasExpressionType($node)->yes() + && !$this->expressionTypes[$exprString]->isTrackingOnly() ) { return $this->expressionTypes[$exprString]->getType(); } @@ -1302,6 +1303,15 @@ public function hasExpressionType(Expr $node): TrinaryLogic return $this->expressionTypes[$exprString]->getCertainty(); } + public function isExpressionTrackingOnly(Expr $node): bool + { + $exprString = $this->getNodeKey($node); + if (!isset($this->expressionTypes[$exprString])) { + return false; + } + return $this->expressionTypes[$exprString]->isTrackingOnly(); + } + /** * @param MethodReflection|FunctionReflection|null $reflection */ @@ -4087,12 +4097,35 @@ private function generalizeVariableTypeHolders( $generalizedExpressions = []; $newVariableTypeHolders = []; foreach ($variableTypeHolders as $variableExprString => $variableTypeHolder) { + $invalidatedByGeneralization = false; foreach ($generalizedExpressions as $generalizedExprString => $generalizedExpr) { if (!$this->shouldInvalidateExpression($generalizedExprString, $generalizedExpr, $variableTypeHolder->getExpr(), $variableExprString)) { continue; } - continue 2; + $invalidatedByGeneralization = true; + break; + } + + if ($invalidatedByGeneralization) { + $expr = $variableTypeHolder->getExpr(); + if ( + $expr instanceof Expr\ArrayDimFetch + && $expr->dim !== null + && $variableTypeHolder->getCertainty()->yes() + && isset($otherVariableTypeHolders[$variableExprString]) + && $otherVariableTypeHolders[$variableExprString]->getCertainty()->yes() + ) { + $newVariableTypeHolders[$variableExprString] = new ExpressionTypeHolder( + $expr, + $variableTypeHolder->getType(), + $variableTypeHolder->getCertainty(), + true, + ); + continue; + } + + continue; } if (!isset($otherVariableTypeHolders[$variableExprString])) { $newVariableTypeHolders[$variableExprString] = $variableTypeHolder; @@ -4109,6 +4142,7 @@ private function generalizeVariableTypeHolders( $variableTypeHolder->getExpr(), $generalizedType, $variableTypeHolder->getCertainty(), + $variableTypeHolder->isTrackingOnly(), ); } diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index 024f71bbcb9..8d0f7c4db3e 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -1343,4 +1343,11 @@ public function testArrayFindKeyExisting(): void ]); } + public function testBug14598(): void + { + $this->reportPossiblyNonexistentConstantArrayOffset = true; + + $this->analyse([__DIR__ . '/data/bug-14598.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Arrays/data/bug-14598.php b/tests/PHPStan/Rules/Arrays/data/bug-14598.php new file mode 100644 index 00000000000..0e7f9b01542 --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/data/bug-14598.php @@ -0,0 +1,64 @@ +, 'off'|'on'>> $raw + * @return array<'A'|'B'|'C', string> + */ +function buildData(array $raw): array +{ + $return = []; + foreach ($raw as $id => $sensors) { + $tmp[$id] = []; + $last = "off"; + foreach ($sensors as $i => $stat) { + if ($last !== $stat) { + $tmp[$id][] = sprintf("%02d", $i); + $last = $stat; + } + } + $return[$id] = count($tmp[$id]) + ? implode(",", $tmp[$id]) + : "invalid"; + } + + return $return; +} + +/** + * @param array<'A'|'B'|'C', array> $raw + */ +function simpleNestedForeach(array $raw): void +{ + $tmp = []; + foreach ($raw as $id => $sensors) { + $tmp[$id] = []; + foreach ($sensors as $i => $stat) { + if ($i > 0) { + $tmp[$id][] = $stat; + } + } + echo count($tmp[$id]); + } +} + +/** + * @param list<'A'|'B'|'C'> $keys + * @param array $values + */ +function nestedWhileLoop(array $keys, array $values): void +{ + $tmp = []; + foreach ($keys as $id) { + $tmp[$id] = []; + $i = 0; + while ($i < count($values)) { + if ($values[$i] > 0) { + $tmp[$id][] = $values[$i]; + } + $i++; + } + echo count($tmp[$id]); + } +}