diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 89b53c46c2..284ec47619 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -53,6 +53,7 @@ use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\ConstantTypeHelper; use PHPStan\Type\ErrorType; +use PHPStan\Type\IntegerRangeType; use PHPStan\Type\MixedType; use PHPStan\Type\ObjectType; use PHPStan\Type\StaticTypeFactory; @@ -987,25 +988,11 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar continue; } - if (!$arrayDimFetch->dim instanceof Expr\BinaryOp\Plus) { + if (!$this->shouldKeepList($arrayDimFetch, $scope, $offsetValueType)) { continue; } - if ( // keep list for $list[$index + 1] assignments - $arrayDimFetch->dim->right instanceof Variable - && $arrayDimFetch->dim->left instanceof Node\Scalar\Int_ - && $arrayDimFetch->dim->left->value === 1 - && $scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->right))->yes() - ) { - $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); - } elseif ( // keep list for $list[1 + $index] assignments - $arrayDimFetch->dim->left instanceof Variable - && $arrayDimFetch->dim->right instanceof Node\Scalar\Int_ - && $arrayDimFetch->dim->right->value === 1 - && $scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->left))->yes() - ) { - $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); - } + $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); } $additionalExpressions = []; @@ -1029,4 +1016,64 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar return [$valueToWrite, $additionalExpressions]; } + private function shouldKeepList(ArrayDimFetch $arrayDimFetch, Scope $scope, Type $offsetValueType): bool + { + if ($arrayDimFetch->dim instanceof Expr\BinaryOp\Plus) { + if ( // keep list for $list[$index + 1] assignments + $arrayDimFetch->dim->right instanceof Variable + && $arrayDimFetch->dim->left instanceof Node\Scalar\Int_ + && $arrayDimFetch->dim->left->value === 1 + && $scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->right))->yes() + ) { + return true; + } elseif ( // keep list for $list[1 + $index] assignments + $arrayDimFetch->dim->left instanceof Variable + && $arrayDimFetch->dim->right instanceof Node\Scalar\Int_ + && $arrayDimFetch->dim->right->value === 1 + && $scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->left))->yes() + ) { + return true; + } + } elseif ( // keep list for $list[count($list) - n] assignments + $arrayDimFetch->dim instanceof Expr\BinaryOp\Minus + && $arrayDimFetch->dim->right instanceof Node\Scalar\Int_ + && $arrayDimFetch->dim->left instanceof Expr\FuncCall + && $arrayDimFetch->dim->left->name instanceof Name + && in_array($arrayDimFetch->dim->left->name->toLowerString(), ['count', 'sizeof'], true) + && count($arrayDimFetch->dim->left->getArgs()) === 1 // could support COUNT_RECURSIVE, COUNT_NORMAL + && $this->isSameVariable($arrayDimFetch->var, $arrayDimFetch->dim->left->getArgs()[0]->value) + && IntegerRangeType::fromInterval(0, null)->isSuperTypeOf($scope->getType($arrayDimFetch->dim))->yes() + && $offsetValueType->isIterableAtLeastOnce()->yes() + ) { + return true; + } elseif ( // keep list for $list[array_key_last($list)] and $list[array_key_first($list)] assignments + $arrayDimFetch->dim instanceof Expr\FuncCall + && $arrayDimFetch->dim->name instanceof Name + && in_array($arrayDimFetch->dim->name->toLowerString(), ['array_key_last', 'array_key_first'], true) + && count($arrayDimFetch->dim->getArgs()) >= 1 + && $this->isSameVariable($arrayDimFetch->var, $arrayDimFetch->dim->getArgs()[0]->value) + ) { + return true; + } elseif ( // keep list for $list[array_search($needle, $list)] assignments + $arrayDimFetch->dim instanceof Expr\FuncCall + && $arrayDimFetch->dim->name instanceof Name + && $arrayDimFetch->dim->name->toLowerString() === 'array_search' + && count($arrayDimFetch->dim->getArgs()) >= 1 + && $this->isSameVariable($arrayDimFetch->var, $arrayDimFetch->dim->getArgs()[1]->value) + ) { + return true; + } + + return false; + } + + private function isSameVariable(Expr $a, Expr $b): bool + { + if ($a instanceof Variable && $b instanceof Variable && is_string($a->name) && is_string($b->name)) { + return $a->name === $b->name; + } + + return false; + } + } diff --git a/tests/PHPStan/Analyser/nsrt/bug-14245.php b/tests/PHPStan/Analyser/nsrt/bug-14245.php new file mode 100644 index 0000000000..63333b5de2 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14245.php @@ -0,0 +1,148 @@ + + */ +function foo(): array { + return []; +} + +function doFoo(): void { + $list = foo(); + $count = count($list); + assertType('list', $list); + if ($count > 0) { + assertType('non-empty-list', $list); + $list[count($list) - 1] = 37; + assertType('non-empty-list', $list); + } + + assertType('list', $list); +} + +function doFoo2(): void { + $list = foo(); + $count = count($list); + assertType('list', $list); + if ($count > 0) { + assertType('non-empty-list', $list); + // we don't know the $list length, + // therefore count() - N might be before the first element -> degrade to array + $list[count($list) - 5] = 37; + assertType('non-empty-array, int>', $list); + } + + assertType('array, int>', $list); +} + +function listKnownSize(): void { + $list = foo(); + assertType('list', $list); + if (count($list) === 5) { + assertType('array{int, int, int, int, int}', $list); + $list[count($list) - 3] = 37; + assertType('array{int, int, 37, int, int}', $list); + } + + assertType('list', $list); +} + +function listKnownHugeSize(): void { + $list = foo(); + assertType('list', $list); + if (count($list) === 50000) { + assertType('non-empty-list', $list); + $list[count($list) - 3000] = 37; + assertType('non-empty-array, int>', $list); + } + + assertType('array, int>', $list); +} + +function overwriteKeyLast(): void { + $list = foo(); + $count = count($list); + assertType('list', $list); + if ($count > 0) { + assertType('non-empty-list', $list); + $list[array_key_last($list)] = 37; + assertType('non-empty-list', $list); + } + + assertType('list', $list); +} + +function overwriteKeyFirst(): void { + $list = foo(); + $count = count($list); + assertType('list', $list); + if ($count > 0) { + assertType('non-empty-list', $list); + $list[array_key_first($list)] = 37; + assertType('non-empty-list', $list); + } + + assertType('list', $list); +} + +function overwriteKeyFirstMaybeEmptyArray(): void { + $list = foo(); + assertType('list', $list); + // empty list might return NULL for array_key_first() + $list[array_key_first($list)] = 37; + assertType('non-empty-list', $list); +} + +function keyDifferentArray(array $arr): void { + $list = foo(); + assertType('list', $list); + $list[array_key_first($arr)] = 37; + assertType('non-empty-array', $list); +} + +function overwriteArraySearch($needle): void { + $list = foo(); + + assertType('list', $list); + // search in empty-array, or with a non-existent key will return false, + // which gets auto-casted to 0, so we still have a list + // https://3v4l.org/RZbOK + $list[array_search($needle, $list)] = 37; + assertType('non-empty-list', $list); +} + +function overwriteArraySearchStrict($needle): void { + $list = foo(); + + assertType('list', $list); + // search in empty-array, or with a non-existent key will return false, + // which gets auto-casted to 0, so we still have a list + // https://3v4l.org/RZbOK + $list[array_search($needle, $list, true)] = 37; + assertType('non-empty-list', $list); +} + +function ArraySearchWithDifferentArray($array2, $needle): void { + $list = foo(); + + assertType('list', $list); + $list[array_search($needle, $array2, true)] = 37; + assertType('non-empty-array', $list); +} + +function ArrayKeyExistsKeepsList($needle): void { + $list = foo(); + + assertType('list', $list); + if (array_key_exists($needle, $list)) { + $list[$needle] = 37; + } + assertType('list', $list); +}