Skip to content

Commit

Permalink
Merge pull request #7167 from AndrolGenhald/bugfix/7164-conflicting-f…
Browse files Browse the repository at this point in the history 
…ake-variable
  • Loading branch information
@weirdan
weirdan committed Dec 15, 2021
2 parents 16c0496 + d62bee3 commit 7e97c5c
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 20 deletions.
14 changes: 11 additions & 3 deletions src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php
View file
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
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
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

0 comments on commit 7e97c5c

Please sign in to comment.