Repository navigation
Add null to array_map() callback parameters of shorter arrays #6874
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.
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
base: 2.3.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace PHPStan\Reflection; | ||
|
|
||
| use PHPStan\Turbo\ReferencedByTurboExtension; | ||
| use PHPStan\Type\Type; | ||
| use PHPStan\Type\TypeCombinator; | ||
| use function count; | ||
| use function is_int; | ||
| use function max; | ||
|
|
||
| /** | ||
| * With more than one array, array_map() calls the callback once per index of | ||
| * the longest array, and passes null for each array that is shorter. | ||
| */ | ||
| #[ReferencedByTurboExtension(key: 'arrayMapCallbackParameterTypes')] | ||
| final class ArrayMapCallbackParameterTypes | ||
|
staabm marked this conversation as resolved.
Resolved
|
||
| { | ||
|
|
||
| /** | ||
| * Adds null to the value type of each array that is shorter than another one | ||
| * when both have their largest size. Only when the size of every array is known. | ||
| * | ||
| * The largest sizes are compared, not the smallest, because two arrays can be | ||
| * one array: array_map($cb, $a, $a) never passes null, whatever sizes $a can have. | ||
| * | ||
| * @param list<Type> $valueTypes | ||
| * @param list<Type> $arrayTypes the array each value type comes from | ||
| * @return list<Type> | ||
| */ | ||
| public static function addPaddingNull(array $valueTypes, array $arrayTypes): array | ||
| { | ||
| if (count($arrayTypes) < 2) { | ||
| return $valueTypes; | ||
| } | ||
|
|
||
| $maxSizes = []; | ||
| foreach ($arrayTypes as $i => $arrayType) { | ||
| $sizes = []; | ||
| foreach ($arrayType->getArraySize()->getFiniteTypes() as $sizeType) { | ||
| foreach ($sizeType->getConstantScalarValues() as $size) { | ||
| if (!is_int($size)) { | ||
| continue; | ||
| } | ||
|
|
||
| $sizes[] = $size; | ||
| } | ||
| } | ||
|
|
||
| if ($sizes === []) { | ||
| // general arrays can have the same size, but the type system cannot say so | ||
| return $valueTypes; | ||
| } | ||
|
|
||
| $maxSizes[$i] = max($sizes); | ||
| } | ||
|
|
||
| $longestSize = max($maxSizes); | ||
| foreach ($valueTypes as $i => $valueType) { | ||
| if ($maxSizes[$i] === $longestSize) { | ||
| continue; | ||
| } | ||
|
|
||
| $valueTypes[$i] = TypeCombinator::addNull($valueType); | ||
| } | ||
|
|
||
| return $valueTypes; | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -206,8 +206,10 @@ public static function applyIntrinsicArgOverrides( | |
| ) { | ||
| $arrayMapArgs = $args[0]->value->getAttribute(ArrayMapArgVisitor::ATTRIBUTE_NAME); | ||
| if ($arrayMapArgs !== null) { | ||
| $callbackParameters = []; | ||
| $nativeCallbackParameters = []; | ||
| $callbackValueTypes = []; | ||
| $callbackArrayTypes = []; | ||
| $nativeCallbackValueTypes = []; | ||
| $nativeCallbackArrayTypes = []; | ||
| foreach ($arrayMapArgs as $arg) { | ||
| $argType = ($typeGetter)($arg->value); | ||
| $nativeArgType = ($nativeTypeGetter)($arg->value); | ||
|
|
@@ -217,7 +219,8 @@ public static function applyIntrinsicArgOverrides( | |
| foreach ($constantArrays as $constantArray) { | ||
| $valueTypes = $constantArray->getValueTypes(); | ||
| foreach ($valueTypes as $valueType) { | ||
| $callbackParameters[] = new DummyParameter('item', ($iterableValueTypeGetter)($valueType), optional: false, passedByReference: PassedByReference::createNo(), variadic: false, defaultValue: null); | ||
| $callbackValueTypes[] = ($iterableValueTypeGetter)($valueType); | ||
| $callbackArrayTypes[] = $valueType; | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -226,16 +229,28 @@ public static function applyIntrinsicArgOverrides( | |
| foreach ($nativeConstantArrays as $constantArray) { | ||
| $valueTypes = $constantArray->getValueTypes(); | ||
| foreach ($valueTypes as $valueType) { | ||
| $nativeCallbackParameters[] = new DummyParameter('item', ($iterableValueTypeGetter)($valueType), optional: false, passedByReference: PassedByReference::createNo(), variadic: false, defaultValue: null); | ||
| $nativeCallbackValueTypes[] = ($iterableValueTypeGetter)($valueType); | ||
| $nativeCallbackArrayTypes[] = $valueType; | ||
| } | ||
| } | ||
| } | ||
| } else { | ||
| $callbackParameters[] = new DummyParameter('item', ($iterableValueTypeGetter)($argType), optional: false, passedByReference: PassedByReference::createNo(), variadic: false, defaultValue: null); | ||
| $nativeCallbackParameters[] = new DummyParameter('item', ($iterableValueTypeGetter)($nativeArgType), optional: false, passedByReference: PassedByReference::createNo(), variadic: false, defaultValue: null); | ||
| $callbackValueTypes[] = ($iterableValueTypeGetter)($argType); | ||
| $callbackArrayTypes[] = $argType; | ||
| $nativeCallbackValueTypes[] = ($iterableValueTypeGetter)($nativeArgType); | ||
| $nativeCallbackArrayTypes[] = $nativeArgType; | ||
| } | ||
| } | ||
|
|
||
| $callbackParameters = []; | ||
| foreach (ArrayMapCallbackParameterTypes::addPaddingNull($callbackValueTypes, $callbackArrayTypes) as $valueType) { | ||
| $callbackParameters[] = new DummyParameter('item', $valueType, optional: false, passedByReference: PassedByReference::createNo(), variadic: false, defaultValue: null); | ||
| } | ||
| $nativeCallbackParameters = []; | ||
| foreach (ArrayMapCallbackParameterTypes::addPaddingNull($nativeCallbackValueTypes, $nativeCallbackArrayTypes) as $valueType) { | ||
| $nativeCallbackParameters[] = new DummyParameter('item', $valueType, optional: false, passedByReference: PassedByReference::createNo(), variadic: false, defaultValue: null); | ||
| } | ||
|
Comment on lines
+245
to
+252
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since we have 2 separate loops for native-types vs. non-native-types, I think we should also have said differently: do we have test-coverage for both these loops here?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just double checked: if I comment out line 251, no tests fail
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added |
||
|
|
||
| $acceptor = $parametersAcceptors[0]; | ||
| $parameters = $acceptor->getParameters(); | ||
| if (isset($parameters[0])) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| <?php | ||
|
|
||
| namespace Bug13217; | ||
|
|
||
| use function PHPStan\Testing\assertNativeType; | ||
| use function PHPStan\Testing\assertType; | ||
|
|
||
| class Foo | ||
| { | ||
|
|
||
| public function shorterSecondArray(): void | ||
| { | ||
| array_map(function ($a, $b) { | ||
| assertType('1|2', $a); | ||
| assertType('3|null', $b); | ||
|
Comment on lines
+14
to
+15
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unrelated side-question: do we differentiate between types beeing infered from native vs. phpdoc?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, they can differ. With native arrays like |
||
| assertNativeType('1|2', $a); | ||
| assertNativeType('3|null', $b); | ||
| }, [1, 2], [3]); | ||
|
|
||
| array_map(fn ($a, $b) => assertType('3|null', $b), [1, 2], [3]); | ||
|
|
||
| assertType('non-empty-list<1|3>', array_map(fn ($a, $b) => $b ?? 1, [1, 2], [3])); | ||
| } | ||
|
|
||
| public function shorterFirstArray(): void | ||
| { | ||
| array_map(function ($a, $b) { | ||
| assertType('1|null', $a); | ||
| assertType('3|4', $b); | ||
| assertNativeType('1|null', $a); | ||
| assertNativeType('3|4', $b); | ||
| }, [1], [3, 4]); | ||
| } | ||
|
|
||
| public function sameSize(): void | ||
| { | ||
| array_map(function ($a, $b) { | ||
| assertType('1|2', $a); | ||
| assertType('3|4', $b); | ||
| }, [1, 2], [3, 4]); | ||
| } | ||
|
|
||
| /** | ||
| * @param array{1, 2}|array{1} $maybe | ||
| */ | ||
| public function sizeRange(array $maybe): void | ||
| { | ||
| array_map(function ($a, $b) { | ||
| assertType('1|2|null', $a); | ||
| assertType('3|4|5', $b); | ||
| assertNativeType('mixed', $a); | ||
| assertNativeType('3|4|5', $b); | ||
| }, $maybe, [3, 4, 5]); | ||
|
|
||
| array_map(function ($a, $b) { | ||
| assertType('1|2', $a); | ||
| assertType('3|null', $b); | ||
| }, $maybe, [3]); | ||
|
|
||
| array_map(function ($a, $b) { | ||
| assertType('1|2', $a); | ||
| assertType('1|2', $b); | ||
| }, $maybe, $maybe); | ||
| } | ||
|
|
||
| public function singleArray(): void | ||
| { | ||
| array_map(function ($a) { | ||
| assertType('1|2', $a); | ||
| }, [1, 2]); | ||
| } | ||
|
|
||
| /** | ||
| * @param list<int> $ints | ||
| * @param list<string> $strings | ||
| */ | ||
| public function generalArrays(array $ints, array $strings): void | ||
| { | ||
| array_map(function ($a, $b) { | ||
| assertType('int', $a); | ||
| assertType('string', $b); | ||
| }, $ints, $strings); | ||
|
|
||
| array_map(function ($a) { | ||
| assertType('int', $a); | ||
| }, $ints); | ||
|
|
||
| array_map(function ($a, $b) { | ||
| assertType('int', $a); | ||
| assertType('3', $b); | ||
| }, $ints, [3]); | ||
| } | ||
|
|
||
| public function returnType(): void | ||
| { | ||
| assertType('non-empty-list<3|null>', array_map('Bug13217\\pick', [1, 2], [3])); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| /** | ||
| * @template T | ||
| * @param mixed $a | ||
| * @param T $b | ||
| * @return T | ||
| */ | ||
| function pick($a, $b) | ||
| { | ||
| return $b; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| array_map(fn($_, $bValue) => $bValue ?? 1, [1, 2], [3]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here.. might need
assertNativeTypeto cover this codeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Covered now. The new
assertNativeTypelines fail when this line or the native padding above it is removed. They also pass with Turbo loaded.