From eb89e0142f4d0dd199eb04b8c2eb5e21c35801b0 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Mon, 12 Oct 2020 13:46:43 -0400 Subject: [PATCH] Fix #4309 - improve reuse of callmap callable inference --- src/Psalm/Internal/Analyzer/FileAnalyzer.php | 1 + .../Internal/Analyzer/FunctionAnalyzer.php | 233 ----------------- .../Expression/Call/FunctionCallAnalyzer.php | 239 +++++++++++++++++- .../Codebase/InternalCallMapHandler.php | 5 + tests/FunctionCallTest.php | 10 + 5 files changed, 253 insertions(+), 235 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/FileAnalyzer.php b/src/Psalm/Internal/Analyzer/FileAnalyzer.php index a02623f9f1d..cdd23eff28c 100644 --- a/src/Psalm/Internal/Analyzer/FileAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FileAnalyzer.php @@ -492,6 +492,7 @@ public static function clearCache(): void \Psalm\Internal\Provider\ClassLikeStorageProvider::deleteAll(); \Psalm\Internal\Provider\FileStorageProvider::deleteAll(); \Psalm\Internal\Provider\FileReferenceProvider::clearCache(); + \Psalm\Internal\Codebase\InternalCallMapHandler::clearCache(); } public function getFileName(): string diff --git a/src/Psalm/Internal/Analyzer/FunctionAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionAnalyzer.php index 461e9b02b44..159d0d8af1a 100644 --- a/src/Psalm/Internal/Analyzer/FunctionAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FunctionAnalyzer.php @@ -43,239 +43,6 @@ public function __construct(PhpParser\Node\Stmt\Function_ $function, SourceAnaly parent::__construct($function, $source, $storage); } - /** - * @param array $call_args - */ - public static function getReturnTypeFromCallMapWithArgs( - StatementsAnalyzer $statements_analyzer, - string $function_id, - array $call_args, - Context $context - ): Type\Union { - $call_map_key = strtolower($function_id); - - $call_map = InternalCallMapHandler::getCallMap(); - - $codebase = $statements_analyzer->getCodebase(); - - if (!isset($call_map[$call_map_key])) { - throw new \InvalidArgumentException('Function ' . $function_id . ' was not found in callmap'); - } - - if (!$call_args) { - switch ($call_map_key) { - case 'hrtime': - return new Type\Union([ - new Type\Atomic\TKeyedArray([ - Type::getInt(), - Type::getInt() - ]) - ]); - - case 'get_called_class': - return new Type\Union([ - new Type\Atomic\TClassString( - $context->self ?: 'object', - $context->self ? new Type\Atomic\TNamedObject($context->self, true) : null - ) - ]); - - case 'get_parent_class': - if ($context->self && $codebase->classExists($context->self)) { - $classlike_storage = $codebase->classlike_storage_provider->get($context->self); - - if ($classlike_storage->parent_classes) { - return new Type\Union([ - new Type\Atomic\TClassString( - array_values($classlike_storage->parent_classes)[0] - ) - ]); - } - } - } - } else { - switch ($call_map_key) { - case 'count': - if (($first_arg_type = $statements_analyzer->node_data->getType($call_args[0]->value))) { - $atomic_types = $first_arg_type->getAtomicTypes(); - - if (count($atomic_types) === 1) { - if (isset($atomic_types['array'])) { - if ($atomic_types['array'] instanceof Type\Atomic\TCallableArray - || $atomic_types['array'] instanceof Type\Atomic\TCallableList - || $atomic_types['array'] instanceof Type\Atomic\TCallableKeyedArray - ) { - return Type::getInt(false, 2); - } - - if ($atomic_types['array'] instanceof Type\Atomic\TNonEmptyArray) { - return new Type\Union([ - $atomic_types['array']->count !== null - ? new Type\Atomic\TLiteralInt($atomic_types['array']->count) - : new Type\Atomic\TInt - ]); - } - - if ($atomic_types['array'] instanceof Type\Atomic\TNonEmptyList) { - return new Type\Union([ - $atomic_types['array']->count !== null - ? new Type\Atomic\TLiteralInt($atomic_types['array']->count) - : new Type\Atomic\TInt - ]); - } - - if ($atomic_types['array'] instanceof Type\Atomic\TKeyedArray - && $atomic_types['array']->sealed - ) { - return new Type\Union([ - new Type\Atomic\TLiteralInt(count($atomic_types['array']->properties)) - ]); - } - } - } - } - - break; - - case 'hrtime': - if (($first_arg_type = $statements_analyzer->node_data->getType($call_args[0]->value))) { - if ((string) $first_arg_type === 'true') { - $int = Type::getInt(); - $int->from_calculation = true; - return $int; - } - - if ((string) $first_arg_type === 'false') { - return new Type\Union([ - new Type\Atomic\TKeyedArray([ - Type::getInt(), - Type::getInt() - ]) - ]); - } - - return new Type\Union([ - new Type\Atomic\TKeyedArray([ - Type::getInt(), - Type::getInt() - ]), - new Type\Atomic\TInt() - ]); - } - - $int = Type::getInt(); - $int->from_calculation = true; - return $int; - - case 'min': - case 'max': - if (isset($call_args[0])) { - $first_arg = $call_args[0]->value; - - if ($first_arg_type = $statements_analyzer->node_data->getType($first_arg)) { - if ($first_arg_type->hasArray()) { - /** @psalm-suppress PossiblyUndefinedStringArrayOffset */ - $array_type = $first_arg_type->getAtomicTypes()['array']; - if ($array_type instanceof Type\Atomic\TKeyedArray) { - return $array_type->getGenericValueType(); - } - - if ($array_type instanceof Type\Atomic\TArray) { - return clone $array_type->type_params[1]; - } - - if ($array_type instanceof Type\Atomic\TList) { - return clone $array_type->type_param; - } - } elseif ($first_arg_type->hasScalarType() - && ($second_arg = ($call_args[1]->value ?? null)) - && ($second_arg_type = $statements_analyzer->node_data->getType($second_arg)) - && $second_arg_type->hasScalarType() - ) { - return Type::combineUnionTypes($first_arg_type, $second_arg_type); - } - } - } - - break; - - case 'get_parent_class': - // this is unreliable, as it's hard to know exactly what's wanted - attempted this in - // https://github.com/vimeo/psalm/commit/355ed831e1c69c96bbf9bf2654ef64786cbe9fd7 - // but caused problems where it didn’t know exactly what level of child we - // were receiving. - // - // Really this should only work on instances we've created with new Foo(), - // but that requires more work - break; - - case 'fgetcsv': - $string_type = Type::getString(); - $string_type->addType(new Type\Atomic\TNull); - $string_type->ignore_nullable_issues = true; - - $call_map_return_type = new Type\Union([ - new Type\Atomic\TNonEmptyList( - $string_type - ), - new Type\Atomic\TFalse, - new Type\Atomic\TNull - ]); - - if ($codebase->config->ignore_internal_nullable_issues) { - $call_map_return_type->ignore_nullable_issues = true; - } - - if ($codebase->config->ignore_internal_falsable_issues) { - $call_map_return_type->ignore_falsable_issues = true; - } - - return $call_map_return_type; - } - } - - if (!$call_map[$call_map_key][0]) { - return Type::getMixed(); - } - - $call_map_return_type = Type::parseString($call_map[$call_map_key][0]); - - switch ($call_map_key) { - case 'mb_strpos': - case 'mb_strrpos': - case 'mb_stripos': - case 'mb_strripos': - case 'strpos': - case 'strrpos': - case 'stripos': - case 'strripos': - case 'strstr': - case 'stristr': - case 'strrchr': - case 'strpbrk': - case 'array_search': - break; - - default: - if ($call_map_return_type->isFalsable() - && $codebase->config->ignore_internal_falsable_issues - ) { - $call_map_return_type->ignore_falsable_issues = true; - } - } - - switch ($call_map_key) { - case 'array_replace': - case 'array_replace_recursive': - if ($codebase->config->ignore_internal_nullable_issues) { - $call_map_return_type->ignore_nullable_issues = true; - } - break; - } - - return $call_map_return_type; - } - /** * @return non-empty-lowercase-string */ diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php index b014f9cbc99..14de9c57b41 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php @@ -31,6 +31,7 @@ use Psalm\Storage\Assertion; use Psalm\Storage\FunctionLikeStorage; use Psalm\Type; +use Psalm\Type\Atomic\TCallable; use Psalm\Type\Atomic\TCallableObject; use Psalm\Type\Atomic\TCallableString; use Psalm\Type\Atomic\TTemplateParam; @@ -314,6 +315,7 @@ public static function analyze( } $template_result = null; + $function_callable = null; if ($function_exists) { if ($function_name instanceof PhpParser\Node\Name && $function_id) { @@ -365,6 +367,7 @@ public static function analyze( $in_call_map, $is_stubbed, $function_storage, + $function_callable, $template_result, $context ); @@ -574,7 +577,7 @@ private static function getAnalyzeNamedExpression( $has_valid_function_call_type = false; foreach ($stmt_name_type->getAtomicTypes() as $var_type_part) { - if ($var_type_part instanceof Type\Atomic\TClosure || $var_type_part instanceof Type\Atomic\TCallable) { + if ($var_type_part instanceof Type\Atomic\TClosure || $var_type_part instanceof TCallable) { if (!$var_type_part->is_pure && $context->pure) { if (IssueBuffer::accepts( new ImpureFunctionCall( @@ -921,6 +924,7 @@ private static function getFunctionCallReturnType( bool $in_call_map, bool $is_stubbed, ?FunctionLikeStorage $function_storage, + ?TCallable $callmap_callable, TemplateResult $template_result, Context $context ) : Type\Union { @@ -1039,10 +1043,15 @@ private static function getFunctionCallReturnType( $stmt_type = Type::getMixed(); } } else { - $stmt_type = FunctionAnalyzer::getReturnTypeFromCallMapWithArgs( + if (!$callmap_callable) { + throw new \UnexpectedValueException('We should have a callmap callable here'); + } + + $stmt_type = self::getReturnTypeFromCallMapWithArgs( $statements_analyzer, $function_id, $stmt->args, + $callmap_callable, $context ); } @@ -1060,6 +1069,232 @@ private static function getFunctionCallReturnType( return $stmt_type; } + /** + * @param array $call_args + */ + private static function getReturnTypeFromCallMapWithArgs( + StatementsAnalyzer $statements_analyzer, + string $function_id, + array $call_args, + TCallable $callmap_callable, + Context $context + ): Type\Union { + $call_map_key = strtolower($function_id); + + $codebase = $statements_analyzer->getCodebase(); + + if (!$call_args) { + switch ($call_map_key) { + case 'hrtime': + return new Type\Union([ + new Type\Atomic\TKeyedArray([ + Type::getInt(), + Type::getInt() + ]) + ]); + + case 'get_called_class': + return new Type\Union([ + new Type\Atomic\TClassString( + $context->self ?: 'object', + $context->self ? new Type\Atomic\TNamedObject($context->self, true) : null + ) + ]); + + case 'get_parent_class': + if ($context->self && $codebase->classExists($context->self)) { + $classlike_storage = $codebase->classlike_storage_provider->get($context->self); + + if ($classlike_storage->parent_classes) { + return new Type\Union([ + new Type\Atomic\TClassString( + array_values($classlike_storage->parent_classes)[0] + ) + ]); + } + } + } + } else { + switch ($call_map_key) { + case 'count': + if (($first_arg_type = $statements_analyzer->node_data->getType($call_args[0]->value))) { + $atomic_types = $first_arg_type->getAtomicTypes(); + + if (count($atomic_types) === 1) { + if (isset($atomic_types['array'])) { + if ($atomic_types['array'] instanceof Type\Atomic\TCallableArray + || $atomic_types['array'] instanceof Type\Atomic\TCallableList + || $atomic_types['array'] instanceof Type\Atomic\TCallableKeyedArray + ) { + return Type::getInt(false, 2); + } + + if ($atomic_types['array'] instanceof Type\Atomic\TNonEmptyArray) { + return new Type\Union([ + $atomic_types['array']->count !== null + ? new Type\Atomic\TLiteralInt($atomic_types['array']->count) + : new Type\Atomic\TInt + ]); + } + + if ($atomic_types['array'] instanceof Type\Atomic\TNonEmptyList) { + return new Type\Union([ + $atomic_types['array']->count !== null + ? new Type\Atomic\TLiteralInt($atomic_types['array']->count) + : new Type\Atomic\TInt + ]); + } + + if ($atomic_types['array'] instanceof Type\Atomic\TKeyedArray + && $atomic_types['array']->sealed + ) { + return new Type\Union([ + new Type\Atomic\TLiteralInt(count($atomic_types['array']->properties)) + ]); + } + } + } + } + + break; + + case 'hrtime': + if (($first_arg_type = $statements_analyzer->node_data->getType($call_args[0]->value))) { + if ((string) $first_arg_type === 'true') { + $int = Type::getInt(); + $int->from_calculation = true; + return $int; + } + + if ((string) $first_arg_type === 'false') { + return new Type\Union([ + new Type\Atomic\TKeyedArray([ + Type::getInt(), + Type::getInt() + ]) + ]); + } + + return new Type\Union([ + new Type\Atomic\TKeyedArray([ + Type::getInt(), + Type::getInt() + ]), + new Type\Atomic\TInt() + ]); + } + + $int = Type::getInt(); + $int->from_calculation = true; + return $int; + + case 'min': + case 'max': + if (isset($call_args[0])) { + $first_arg = $call_args[0]->value; + + if ($first_arg_type = $statements_analyzer->node_data->getType($first_arg)) { + if ($first_arg_type->hasArray()) { + /** @psalm-suppress PossiblyUndefinedStringArrayOffset */ + $array_type = $first_arg_type->getAtomicTypes()['array']; + if ($array_type instanceof Type\Atomic\TKeyedArray) { + return $array_type->getGenericValueType(); + } + + if ($array_type instanceof Type\Atomic\TArray) { + return clone $array_type->type_params[1]; + } + + if ($array_type instanceof Type\Atomic\TList) { + return clone $array_type->type_param; + } + } elseif ($first_arg_type->hasScalarType() + && ($second_arg = ($call_args[1]->value ?? null)) + && ($second_arg_type = $statements_analyzer->node_data->getType($second_arg)) + && $second_arg_type->hasScalarType() + ) { + return Type::combineUnionTypes($first_arg_type, $second_arg_type); + } + } + } + + break; + + case 'get_parent_class': + // this is unreliable, as it's hard to know exactly what's wanted - attempted this in + // https://github.com/vimeo/psalm/commit/355ed831e1c69c96bbf9bf2654ef64786cbe9fd7 + // but caused problems where it didn’t know exactly what level of child we + // were receiving. + // + // Really this should only work on instances we've created with new Foo(), + // but that requires more work + break; + + case 'fgetcsv': + $string_type = Type::getString(); + $string_type->addType(new Type\Atomic\TNull); + $string_type->ignore_nullable_issues = true; + + $call_map_return_type = new Type\Union([ + new Type\Atomic\TNonEmptyList( + $string_type + ), + new Type\Atomic\TFalse, + new Type\Atomic\TNull + ]); + + if ($codebase->config->ignore_internal_nullable_issues) { + $call_map_return_type->ignore_nullable_issues = true; + } + + if ($codebase->config->ignore_internal_falsable_issues) { + $call_map_return_type->ignore_falsable_issues = true; + } + + return $call_map_return_type; + } + } + + $stmt_type = $callmap_callable->return_type + ? clone $callmap_callable->return_type + : Type::getMixed(); + + switch ($function_id) { + case 'mb_strpos': + case 'mb_strrpos': + case 'mb_stripos': + case 'mb_strripos': + case 'strpos': + case 'strrpos': + case 'stripos': + case 'strripos': + case 'strstr': + case 'stristr': + case 'strrchr': + case 'strpbrk': + case 'array_search': + break; + + default: + if ($stmt_type->isFalsable() + && $codebase->config->ignore_internal_falsable_issues + ) { + $stmt_type->ignore_falsable_issues = true; + } + } + + switch ($call_map_key) { + case 'array_replace': + case 'array_replace_recursive': + if ($codebase->config->ignore_internal_nullable_issues) { + $stmt_type->ignore_nullable_issues = true; + } + break; + } + + return $stmt_type; + } + private static function taintReturnType( StatementsAnalyzer $statements_analyzer, PhpParser\Node\Expr\FuncCall $stmt, diff --git a/src/Psalm/Internal/Codebase/InternalCallMapHandler.php b/src/Psalm/Internal/Codebase/InternalCallMapHandler.php index 69168953c64..30361679a90 100644 --- a/src/Psalm/Internal/Codebase/InternalCallMapHandler.php +++ b/src/Psalm/Internal/Codebase/InternalCallMapHandler.php @@ -414,4 +414,9 @@ public static function inCallMap(string $key): bool { return isset(self::getCallMap()[strtolower($key)]); } + + public static function clearCache() : void + { + self::$call_map_callables = []; + } } diff --git a/tests/FunctionCallTest.php b/tests/FunctionCallTest.php index b3080a7cfc3..e3f6c2fbbb3 100644 --- a/tests/FunctionCallTest.php +++ b/tests/FunctionCallTest.php @@ -1362,6 +1362,16 @@ function foo(string $s) { return preg_split("/ /", $s); }' ], + 'mbConvertEncodingWithArray' => [ + ' $str + * @return array + */ + function test2(array $str): array { + return mb_convert_encoding($str, "UTF-8", "UTF-8"); + }' + ], ]; }