From b78a188061242d4e5f5ea5d0e7d047ae4e0786d3 Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Wed, 15 Dec 2021 09:12:14 -0600 Subject: [PATCH 1/5] Assign id to array_map fake variables to avoid conflicts and ensure removal (fixes #7164). --- .../ArrayMapReturnTypeProvider.php | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php index 0e5e0db33..a47c17d55 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php @@ -357,6 +357,8 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface foreach ($mapping_function_ids as $mapping_function_id) { $mapping_function_id_parts = explode('&', $mapping_function_id); + $fake_var_id = mt_rand(); + foreach ($mapping_function_id_parts as $mapping_function_id_part) { $fake_args = []; @@ -365,7 +367,7 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface new VirtualArrayDimFetch( $array_arg->value, new VirtualVariable( - '__fake_offset_var__', + "__fake_{$fake_var_id}_offset_var__", $array_arg->value->getAttributes() ), $array_arg->value->getAttributes() @@ -390,7 +392,7 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface if ($is_instance) { $fake_method_call = new VirtualMethodCall( new VirtualVariable( - '__fake_method_call_var__', + "__fake_{$fake_var_id}_method_call_var__", $function_call_arg->getAttributes() ), new VirtualIdentifier( @@ -416,8 +418,8 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface } } - $context->vars_in_scope['$__fake_offset_var__'] = Type::getMixed(); - $context->vars_in_scope['$__fake_method_call_var__'] = $lhs_instance_type + $context->vars_in_scope["\$__fake_{$fake_var_id}_offset_var__"] = Type::getMixed(); + $context->vars_in_scope["\$__fake_{$fake_var_id}_method_call_var__"] = $lhs_instance_type ?: new Union([ new TNamedObject($callable_fq_class_name) ]); @@ -428,9 +430,6 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface $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( @@ -445,7 +444,7 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface $function_call_arg->getAttributes() ); - $context->vars_in_scope['$__fake_offset_var__'] = Type::getMixed(); + $context->vars_in_scope["\$__fake_{$fake_var_id}_offset_var__"] = Type::getMixed(); $fake_method_return_type = self::executeFakeCall( $statements_source, @@ -453,8 +452,6 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface $context, $assertions ); - - unset($context->vars_in_scope['$__fake_offset_var__']); } $function_id_return_type = $fake_method_return_type ?? Type::getMixed(); @@ -468,7 +465,7 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface $function_call_arg->getAttributes() ); - $context->vars_in_scope['$__fake_offset_var__'] = Type::getMixed(); + $context->vars_in_scope["\$__fake_{$fake_var_id}_offset_var__"] = Type::getMixed(); $fake_function_return_type = self::executeFakeCall( $statements_source, @@ -477,10 +474,14 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface $assertions ); - unset($context->vars_in_scope['$__fake_offset_var__']); - $function_id_return_type = $fake_function_return_type ?? Type::getMixed(); } + + foreach ($context->vars_in_scope as $var_in_scope => $_) { + if (str_contains($var_in_scope, "__fake_{$fake_var_id}_")) { + unset($context->vars_in_scope[$var_in_scope]); + } + } } $mapping_return_type = Type::combineUnionTypes( From 00749c84fc6206d31e3421e9ce1a007b267d0fb5 Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Wed, 15 Dec 2021 09:58:36 -0600 Subject: [PATCH 2/5] Fix array_filter return type provider. --- .../ArrayFilterReturnTypeProvider.php | 13 +++++-- .../ArrayMapReturnTypeProvider.php | 34 ++++++++++++++----- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php index 03db510b8..9eed10591 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php @@ -29,6 +29,7 @@ use function array_map; use function array_slice; use function count; use function is_string; +use function mt_rand; use function reset; use function spl_object_id; @@ -185,13 +186,15 @@ class ArrayFilterReturnTypeProvider implements FunctionReturnTypeProviderInterfa if ($array_arg && $mapping_function_ids) { $assertions = []; + $fake_var_id = mt_rand(); ArrayMapReturnTypeProvider::getReturnTypeFromMappingIds( $statements_source, $mapping_function_ids, $context, $function_call_arg, array_slice($call_args, 0, 1), - $assertions + $assertions, + $fake_var_id ); $array_var_id = ExpressionIdentifier::getArrayVarId( @@ -200,10 +203,12 @@ class ArrayFilterReturnTypeProvider implements FunctionReturnTypeProviderInterfa $statements_source ); - if (isset($assertions[$array_var_id . '[$__fake_offset_var__]'])) { + if (isset($assertions[$array_var_id . "[\$__fake_{$fake_var_id}_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_id}_offset_var__]"] + ]; $reconciled_types = Reconciler::reconcileKeyedTypes( $assertions, @@ -221,6 +226,8 @@ class ArrayFilterReturnTypeProvider implements FunctionReturnTypeProviderInterfa $inner_type = $reconciled_types['$inner_type']; } } + + ArrayMapReturnTypeProvider::cleanContext($context, $fake_var_id); } } elseif (($function_call_arg->value instanceof PhpParser\Node\Expr\Closure || $function_call_arg->value instanceof PhpParser\Node\Expr\ArrowFunction) diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php index a47c17d55..d0717b3e0 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php @@ -39,7 +39,9 @@ use function array_slice; 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; @@ -340,6 +342,7 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface /** * @param non-empty-array $mapping_function_ids * @param list $array_args + * @param int|null $fake_var_id Set the fake variable id to a known value and don't clear it from the context * @param-out array>>|null $assertions */ public static function getReturnTypeFromMappingIds( @@ -348,16 +351,22 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface Context $context, PhpParser\Node\Arg $function_call_arg, array $array_args, - ?array &$assertions = null + ?array &$assertions = null, + ?int $fake_var_id = 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); - $fake_var_id = mt_rand(); + if ($fake_var_id === null) { + $fake_var_id = mt_rand(); + $clean_context = true; + } foreach ($mapping_function_id_parts as $mapping_function_id_part) { $fake_args = []; @@ -476,14 +485,14 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface $function_id_return_type = $fake_function_return_type ?? Type::getMixed(); } - - foreach ($context->vars_in_scope as $var_in_scope => $_) { - if (str_contains($var_in_scope, "__fake_{$fake_var_id}_")) { - unset($context->vars_in_scope[$var_in_scope]); - } - } } + if ($clean_context) { + self::cleanContext($context, $fake_var_id); + } + + $fake_var_id = null; + $mapping_return_type = Type::combineUnionTypes( $function_id_return_type, $mapping_return_type, @@ -493,4 +502,13 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface return $mapping_return_type; } + + public static function cleanContext(Context $context, int $fake_var_id): void + { + foreach ($context->vars_in_scope as $var_in_scope => $_) { + if (str_contains($var_in_scope, "__fake_{$fake_var_id}_")) { + unset($context->vars_in_scope[$var_in_scope]); + } + } + } } From 04c024354773f76c9fe2ab4bdc937902d32b9c06 Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Wed, 15 Dec 2021 11:02:24 -0600 Subject: [PATCH 3/5] Fix trailing comma for PHP < 8.0. --- .../Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php index d0717b3e0..3b43aae5f 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php @@ -352,7 +352,7 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface PhpParser\Node\Arg $function_call_arg, array $array_args, ?array &$assertions = null, - ?int $fake_var_id = null, + ?int $fake_var_id = null ): Union { $mapping_return_type = null; From 03ccb9b548952aa107cd44973d62217a1710f54d Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Wed, 15 Dec 2021 11:09:10 -0600 Subject: [PATCH 4/5] Add test for nested `array_map` return type. --- tests/ReturnTypeTest.php | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/ReturnTypeTest.php b/tests/ReturnTypeTest.php index 7834103df..1241c9bd1 100644 --- a/tests/ReturnTypeTest.php +++ b/tests/ReturnTypeTest.php @@ -1065,6 +1065,29 @@ class ReturnTypeTest extends TestCase } }' ], + 'nestedArrayMapReturnTypeDoesntCrash' => [ + ' Date: Wed, 15 Dec 2021 11:31:54 -0600 Subject: [PATCH 5/5] Rename `$fake_var_id` to `$fake_var_discriminator`. --- .../ArrayFilterReturnTypeProvider.php | 11 ++++--- .../ArrayMapReturnTypeProvider.php | 33 +++++++++---------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php index 9eed10591..6cc324213 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php @@ -186,7 +186,7 @@ class ArrayFilterReturnTypeProvider implements FunctionReturnTypeProviderInterfa if ($array_arg && $mapping_function_ids) { $assertions = []; - $fake_var_id = mt_rand(); + $fake_var_discriminator = mt_rand(); ArrayMapReturnTypeProvider::getReturnTypeFromMappingIds( $statements_source, $mapping_function_ids, @@ -194,7 +194,7 @@ class ArrayFilterReturnTypeProvider implements FunctionReturnTypeProviderInterfa $function_call_arg, array_slice($call_args, 0, 1), $assertions, - $fake_var_id + $fake_var_discriminator ); $array_var_id = ExpressionIdentifier::getArrayVarId( @@ -203,11 +203,12 @@ class ArrayFilterReturnTypeProvider implements FunctionReturnTypeProviderInterfa $statements_source ); - if (isset($assertions[$array_var_id . "[\$__fake_{$fake_var_id}_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_{$fake_var_id}_offset_var__]"] + '$inner_type' => + $assertions["{$array_var_id}[\$__fake_{$fake_var_discriminator}_offset_var__]"], ]; $reconciled_types = Reconciler::reconcileKeyedTypes( @@ -227,7 +228,7 @@ class ArrayFilterReturnTypeProvider implements FunctionReturnTypeProviderInterfa } } - ArrayMapReturnTypeProvider::cleanContext($context, $fake_var_id); + 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) diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php index 3b43aae5f..4437ff462 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php @@ -342,7 +342,8 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface /** * @param non-empty-array $mapping_function_ids * @param list $array_args - * @param int|null $fake_var_id Set the fake variable id to a known value and don't clear it from the context + * @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>>|null $assertions */ public static function getReturnTypeFromMappingIds( @@ -352,7 +353,7 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface PhpParser\Node\Arg $function_call_arg, array $array_args, ?array &$assertions = null, - ?int $fake_var_id = null + ?int $fake_var_discriminator = null ): Union { $mapping_return_type = null; @@ -363,8 +364,8 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface foreach ($mapping_function_ids as $mapping_function_id) { $mapping_function_id_parts = explode('&', $mapping_function_id); - if ($fake_var_id === null) { - $fake_var_id = mt_rand(); + if ($fake_var_discriminator === null) { + $fake_var_discriminator = mt_rand(); $clean_context = true; } @@ -376,7 +377,7 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface new VirtualArrayDimFetch( $array_arg->value, new VirtualVariable( - "__fake_{$fake_var_id}_offset_var__", + "__fake_{$fake_var_discriminator}_offset_var__", $array_arg->value->getAttributes() ), $array_arg->value->getAttributes() @@ -401,7 +402,7 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface if ($is_instance) { $fake_method_call = new VirtualMethodCall( new VirtualVariable( - "__fake_{$fake_var_id}_method_call_var__", + "__fake_{$fake_var_discriminator}_method_call_var__", $function_call_arg->getAttributes() ), new VirtualIdentifier( @@ -427,11 +428,9 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface } } - $context->vars_in_scope["\$__fake_{$fake_var_id}_offset_var__"] = Type::getMixed(); - $context->vars_in_scope["\$__fake_{$fake_var_id}_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, @@ -453,7 +452,7 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface $function_call_arg->getAttributes() ); - $context->vars_in_scope["\$__fake_{$fake_var_id}_offset_var__"] = Type::getMixed(); + $context->vars_in_scope["\$__fake_{$fake_var_discriminator}_offset_var__"] = Type::getMixed(); $fake_method_return_type = self::executeFakeCall( $statements_source, @@ -474,7 +473,7 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface $function_call_arg->getAttributes() ); - $context->vars_in_scope["\$__fake_{$fake_var_id}_offset_var__"] = Type::getMixed(); + $context->vars_in_scope["\$__fake_{$fake_var_discriminator}_offset_var__"] = Type::getMixed(); $fake_function_return_type = self::executeFakeCall( $statements_source, @@ -488,10 +487,10 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface } if ($clean_context) { - self::cleanContext($context, $fake_var_id); + self::cleanContext($context, $fake_var_discriminator); } - $fake_var_id = null; + $fake_var_discriminator = null; $mapping_return_type = Type::combineUnionTypes( $function_id_return_type, @@ -503,10 +502,10 @@ class ArrayMapReturnTypeProvider implements FunctionReturnTypeProviderInterface return $mapping_return_type; } - public static function cleanContext(Context $context, int $fake_var_id): void + 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_id}_")) { + if (str_contains($var_in_scope, "__fake_{$fake_var_discriminator}_")) { unset($context->vars_in_scope[$var_in_scope]); } }