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();
Copy link
Contributor

Choose a reason for hiding this comment

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

is isFirstClassCallable() check needed first? The getArgs() got assert notice when there is ... variadic place holder.

https://github.com/nikic/PHP-Parser/blob/20b0d55f6655ef2d73d841f6b7d87117a90f9ee2/lib/PhpParser/Node/Expr/CallLike.php#L32

Copy link
Member

Choose a reason for hiding this comment

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

The extensions are not called with first-class callables at all.


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) {
$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));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add another test cases where $array2 is a general array, please? And maybe in the other section somewhere also one where $array1 is a general array but $array2 is a constant array.

a bit more mixed keys (strings or ints) would be interesting too 😊 that would also help massively with potential future refactoring


/**
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