Skip to content

Fix phpstan/phpstan#14245: list lost after overwriting last element #5169

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
staabm merged 9 commits into from
Mar 10, 2026
Merged

Fix phpstan/phpstan#14245: list lost after overwriting last element #5169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cc9deae
Preserve list type when overwriting existing elements via count()-1, …
phpstan-bot Mar 9, 2026
31f0c2a
improve tests
staabm Mar 9, 2026
8ee03f7
more tests
staabm Mar 9, 2026
140d391
extract method
staabm Mar 10, 2026
7126910
remove ai leftover
staabm Mar 10, 2026
9a5f88d
same for sizeof
staabm Mar 10, 2026
7c97da4
array_search() keeps list
staabm Mar 10, 2026
39d803d
test huge list
staabm Mar 10, 2026
5c070ed
test array_key_exists() keeps list
staabm Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 63 additions & 16 deletions src/Analyser/ExprHandler/AssignHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -987,25 +988,11 @@
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 = [];
Expand All @@ -1029,4 +1016,64 @@
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()

Check warning on line 1026 in src/Analyser/ExprHandler/AssignHandler.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 				$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()
+				&& !$scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->right))->no()
 			) {
 				return true;
 			} elseif ( // keep list for $list[1 + $index] assignments

Check warning on line 1026 in src/Analyser/ExprHandler/AssignHandler.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 				$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()
+				&& !$scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->right))->no()
 			) {
 				return true;
 			} elseif ( // keep list for $list[1 + $index] assignments
) {
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()

Check warning on line 1033 in src/Analyser/ExprHandler/AssignHandler.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 				$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()
+				&& !$scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->left))->no()
 			) {
 				return true;
 			}

Check warning on line 1033 in src/Analyser/ExprHandler/AssignHandler.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 				$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()
+				&& !$scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->left))->no()
 			) {
 				return true;
 			}
) {
return true;
}
} elseif ( // keep list for $list[count($list) - n] assignments
Copy link
Contributor

@VincentLanglet VincentLanglet Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cant we risk having n too big and set the offset -1 ?

Copy link
Contributor

@staabm staabm Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the logic here will only add non-empty-list to a pre-existing maybe already degraded because too huge array, I don't see how a too big "n" value can be a problem here.

added a test with a huge array and a big offset

$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()

Check warning on line 1046 in src/Analyser/ExprHandler/AssignHandler.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 			&& 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()
+			&& !$offsetValueType->isIterableAtLeastOnce()->no()
 		) {
 			return true;
 		} elseif ( // keep list for $list[array_key_last($list)] and $list[array_key_first($list)] assignments

Check warning on line 1046 in src/Analyser/ExprHandler/AssignHandler.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 			&& 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()
+			&& !$offsetValueType->isIterableAtLeastOnce()->no()
 		) {
 			return true;
 		} elseif ( // keep list for $list[array_key_last($list)] and $list[array_key_first($list)] assignments
) {
return true;
} elseif ( // keep list for $list[array_key_last($list)] and $list[array_key_first($list)] assignments
Copy link
Contributor

@VincentLanglet VincentLanglet Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Array_search works too

Copy link
Contributor

@staabm staabm Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added 👍

$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;
}

}
148 changes: 148 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-14245.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
<?php declare(strict_types = 1);

namespace Bug14245;

use function array_key_exists;
use function array_key_first;
use function array_key_last;
use function PHPStan\Testing\assertType;

/**
* @return list<int>
*/
function foo(): array {
return [];
}

function doFoo(): void {
$list = foo();
$count = count($list);
assertType('list<int>', $list);
if ($count > 0) {
assertType('non-empty-list<int>', $list);
$list[count($list) - 1] = 37;
assertType('non-empty-list<int>', $list);
}

assertType('list<int>', $list);
}

function doFoo2(): void {
$list = foo();
$count = count($list);
assertType('list<int>', $list);
if ($count > 0) {
assertType('non-empty-list<int>', $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<-4, max>, int>', $list);
}

assertType('array<int<-4, max>, int>', $list);
}

function listKnownSize(): void {
$list = foo();
assertType('list<int>', $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<int>', $list);
}

function listKnownHugeSize(): void {
$list = foo();
assertType('list<int>', $list);
if (count($list) === 50000) {
assertType('non-empty-list<int>', $list);
$list[count($list) - 3000] = 37;
assertType('non-empty-array<int<-2999, max>, int>', $list);
}

assertType('array<int<-2999, max>, int>', $list);
}

function overwriteKeyLast(): void {
$list = foo();
$count = count($list);
assertType('list<int>', $list);
if ($count > 0) {
assertType('non-empty-list<int>', $list);
$list[array_key_last($list)] = 37;
assertType('non-empty-list<int>', $list);
}

assertType('list<int>', $list);
}

function overwriteKeyFirst(): void {
$list = foo();
$count = count($list);
assertType('list<int>', $list);
if ($count > 0) {
assertType('non-empty-list<int>', $list);
$list[array_key_first($list)] = 37;
assertType('non-empty-list<int>', $list);
}

assertType('list<int>', $list);
}

function overwriteKeyFirstMaybeEmptyArray(): void {
$list = foo();
assertType('list<int>', $list);
// empty list might return NULL for array_key_first()
$list[array_key_first($list)] = 37;
assertType('non-empty-list<int>', $list);
}

function keyDifferentArray(array $arr): void {
$list = foo();
assertType('list<int>', $list);
$list[array_key_first($arr)] = 37;
assertType('non-empty-array<int|string, int>', $list);
}

function overwriteArraySearch($needle): void {
$list = foo();

assertType('list<int>', $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<int>', $list);
}

function overwriteArraySearchStrict($needle): void {
$list = foo();

assertType('list<int>', $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<int>', $list);
}

function ArraySearchWithDifferentArray($array2, $needle): void {
$list = foo();

assertType('list<int>', $list);
$list[array_search($needle, $array2, true)] = 37;
assertType('non-empty-array<int|string, int>', $list);
}

function ArrayKeyExistsKeepsList($needle): void {
$list = foo();

assertType('list<int>', $list);
if (array_key_exists($needle, $list)) {
$list[$needle] = 37;
}
assertType('list<int>', $list);
}
Loading