Skip to content

Don't lose known offset-types in array_replace() #4826

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

Open
staabm wants to merge 7 commits into
base: 2.1.x
Choose a base branch
from
+201 −4
Open

Don't lose known offset-types in array_replace() #4826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3464c8b
Don't lose known offset-types in array_replace()
staabm Jan 27, 2026
04d4024
more tests
staabm Jan 27, 2026
4f502b3
more tests
staabm Jan 27, 2026
18c828a
cs
staabm Jan 27, 2026
13693a7
more tests
staabm Jan 28, 2026
146c533
Update ArrayReplaceFunctionReturnTypeExtension.php
staabm Jan 28, 2026
dcdfcef
reflect new comment
staabm Jan 28, 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
2 changes: 2 additions & 0 deletions src/Type/Php/ArrayMergeFunctionDynamicReturnTypeExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection,
}
} else {
foreach ($offsetTypes as $key => [$hasOffsetValue, $offsetValueType]) {
// more precise values-types will be calculated elsewhere.
// just remember the offset key.
$offsetTypes[$key] = [
$hasOffsetValue->and(TrinaryLogic::createMaybe()),
new MixedType(),
Expand Down
70 changes: 70 additions & 0 deletions src/Type/Php/ArrayReplaceFunctionReturnTypeExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,24 @@
use PHPStan\Reflection\FunctionReflection;
use PHPStan\TrinaryLogic;
use PHPStan\Type\Accessory\AccessoryArrayListType;
use PHPStan\Type\Accessory\HasOffsetType;
use PHPStan\Type\Accessory\HasOffsetValueType;
use PHPStan\Type\Accessory\NonEmptyArrayType;
use PHPStan\Type\ArrayType;
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantArrayTypeBuilder;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\DynamicFunctionReturnTypeExtension;
use PHPStan\Type\MixedType;
use PHPStan\Type\NeverType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\TypeUtils;
use function array_keys;
use function count;
use function in_array;
use function is_string;
use function strtolower;

#[AutowiredService]
Expand Down Expand Up @@ -97,6 +102,46 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection,
return $newArrayBuilder->getArray();
}

$offsetTypes = [];
foreach ($argTypes as $argType) {
$constArrays = $argType->getConstantArrays();
if ($constArrays !== []) {
foreach ($constArrays as $constantArray) {
foreach ($constantArray->getKeyTypes() as $keyType) {
$hasOffsetValue = TrinaryLogic::createFromBoolean($argType->hasOffsetValueType($keyType)->yes());
Copy link
Contributor

@VincentLanglet VincentLanglet Jan 28, 2026

Choose a reason for hiding this comment

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

It's unclear to me reading this why you're doing

TrinaryLogic::createFromBoolean($argType->hasOffsetValueType($keyType)->yes())

and not directly

$argType->hasOffsetValueType($keyType)

You're transforming the Maybe into a No ?

Copy link
Contributor Author

@staabm staabm Jan 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

correct. I need the trinary, as I later on combine it with $hasOffsetValue->and(TrinaryLogic::createMaybe()) for non constant arrays (else case)

$offsetTypes[$keyType->getValue()] = [
Comment on lines +111 to +112
Copy link
Contributor

@VincentLanglet VincentLanglet Jan 28, 2026

Choose a reason for hiding this comment

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

If multiple constantArray has the same Key, like array{1: int, 2: string}|array{1: int, 3: string} you will call hasOffsetValueType and getOffsetValueType two times to set the same value ; not sure if we could avoid the extra calls.

Copy link
Contributor Author

@staabm staabm Jan 28, 2026

Choose a reason for hiding this comment

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

as long as this is not identified as a bottleneck I don't want to complicate it further

$hasOffsetValue,
$argType->getOffsetValueType($keyType),
];
}
}
} else {
foreach ($offsetTypes as $key => [$hasOffsetValue, $offsetValueType]) {
// more precise values-types will be calculated elsewhere.
// just remember the offset key.
$offsetTypes[$key] = [
$hasOffsetValue->and(TrinaryLogic::createMaybe()),
new MixedType(),
Copy link
Contributor

@VincentLanglet VincentLanglet Jan 28, 2026

Choose a reason for hiding this comment

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

Technically we could provide better than Mixed here.
Maybe a comment to explain there is no need to provide Union($offsetValueType, $argType->getArrayValues) since the value won't be used since we're only instantiating HasOffsetType

Copy link
Contributor Author

@staabm staabm Jan 28, 2026

Choose a reason for hiding this comment

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

added a comment

];
}
}

foreach (TypeUtils::getAccessoryTypes($argType) as $accessoryType) {
if (
!($accessoryType instanceof HasOffsetType)
&& !($accessoryType instanceof HasOffsetValueType)
) {
continue;
}

$offsetType = $accessoryType->getOffsetType();
$offsetTypes[$offsetType->getValue()] = [
TrinaryLogic::createYes(),
$argType->getOffsetValueType($offsetType),
];
}
}

$keyTypes = [];
$valueTypes = [];
$nonEmpty = false;
Expand Down Expand Up @@ -133,6 +178,31 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection,
if ($isList) {
$arrayType = TypeCombinator::intersect($arrayType, new AccessoryArrayListType());
}
if ($offsetTypes !== []) {
$knownOffsetValues = [];
foreach ($offsetTypes as $key => [$hasOffsetValue, $offsetType]) {
Copy link
Contributor

@VincentLanglet VincentLanglet Jan 28, 2026

Choose a reason for hiding this comment

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

Eventually, looking at the usage you could have use array<Type|null> as offsetTypes:

  • a Type means HasOffsetValueType (the yes case)
  • null means HasOffsetType (the maybe case)

Copy link
Contributor Author

@staabm staabm Jan 28, 2026

Choose a reason for hiding this comment

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

agree we could transform it. not sure this will lead to logic which is easier to follow?

$keyType = is_string($key) ? new ConstantStringType($key) : new ConstantIntegerType($key);

if ($hasOffsetValue->yes()) {
// the last known offset will overwrite previous values
$hasOffsetType = new HasOffsetValueType(
$keyType,
$offsetType,
);
} elseif ($hasOffsetValue->maybe()) {
$hasOffsetType = new HasOffsetType(
$keyType,
);
} else {
continue;
}

$knownOffsetValues[] = $hasOffsetType;
}
if ($knownOffsetValues !== []) {
$arrayType = TypeCombinator::intersect($arrayType, ...$knownOffsetValues);
}
}

return $arrayType;
}
Expand Down
125 changes: 125 additions & 0 deletions tests/PHPStan/Analyser/nsrt/array-replace-const-non-const.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
<?php

namespace ArrayReplaceConstNonConst;

use function array_replace;
use function PHPStan\Testing\assertType;

function doFoo(array $post): void {
assertType(
"non-empty-array&hasOffset('a')&hasOffset('b')&hasOffset(10)",
array_replace(['a' => 1, 'b' => false, 10 => 99], $post)
);
}

function doBar(array $array): void {
assertType(
"non-empty-array&hasOffsetValue('a', 1)&hasOffsetValue('b', false)&hasOffsetValue(10, 99)",
array_replace($array, ['a' => 1, 'b' => false, 10 => 99])
);
}

function doFooBar(array $array): void {
assertType(
"non-empty-array&hasOffset('x')&hasOffsetValue('a', 1)&hasOffsetValue('b', false)&hasOffsetValue('c', 'e')",
array_replace(['c' => 'd', 'x' => 'y'], $array, ['a' => 1, 'b' => false, 'c' => 'e'])
);
}

/**
* @param array{a?: 1, b: 2} $array
*/
function doOptShapeKeys(array $array, array $arr2): void {
assertType("non-empty-array&hasOffsetValue('b', 2)", array_replace($arr2, $array));
assertType("non-empty-array&hasOffset('b')", array_replace($array, $arr2));
}

function hasOffsetKeys(array $array, array $arr2): void {
if (array_key_exists('b', $array)) {
assertType("non-empty-array&hasOffsetValue('b', mixed)", array_replace($arr2, $array));
assertType("non-empty-array&hasOffset('b')", array_replace($array, $arr2));
}
}

function maybeHasOffsetKeys(array $array): void {
$arr2 = [];
if (rand(0,1)) {
$arr2 ['ab'] = 'def';
}

assertType("array", array_replace($arr2, $array));
assertType("array", array_replace($array, $arr2));
}

function hasOffsetValueKeys(array $hasB, array $mixedArray, array $hasC): void {
$hasB['b'] = 123;
$hasC['c'] = 'def';

assertType("non-empty-array&hasOffsetValue('b', 123)", array_replace($mixedArray, $hasB));
assertType("non-empty-array&hasOffset('b')", array_replace($hasB, $mixedArray));

assertType(
"non-empty-array&hasOffset('b')&hasOffsetValue('c', 'def')",
array_replace($mixedArray, $hasB, $hasC)
);
assertType(
"non-empty-array&hasOffset('b')&hasOffsetValue('c', 'def')",
array_replace($hasB, $mixedArray, $hasC)
);

assertType(
"non-empty-array&hasOffset('c')&hasOffsetValue('b', 123)",
array_replace($hasC, $mixedArray, $hasB)
);
assertType(
"non-empty-array&hasOffset('b')&hasOffset('c')",
array_replace($hasC, $hasB, $mixedArray)
);

if (rand(0, 1)) {
$hasBorC = ['b' => 1];
} else {
$hasBorC = ['c' => 2];
}
assertType('array{b: 1}|array{c: 2}', $hasBorC);
assertType("non-empty-array", array_replace($mixedArray, $hasBorC));
assertType("non-empty-array", array_replace($hasBorC, $mixedArray));

if (rand(0, 1)) {
$differentCs = ['c' => 10];
} else {
$differentCs = ['c' => 20];
}
assertType('array{c: 10}|array{c: 20}', $differentCs);
assertType("non-empty-array&hasOffsetValue('c', 10|20)", array_replace($mixedArray, $differentCs));
assertType("non-empty-array&hasOffset('c')", array_replace($differentCs, $mixedArray));

assertType("non-empty-array&hasOffsetValue('c', 10|20)", array_replace($mixedArray, $hasBorC, $differentCs));
assertType("non-empty-array", array_replace($differentCs, $mixedArray, $hasBorC)); // could be non-empty-array&hasOffset('c')
assertType("non-empty-array&hasOffsetValue('c', 10|20)", array_replace($hasBorC, $mixedArray, $differentCs));
assertType("non-empty-array", array_replace($differentCs, $hasBorC, $mixedArray)); // could be non-empty-array&hasOffset('c')
}

/**
* @param array{a?: 1, b?: 2} $allOptional
*/
function doAllOptional(array $allOptional, array $arr2): void {
assertType("array", array_replace($arr2, $allOptional));
assertType("array", array_replace($allOptional, $arr2));
}

function withArrayReplacement(array $base): void {
$replacements = [ 'citrus' => [ 'grapefruit' ] ];
$replacements2 = [ 'citrus' => [ 'kumquat', 'citron' ], 'pome' => [ 'loquat' ] ];

$basket = array_replace($base, $replacements, $replacements2);
assertType("non-empty-array&hasOffsetValue('citrus', array{'kumquat', 'citron'})&hasOffsetValue('pome', array{'loquat'})", $basket);
}

/**
* @param array{foo: int, x: string}|array{foo: string, y: 1} $arr1
*/
function doUnions(array $arr1, array $arr2): void {
assertType("non-empty-array&hasOffset('foo')", array_replace($arr1, $arr2));
assertType("non-empty-array&hasOffsetValue('foo', int|string)", array_replace($arr2, $arr1));
}
8 changes: 4 additions & 4 deletions tests/PHPStan/Analyser/nsrt/array-replace.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,11 @@ public function arrayReplaceUnionTypeArrayShapes($array1, $array2): void
*/
public function arrayReplaceArrayShapeAndGeneralArray($array1, $array2, $array3): void
{
assertType("non-empty-array<string, '1'|'2'|int>", array_replace($array1, $array2));
assertType("non-empty-array<string, '1'|'2'|int>", array_replace($array2, $array1));
assertType("non-empty-array<string, '1'|'2'|int>&hasOffset('bar')&hasOffset('foo')", array_replace($array1, $array2));
assertType("non-empty-array<string, '1'|'2'|int>&hasOffsetValue('bar', '2')&hasOffsetValue('foo', '1')", array_replace($array2, $array1));

assertType("non-empty-array<'bar'|'foo'|int, string>", array_replace($array1, $array3));
assertType("non-empty-array<'bar'|'foo'|int, string>", array_replace($array3, $array1));
assertType("non-empty-array<'bar'|'foo'|int, string>&hasOffset('bar')&hasOffset('foo')", array_replace($array1, $array3));
assertType("non-empty-array<'bar'|'foo'|int, string>&hasOffsetValue('bar', '2')&hasOffsetValue('foo', '1')", array_replace($array3, $array1));
Copy link
Contributor Author

@staabm staabm Jan 27, 2026

Choose a reason for hiding this comment

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

this kind of overlaps. should we do something about it, or is it fine?


assertType("array<int|string, int|string>", array_replace($array2, $array3));
}
Expand Down
Loading