Skip to content

Rework ArrayReplaceFunctionReturnTypeExtension #3958

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
wants to merge 2 commits into
base: 1.12.x
Choose a base branch
from
Open

Rework ArrayReplaceFunctionReturnTypeExtension #3958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2e5bfcf
Rework ArrayReplaceFunctionReturnTypeExtension
VincentLanglet Apr 21, 2025
a8f870a
More test
VincentLanglet Apr 22, 2025
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
126 changes: 94 additions & 32 deletions src/Type/Php/ArrayReplaceFunctionReturnTypeExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,21 @@
use PhpParser\Node\Expr\FuncCall;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\FunctionReflection;
use PHPStan\TrinaryLogic;
use PHPStan\Type\Accessory\AccessoryArrayListType;
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\NeverType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use function array_keys;
use function count;
use function in_array;
use function strtolower;

final class ArrayReplaceFunctionReturnTypeExtension implements DynamicFunctionReturnTypeExtension
Expand All @@ -23,54 +32,107 @@ public function isFunctionSupported(FunctionReflection $functionReflection): boo

public function getTypeFromFunctionCall(FunctionReflection $functionReflection, FuncCall $functionCall, Scope $scope): ?Type
{
$arrayTypes = $this->collectArrayTypes($functionCall, $scope);
$args = $functionCall->getArgs();

if (count($arrayTypes) === 0) {
if (!isset($args[0])) {
return null;
}

return $this->getResultType(...$arrayTypes);
}
$argTypes = [];
$optionalArgTypes = [];
foreach ($args as $arg) {
$argType = $scope->getType($arg->value);

private function getResultType(Type ...$arrayTypes): Type
{
$keyTypes = [];
$valueTypes = [];
$nonEmptyArray = false;
foreach ($arrayTypes as $arrayType) {
if (!$nonEmptyArray && $arrayType->isIterableAtLeastOnce()->yes()) {
$nonEmptyArray = true;
if ($arg->unpack) {
if ($argType->isConstantArray()->yes()) {
foreach ($argType->getConstantArrays() as $constantArray) {
foreach ($constantArray->getValueTypes() as $valueType) {
$argTypes[] = $valueType;
}
}
} else {
$argTypes[] = $argType->getIterableValueType();
}

if (!$argType->isIterableAtLeastOnce()->yes()) {
// unpacked params can be empty, making them optional
$optionalArgTypesOffset = count($argTypes) - 1;
foreach (array_keys($argTypes) as $key) {
$optionalArgTypes[] = $optionalArgTypesOffset + $key;
}
}
} else {
$argTypes[] = $argType;
}

$keyTypes[] = $arrayType->getIterableKeyType();
$valueTypes[] = $arrayType->getIterableValueType();
}

$keyType = TypeCombinator::union(...$keyTypes);
$valueType = TypeCombinator::union(...$valueTypes);
$allConstant = TrinaryLogic::createYes()->lazyAnd(
$argTypes,
static fn (Type $argType) => $argType->isConstantArray(),
);

if ($allConstant->yes()) {
$newArrayBuilder = ConstantArrayTypeBuilder::createEmpty();

foreach ($argTypes as $argType) {
/** @var array<int|string, ConstantIntegerType|ConstantStringType> $keyTypes */
$keyTypes = [];
foreach ($argType->getConstantArrays() as $constantArray) {
foreach ($constantArray->getKeyTypes() as $keyType) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is what you want. Let's say you have array{foo: int}|array{bar: string}.

You'll end up with two values in $keyTypes and you'll create a flat array out of them. Also you'll overwrite a value if two or more arrays in a union share the same key but with different type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure to understand your point.

This is the current behavior we have with array_replace vs array_merge
https://phpstan.org/r/4ec3a6a9-2645-4565-a81e-577b518eb7c3

Since both function are working almost the same way, I copied and adapt the logic from ArrayMergeFunctionReturnTypeExtension

phpstan-src/src/Type/Php/ArrayMergeFunctionDynamicReturnTypeExtension.php

Lines 74 to 94 in 84e2316

if ($allConstant->yes()) {
$newArrayBuilder = ConstantArrayTypeBuilder::createEmpty();
foreach ($argTypes as $argType) {
/** @var array<int|string, ConstantIntegerType|ConstantStringType> $keyTypes */
$keyTypes = [];
foreach ($argType->getConstantArrays() as $constantArray) {
foreach ($constantArray->getKeyTypes() as $keyType) {
$keyTypes[$keyType->getValue()] = $keyType;
}
}
foreach ($keyTypes as $keyType) {
$newArrayBuilder->setOffsetValueType(
$keyType instanceof ConstantIntegerType ? null : $keyType,
$argType->getOffsetValueType($keyType),
!$argType->hasOffsetValueType($keyType)->yes(),
);
}
}
return $newArrayBuilder->getArray();

In which we're doing

foreach ($argType->getConstantArrays() as $constantArray) {
					foreach ($constantArray->getKeyTypes() as $keyType) {

With this PR, we're now getting

array{bar?: int, foo: 'thing'}

with both array_merge and array_replace

$keyTypes[$keyType->getValue()] = $keyType;
}
}

foreach ($keyTypes as $keyType) {
$newArrayBuilder->setOffsetValueType(
$keyType,
$argType->getOffsetValueType($keyType),
!$argType->hasOffsetValueType($keyType)->yes(),
);
}
}

$arrayType = new ArrayType($keyType, $valueType);
return $nonEmptyArray ? TypeCombinator::intersect($arrayType, new NonEmptyArrayType()) : $arrayType;
}
return $newArrayBuilder->getArray();
}

/**
* @return Type[]
*/
private function collectArrayTypes(FuncCall $functionCall, Scope $scope): array
{
$args = $functionCall->getArgs();
$keyTypes = [];
$valueTypes = [];
$nonEmpty = false;
$isList = true;
foreach ($argTypes as $key => $argType) {
$keyType = $argType->getIterableKeyType();
$keyTypes[] = $keyType;
$valueTypes[] = $argType->getIterableValueType();

if (!$argType->isList()->yes()) {
$isList = false;
}

$arrayTypes = [];
foreach ($args as $arg) {
$argType = $scope->getType($arg->value);
if (!$argType->isArray()->yes()) {
if (in_array($key, $optionalArgTypes, true) || !$argType->isIterableAtLeastOnce()->yes()) {
continue;
}

$arrayTypes[] = $arg->unpack ? $argType->getIterableValueType() : $argType;
$nonEmpty = true;
}

$keyType = TypeCombinator::union(...$keyTypes);
if ($keyType instanceof NeverType) {
return new ConstantArrayType([], []);
}

$arrayType = new ArrayType(
$keyType,
TypeCombinator::union(...$valueTypes),
);

if ($nonEmpty) {
$arrayType = TypeCombinator::intersect($arrayType, new NonEmptyArrayType());
}
if ($isList) {
$arrayType = TypeCombinator::intersect($arrayType, new AccessoryArrayListType());
}

return $arrayTypes;
return $arrayType;
}

}
40 changes: 37 additions & 3 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 @@ -23,9 +23,9 @@ public function arrayReplace($array1, $array2): void
*/
public function arrayReplaceArrayShapes($array1, $array2): void
{
assertType("non-empty-array<'bar'|'foo', '1'|'2'>", array_replace($array1));
assertType("non-empty-array<'bar'|'foo', '1'|'2'>", array_replace([], $array1));
assertType("non-empty-array<'bar'|'foo', '1'|'2'|'4'>", array_replace($array1, $array2));
assertType("array{foo: '1', bar: '2'}", array_replace($array1));
assertType("array{foo: '1', bar: '2'}", array_replace([], $array1));
assertType("array{foo: '1', bar: '4'}", array_replace($array1, $array2));
}

/**
Expand Down Expand Up @@ -68,4 +68,38 @@ public function arrayReplaceUnionTypeArrayShapes($array1, $array2): void
assertType("array<int, array{bar: '2'}|array{bar: '3'}|array{foo: '1'}|array{foo: '2'}>", array_replace($array1, $array2));
assertType("array<int, array{bar: '2'}|array{bar: '3'}|array{foo: '1'}|array{foo: '2'}>", array_replace($array2, $array1));
}

/**
* @param array{foo: '1', bar: '2'} $array1
* @param array<string, int> $array2
* @param array<int, string> $array3
*/
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<'bar'|'foo'|int, string>", array_replace($array1, $array3));
assertType("non-empty-array<'bar'|'foo'|int, string>", array_replace($array3, $array1));

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

/**
* @param array{0: 1, 1: 2} $array1
* @param array{1: 3, 2: 4} $array2
*/
public function arrayReplaceNumericKeys($array1, $array2): void
{
assertType("array{1, 3, 4}", array_replace($array1, $array2));
}

/**
* @param list<int> $array1
* @param list<int> $array2
*/
public function arrayReplaceLists($array1, $array2): void
{
assertType("list<int>", array_replace($array1, $array2));
}
}
10 changes: 10 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-12828.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php declare(strict_types = 1);

namespace Bug12828;

use function PHPStan\Testing\assertType;

$a = ['abc' => 'def', 'hello' => 'world'];
assertType("array{abc: 'def', hello: 'world'}", $a);
$a = array_replace($a, ['hello' => 'country']);
assertType("array{abc: 'def', hello: 'country'}", $a);
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/nsrt/native-types.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ class TestPhp8Stubs
public function doFoo(): void
{
$a = array_replace([1, 2, 3], [4, 5, 6]);
assertType('non-empty-array<0|1|2, 1|2|3|4|5|6>', $a);
assertType('array{4, 5, 6}', $a);
assertNativeType('array', $a);
}

Expand Down
Loading