Skip to content
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

Jump to bottom

Fix conflicting array_map return provider fake variables. #7167

Merged
merged 5 commits into from Dec 15, 2021
Merged

Fix conflicting array_map return provider fake variables. #7167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b78a188
Assign id to array_map fake variables to avoid conflicts and ensure r…
AndrolGenhald Dec 15, 2021
00749c8
Fix array_filter return type provider.
AndrolGenhald Dec 15, 2021
04c0243
Fix trailing comma for PHP < 8.0.
AndrolGenhald Dec 15, 2021
03ccb9b
Add test for nested `array_map` return type.
AndrolGenhald Dec 15, 2021
d62bee3
Rename `$fake_var_id` to `$fake_var_discriminator`.
AndrolGenhald Dec 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php
View file
Open in desktop
Expand Up @@ -29,6 +29,7 @@
use function array_slice;
use function count;
use function is_string;
use function mt_rand;
use function reset;
use function spl_object_id;

Expand Down Expand Up @@ -185,13 +186,15 @@ static function ($keyed_type) {
if ($array_arg && $mapping_function_ids) {
$assertions = [];

$fake_var_discriminator = mt_rand();
ArrayMapReturnTypeProvider::getReturnTypeFromMappingIds(
$statements_source,
$mapping_function_ids,
$context,
$function_call_arg,
array_slice($call_args, 0, 1),
$assertions
$assertions,
$fake_var_discriminator
);

$array_var_id = ExpressionIdentifier::getArrayVarId(
Expand All @@ -200,10 +203,13 @@ static function ($keyed_type) {
$statements_source
);

if (isset($assertions[$array_var_id . '[$__fake_offset_var__]'])) {
if (isset($assertions[$array_var_id . "[\$__fake_{$fake_var_discriminator}_offset_var__]"])) {
$changed_var_ids = [];

$assertions = ['$inner_type' => $assertions[$array_var_id . '[$__fake_offset_var__]']];
$assertions = [
'$inner_type' =>
$assertions["{$array_var_id}[\$__fake_{$fake_var_discriminator}_offset_var__]"],
];

$reconciled_types = Reconciler::reconcileKeyedTypes(
$assertions,
Expand All @@ -221,6 +227,8 @@ static function ($keyed_type) {
$inner_type = $reconciled_types['$inner_type'];
}
}

ArrayMapReturnTypeProvider::cleanContext($context, $fake_var_discriminator);
}
} elseif (($function_call_arg->value instanceof PhpParser\Node\Expr\Closure
|| $function_call_arg->value instanceof PhpParser\Node\Expr\ArrowFunction)
Expand Down
52 changes: 35 additions & 17 deletions src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php
View file
Open in desktop
Expand Up @@ -39,7 +39,9 @@
use function count;
use function explode;
use function in_array;
use function mt_rand;
use function reset;
use function str_contains;
use function strpos;
use function substr;

Expand Down Expand Up @@ -340,6 +342,8 @@ private static function executeFakeCall(
/**
* @param non-empty-array<int, string> $mapping_function_ids
* @param list<PhpParser\Node\Arg> $array_args
* @param int|null $fake_var_discriminator Set the fake variable id to a known value with the discriminator
* as a substring, and don't clear it from the context.
* @param-out array<string, array<array<int, string>>>|null $assertions
*/
public static function getReturnTypeFromMappingIds(
Expand All @@ -348,15 +352,23 @@ public static function getReturnTypeFromMappingIds(
Context $context,
PhpParser\Node\Arg $function_call_arg,
array $array_args,
?array &$assertions = null
?array &$assertions = null,
?int $fake_var_discriminator = null
): Union {
$mapping_return_type = null;

$codebase = $statements_source->getCodebase();

$clean_context = false;

foreach ($mapping_function_ids as $mapping_function_id) {
$mapping_function_id_parts = explode('&', $mapping_function_id);

if ($fake_var_discriminator === null) {
$fake_var_discriminator = mt_rand();
$clean_context = true;
}

foreach ($mapping_function_id_parts as $mapping_function_id_part) {
$fake_args = [];

Expand All @@ -365,7 +377,7 @@ public static function getReturnTypeFromMappingIds(
new VirtualArrayDimFetch(
$array_arg->value,
new VirtualVariable(
'__fake_offset_var__',
"__fake_{$fake_var_discriminator}_offset_var__",
$array_arg->value->getAttributes()
),
$array_arg->value->getAttributes()
Expand All @@ -390,7 +402,7 @@ public static function getReturnTypeFromMappingIds(
if ($is_instance) {
$fake_method_call = new VirtualMethodCall(
new VirtualVariable(
'__fake_method_call_var__',
"__fake_{$fake_var_discriminator}_method_call_var__",
$function_call_arg->getAttributes()
),
new VirtualIdentifier(
Expand All @@ -416,21 +428,16 @@ public static function getReturnTypeFromMappingIds(
}
}

$context->vars_in_scope['$__fake_offset_var__'] = Type::getMixed();
$context->vars_in_scope['$__fake_method_call_var__'] = $lhs_instance_type
?: new Union([
new TNamedObject($callable_fq_class_name)
]);
$context->vars_in_scope["\$__fake_{$fake_var_discriminator}_offset_var__"] = Type::getMixed();
$context->vars_in_scope["\$__fake_{$fake_var_discriminator}_method_call_var__"] =
$lhs_instance_type ?: new Union([new TNamedObject($callable_fq_class_name)]);

$fake_method_return_type = self::executeFakeCall(
$statements_source,
$fake_method_call,
$context,
$assertions
);

unset($context->vars_in_scope['$__fake_offset_var__']);
unset($context->vars_in_scope['$__method_call_var__']);
} else {
$fake_method_call = new VirtualStaticCall(
new VirtualFullyQualified(
Expand All @@ -445,16 +452,14 @@ public static function getReturnTypeFromMappingIds(
$function_call_arg->getAttributes()
);

$context->vars_in_scope['$__fake_offset_var__'] = Type::getMixed();
$context->vars_in_scope["\$__fake_{$fake_var_discriminator}_offset_var__"] = Type::getMixed();

$fake_method_return_type = self::executeFakeCall(
$statements_source,
$fake_method_call,
$context,
$assertions
);

unset($context->vars_in_scope['$__fake_offset_var__']);
}

$function_id_return_type = $fake_method_return_type ?? Type::getMixed();
Expand All @@ -468,7 +473,7 @@ public static function getReturnTypeFromMappingIds(
$function_call_arg->getAttributes()
);

$context->vars_in_scope['$__fake_offset_var__'] = Type::getMixed();
$context->vars_in_scope["\$__fake_{$fake_var_discriminator}_offset_var__"] = Type::getMixed();

$fake_function_return_type = self::executeFakeCall(
$statements_source,
Expand All @@ -477,12 +482,16 @@ public static function getReturnTypeFromMappingIds(
$assertions
);

unset($context->vars_in_scope['$__fake_offset_var__']);

$function_id_return_type = $fake_function_return_type ?? Type::getMixed();
}
}

if ($clean_context) {
self::cleanContext($context, $fake_var_discriminator);
}

$fake_var_discriminator = null;

$mapping_return_type = Type::combineUnionTypes(
$function_id_return_type,
$mapping_return_type,
Expand All @@ -492,4 +501,13 @@ public static function getReturnTypeFromMappingIds(

return $mapping_return_type;
}

public static function cleanContext(Context $context, int $fake_var_discriminator): void
{
foreach ($context->vars_in_scope as $var_in_scope => $_) {
if (str_contains($var_in_scope, "__fake_{$fake_var_discriminator}_")) {
unset($context->vars_in_scope[$var_in_scope]);
}
}
}
}
23 changes: 23 additions & 0 deletions tests/ReturnTypeTest.php
View file
Open in desktop
Expand Up @@ -1065,6 +1065,29 @@ public function __construct() {
}
}'
],
'nestedArrayMapReturnTypeDoesntCrash' => [
'<?php
function bar(array $a): array {
return $a;
}

/**
* @param array[] $x
*
* @return array[]
*/
function foo(array $x): array {
return array_map(
"array_merge",
array_map(
"bar",
$x
),
$x
);
}
',
],
];
}

Expand Down