diff --git a/src/Type/Accessory/HasOffsetType.php b/src/Type/Accessory/HasOffsetType.php index da6c176655..6eb0ea07c2 100644 --- a/src/Type/Accessory/HasOffsetType.php +++ b/src/Type/Accessory/HasOffsetType.php @@ -52,10 +52,7 @@ public function __construct(private ConstantStringType|ConstantIntegerType $offs { } - /** - * @return ConstantStringType|ConstantIntegerType - */ - public function getOffsetType(): Type + public function getOffsetType(): ConstantStringType|ConstantIntegerType { return $this->offsetType; } diff --git a/src/Type/Php/ArrayMergeFunctionDynamicReturnTypeExtension.php b/src/Type/Php/ArrayMergeFunctionDynamicReturnTypeExtension.php index 1d4f7a1122..1d280be17b 100644 --- a/src/Type/Php/ArrayMergeFunctionDynamicReturnTypeExtension.php +++ b/src/Type/Php/ArrayMergeFunctionDynamicReturnTypeExtension.php @@ -8,6 +8,8 @@ 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; @@ -16,12 +18,15 @@ use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\DynamicFunctionReturnTypeExtension; use PHPStan\Type\IntegerType; +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_int; #[AutowiredService] final class ArrayMergeFunctionDynamicReturnTypeExtension implements DynamicFunctionReturnTypeExtension @@ -96,6 +101,43 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection, return $newArrayBuilder->getArray(); } + $offsetTypes = []; + foreach ($argTypes as $argType) { + if ($argType->isConstantArray()->yes()) { + foreach ($argType->getConstantArrays() as $constantArray) { + foreach ($constantArray->getKeyTypes() as $keyType) { + $hasOffsetValue = TrinaryLogic::createFromBoolean($argType->hasOffsetValueType($keyType)->yes()); + $offsetTypes[$keyType->getValue()] = [ + $hasOffsetValue, + $argType->getOffsetValueType($keyType), + ]; + } + } + } else { + foreach ($offsetTypes as $key => [$hasOffsetValue, $offsetValueType]) { + $offsetTypes[$key] = [ + $hasOffsetValue->and(TrinaryLogic::createMaybe()), + new MixedType(), + ]; + } + } + + 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; @@ -132,6 +174,36 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection, if ($isList) { $arrayType = TypeCombinator::intersect($arrayType, new AccessoryArrayListType()); } + if ($offsetTypes !== []) { + $knownOffsetValues = []; + foreach ($offsetTypes as $key => [$hasOffsetValue, $offsetType]) { + if (is_int($key)) { + // int keys will be appended and renumbered. + // at this point we can't reason about them, because unknown arrays are in the mix. + continue; + } + $keyType = new ConstantStringType($key); + + if ($hasOffsetValue->yes()) { + // the last string-keyed 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; } diff --git a/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php b/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php index c9fe4f942a..17c38c25aa 100644 --- a/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php @@ -4697,11 +4697,11 @@ public static function dataArrayFunctions(): array 'array_merge($generalStringKeys, $generalDateTimeValues)', ], [ - 'non-empty-array<1|string, int|stdClass>', + "non-empty-array<1|string, int|stdClass>&hasOffsetValue('foo', stdClass)", 'array_merge($generalStringKeys, $stringOrIntegerKeys)', ], [ - 'non-empty-array<1|string, int|stdClass>', + "non-empty-array<1|string, int|stdClass>&hasOffset('foo')", 'array_merge($stringOrIntegerKeys, $generalStringKeys)', ], [ diff --git a/tests/PHPStan/Analyser/nsrt/array-merge-const-non-const.php b/tests/PHPStan/Analyser/nsrt/array-merge-const-non-const.php new file mode 100644 index 0000000000..586d2b593b --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/array-merge-const-non-const.php @@ -0,0 +1,123 @@ + 1, 'b' => false, 10 => 99], $post) + ); +} + +function doBar(array $array): void { + assertType( + "non-empty-array&hasOffsetValue('a', 1)&hasOffsetValue('b', false)", + array_merge($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_merge(['c' => 'd', 'x' => 'y'], $array, ['a' => 1, 'b' => false, 'c' => 'e']) + ); +} + +function doFooInts(array $array): void { + assertType( + "non-empty-array&hasOffsetValue('a', 1)&hasOffsetValue('c', 'e')", + array_merge([1 => 'd'], $array, ['a' => 1, 3 => false, 'c' => 'e']) + ); +} + +/** + * @param array $array + */ +function floatKey(array $array): void { + assertType( + "non-empty-array&hasOffsetValue('a', '1')&hasOffsetValue('c', 'e')", + array_merge([4.23 => 'd'], $array, ['a' => '1', 3 => 'false', 'c' => 'e']) + ); +} + +function doOptKeys(array $array, array $arr2): void { + if (rand(0, 1)) { + $array['abc'] = 'def'; + } + assertType("array", array_merge($arr2, $array)); + assertType("array", array_merge($array, $arr2)); +} + +/** + * @param array{a?: 1, b: 2} $array + */ +function doOptShapeKeys(array $array, array $arr2): void { + assertType("non-empty-array&hasOffsetValue('b', 2)", array_merge($arr2, $array)); + assertType("non-empty-array&hasOffset('b')", array_merge($array, $arr2)); +} + +function hasOffsetKeys(array $array, array $arr2): void { + if (array_key_exists('b', $array)) { + assertType("non-empty-array&hasOffsetValue('b', mixed)", array_merge($arr2, $array)); + assertType("non-empty-array&hasOffset('b')", array_merge($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_merge($mixedArray, $hasB)); + assertType("non-empty-array&hasOffset('b')", array_merge($hasB, $mixedArray)); + + assertType( + "non-empty-array&hasOffset('b')&hasOffsetValue('c', 'def')", + array_merge($mixedArray, $hasB, $hasC) + ); + assertType( + "non-empty-array&hasOffset('b')&hasOffsetValue('c', 'def')", + array_merge($hasB, $mixedArray, $hasC) + ); + + assertType( + "non-empty-array&hasOffset('c')&hasOffsetValue('b', 123)", + array_merge($hasC, $mixedArray, $hasB) + ); + assertType( + "non-empty-array&hasOffset('b')&hasOffset('c')", + array_merge($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_merge($mixedArray, $hasBorC)); + assertType("non-empty-array", array_merge($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_merge($mixedArray, $differentCs)); + assertType("non-empty-array&hasOffset('c')", array_merge($differentCs, $mixedArray)); + + assertType("non-empty-array&hasOffsetValue('c', 10|20)", array_merge($mixedArray, $hasBorC, $differentCs)); + assertType("non-empty-array", array_merge($differentCs, $mixedArray, $hasBorC)); // could be non-empty-array&hasOffset('c') + assertType("non-empty-array&hasOffsetValue('c', 10|20)", array_merge($hasBorC, $mixedArray, $differentCs)); + assertType("non-empty-array", array_merge($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_merge($arr2, $allOptional)); + assertType("array", array_merge($allOptional, $arr2)); +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-2911.php b/tests/PHPStan/Analyser/nsrt/bug-2911.php index 3cfbd308f1..1b189148b3 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-2911.php +++ b/tests/PHPStan/Analyser/nsrt/bug-2911.php @@ -49,23 +49,23 @@ public function __construct(MutatorConfig $config) private function getResultSettings(array $settings): array { $settings = array_merge(self::DEFAULT_SETTINGS, $settings); - assertType('non-empty-array', $settings); + assertType("non-empty-array&hasOffset('limit')&hasOffset('remove')", $settings); if (!is_string($settings['remove'])) { throw $this->configException($settings, 'remove'); } - assertType("non-empty-array&hasOffsetValue('remove', string)", $settings); + assertType("non-empty-array&hasOffset('limit')&hasOffsetValue('remove', string)", $settings); $settings['remove'] = strtolower($settings['remove']); - assertType("non-empty-array&hasOffsetValue('remove', lowercase-string)", $settings); + assertType("non-empty-array&hasOffset('limit')&hasOffsetValue('remove', lowercase-string)", $settings); if (!in_array($settings['remove'], ['first', 'last', 'all'], true)) { throw $this->configException($settings, 'remove'); } - assertType("non-empty-array&hasOffsetValue('remove', 'all'|'first'|'last')", $settings); + assertType("non-empty-array&hasOffset('limit')&hasOffsetValue('remove', 'all'|'first'|'last')", $settings); if (!is_numeric($settings['limit']) || $settings['limit'] < 1) { throw $this->configException($settings, 'limit'); @@ -110,13 +110,13 @@ private function getResultSettings(array $settings): array { $settings = array_merge(self::DEFAULT_SETTINGS, $settings); - assertType('non-empty-array', $settings); + assertType("non-empty-array&hasOffset('limit')&hasOffset('remove')", $settings); if (!is_string($settings['remove'])) { throw new Exception(); } - assertType("non-empty-array&hasOffsetValue('remove', string)", $settings); + assertType("non-empty-array&hasOffset('limit')&hasOffsetValue('remove', string)", $settings); if (!is_int($settings['limit'])) { throw new Exception(); diff --git a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php index dd51c3df3a..0d4161ee54 100644 --- a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php @@ -1276,4 +1276,9 @@ public function testBug9494(): void $this->analyse([__DIR__ . '/data/bug-9494.php'], []); } + public function testBug8438(): void + { + $this->analyse([__DIR__ . '/data/bug-8438.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/bug-8438.php b/tests/PHPStan/Rules/Methods/data/bug-8438.php new file mode 100644 index 0000000000..2459ff616e --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-8438.php @@ -0,0 +1,26 @@ + $array + * + * @return array{expr: mixed, ...} + */ + protected function foo(array $array): array + { + $rnd = mt_rand(); + if ($rnd === 0) { + return ['expr' => 'test']; + } elseif ($rnd === 1) { + // no error with checkBenevolentUnionTypes: false (default even with l9 + strict rules) + return ['expr' => 'test', 1 => 'ok']; + } else { + // phpstan must understand 'expr' key is always present in the result, + // then there will be no error here neither + return array_merge($array, ['expr' => 'test', 1 => 'ok']); + } + } +}