From c0a6fc91252ed54959e0fcebb7c813e5898ccf74 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Fri, 10 Feb 2017 18:12:59 -0500 Subject: [PATCH] Fix #90 - add genericised stubs for common array functions --- psalm.xml | 1 + src/Psalm/Checker/FunctionChecker.php | 151 ++++-------------- src/Psalm/Checker/FunctionLikeChecker.php | 16 +- src/Psalm/Checker/ProjectChecker.php | 7 + .../Statements/Block/ForeachChecker.php | 85 ++++++---- .../Statements/Expression/CallChecker.php | 45 +++--- src/Psalm/Config.php | 16 ++ src/Psalm/Stubs/CoreGenericFunctions.php | 94 +++++++++++ tests/FunctionCallTest.php | 47 +++++- tests/Php71Test.php | 2 +- 10 files changed, 276 insertions(+), 188 deletions(-) create mode 100644 src/Psalm/Stubs/CoreGenericFunctions.php diff --git a/psalm.xml b/psalm.xml index 65fcaa1c0..cab291288 100644 --- a/psalm.xml +++ b/psalm.xml @@ -12,6 +12,7 @@ + diff --git a/src/Psalm/Checker/FunctionChecker.php b/src/Psalm/Checker/FunctionChecker.php index 798b3d369..772361b54 100644 --- a/src/Psalm/Checker/FunctionChecker.php +++ b/src/Psalm/Checker/FunctionChecker.php @@ -27,6 +27,11 @@ class FunctionChecker extends FunctionLikeChecker */ protected static $builtin_functions = []; + /** + * @var array + */ + public static $stubbed_functions = []; + /** * @param mixed $function * @param StatementsSource $source @@ -61,6 +66,10 @@ class FunctionChecker extends FunctionLikeChecker return true; } + if (isset(self::$stubbed_functions[$function_id])) { + return true; + } + if (self::extractReflectionInfo($function_id) === false) { return false; } @@ -75,7 +84,11 @@ class FunctionChecker extends FunctionLikeChecker */ public static function getStorage($function_id, $file_path) { - if (isset(self::$builtin_functions[$function_id]) && self::$builtin_functions[$function_id]) { + if (isset(self::$stubbed_functions[$function_id])) { + return self::$stubbed_functions[$function_id]; + } + + if (isset(self::$builtin_functions[$function_id])) { return self::$builtin_functions[$function_id]; } @@ -149,18 +162,22 @@ class FunctionChecker extends FunctionLikeChecker */ public static function getFunctionReturnType($function_id, $file_path, array $function_template_types = null) { - if (!isset(FileChecker::$storage[$file_path])) { - return null; + if (isset(self::$stubbed_functions[$function_id])) { + $function_return_type = self::$stubbed_functions[$function_id]->return_type; + } else { + if (!isset(FileChecker::$storage[$file_path])) { + return null; + } + + $file_storage = FileChecker::$storage[$file_path]; + + if (!isset($file_storage->functions[$function_id])) { + throw new \InvalidArgumentException('Do not know function ' . $function_id . ' in file ' . $file_path); + } + + $function_return_type = $file_storage->functions[$function_id]->return_type; } - $file_storage = FileChecker::$storage[$file_path]; - - if (!isset($file_storage->functions[$function_id])) { - throw new \InvalidArgumentException('Do not know function ' . $function_id . ' in file ' . $file_path); - } - - $function_return_type = $file_storage->functions[$function_id]->return_type; - if ($function_return_type) { if ($function_template_types) { $type_tokens = Type::tokenize((string)$function_return_type); @@ -433,45 +450,6 @@ class FunctionChecker extends FunctionLikeChecker ? $first_arg->inferredType->types['array'] : null; - if ($call_map_key === 'array_values' || - $call_map_key === 'array_unique' || - $call_map_key === 'array_intersect' || - $call_map_key === 'array_slice' - ) { - if ($first_arg_array_generic || $first_arg_array_objectlike) { - if ($first_arg_array_generic) { - $inner_type = clone $first_arg_array_generic->type_params[1]; - } else { - /** @var Type\Atomic\ObjectLike $first_arg_array_objectlike */ - $inner_type = $first_arg_array_objectlike->getGenericTypeParam(); - } - - return new Type\Union([ - new Type\Atomic\TArray([ - Type::getInt(), - $inner_type - ]) - ]); - } - } - - if ($call_map_key === 'array_keys') { - if ($first_arg_array_generic || $first_arg_array_objectlike) { - if ($first_arg_array_generic) { - $inner_type = clone $first_arg_array_generic->type_params[0]; - } else { - $inner_type = Type::getString(); - } - - return new Type\Union([ - new Type\Atomic\TArray([ - Type::getInt(), - $inner_type - ]) - ]); - } - } - if ($call_map_key === 'array_merge') { $inner_value_types = []; $inner_key_types = []; @@ -510,62 +488,6 @@ class FunctionChecker extends FunctionLikeChecker return Type::getArray(); } - if ($call_map_key === 'array_combine') { - $second_arg_array_generic = $second_arg - && isset($second_arg->inferredType) - && isset($second_arg->inferredType->types['array']) - && $second_arg->inferredType->types['array'] instanceof Type\Atomic\TArray - ? $second_arg->inferredType->types['array'] - : null; - - $second_arg_array_objectlike = $second_arg - && isset($second_arg->inferredType) - && isset($second_arg->inferredType->types['array']) - && $second_arg->inferredType->types['array'] instanceof Type\Atomic\ObjectLike - ? $second_arg->inferredType->types['array'] - : null; - - if ($second_arg_array_generic || $second_arg_array_objectlike) { - if ($first_arg && isset($first_arg->inferredType) && $first_arg->inferredType->hasArray()) { - if ($first_arg_array_generic) { - $keys_inner_type = clone $first_arg_array_generic->type_params[1]; - } else { - /** @var Type\Atomic\ObjectLike $first_arg_array_objectlike */ - $keys_inner_type = $first_arg_array_objectlike->getGenericTypeParam(); - } - } else { - $keys_inner_type = Type::getMixed(); - } - - if ($second_arg_array_generic) { - $values_inner_type = clone $second_arg_array_generic->type_params[1]; - } else { - /** @var Type\Atomic\ObjectLike $second_arg_array_objectlike */ - $values_inner_type = $second_arg_array_objectlike->getGenericTypeParam(); - } - - return new Type\Union([ - new Type\Atomic\TArray([ - $keys_inner_type, - $values_inner_type - ]) - ]); - } - } - - if ($call_map_key === 'array_diff') { - if (!$first_arg_array_generic) { - return Type::getArray(); - } - - return new Type\Union([ - new Type\Atomic\TArray([ - Type::getInt(), - clone $first_arg_array_generic->type_params[1] - ]) - ]); - } - if ($call_map_key === 'array_filter') { if (!$first_arg_array_generic) { return Type::getArray(); @@ -588,22 +510,6 @@ class FunctionChecker extends FunctionLikeChecker ]); } - if ($call_map_key === 'array_diff_key') { - if (!$first_arg_array_generic) { - return Type::getArray(); - } - - return clone $first_arg->inferredType; - } - - if ($call_map_key === 'array_shift' || $call_map_key === 'array_pop') { - if (!$first_arg_array_generic) { - return Type::getMixed(); - } - - return clone $first_arg_array_generic->type_params[1]; - } - return null; } @@ -780,5 +686,6 @@ class FunctionChecker extends FunctionLikeChecker public static function clearCache() { self::$builtin_functions = []; + self::$stubbed_functions = []; } } diff --git a/src/Psalm/Checker/FunctionLikeChecker.php b/src/Psalm/Checker/FunctionLikeChecker.php index 28269b3fa..20f418653 100644 --- a/src/Psalm/Checker/FunctionLikeChecker.php +++ b/src/Psalm/Checker/FunctionLikeChecker.php @@ -485,13 +485,19 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo $cased_function_id = ($namespace ? $namespace . '\\' : '') . $function->name; $function_id = strtolower($cased_function_id); - $file_storage = FileChecker::$storage[$source->getFilePath()]; + $project_checker = $source->getFileChecker()->project_checker; - if (isset($file_storage->functions[$function_id])) { - return $file_storage->functions[$function_id]; + if ($project_checker->register_global_functions) { + $storage = FunctionChecker::$stubbed_functions[$function_id] = new FunctionLikeStorage(); + } else { + $file_storage = FileChecker::$storage[$source->getFilePath()]; + + if (isset($file_storage->functions[$function_id])) { + return $file_storage->functions[$function_id]; + } + + $storage = $file_storage->functions[$function_id] = new FunctionLikeStorage(); } - - $storage = $file_storage->functions[$function_id] = new FunctionLikeStorage(); } elseif ($function instanceof PhpParser\Node\Stmt\ClassMethod) { $fq_class_name = (string)$source->getFQCLN(); diff --git a/src/Psalm/Checker/ProjectChecker.php b/src/Psalm/Checker/ProjectChecker.php index bfe69f1b8..9aec267f0 100644 --- a/src/Psalm/Checker/ProjectChecker.php +++ b/src/Psalm/Checker/ProjectChecker.php @@ -155,6 +155,13 @@ class ProjectChecker */ public $fake_files = []; + /** + * Whether to log functions just at the file level or globally (for stubs) + * + * @var boolean + */ + public $register_global_functions = false; + const TYPE_CONSOLE = 'console'; const TYPE_JSON = 'json'; const TYPE_EMACS = 'emacs'; diff --git a/src/Psalm/Checker/Statements/Block/ForeachChecker.php b/src/Psalm/Checker/Statements/Block/ForeachChecker.php index 776024568..cc9d3073f 100644 --- a/src/Psalm/Checker/Statements/Block/ForeachChecker.php +++ b/src/Psalm/Checker/Statements/Block/ForeachChecker.php @@ -58,41 +58,36 @@ class ForeachChecker } if ($iterator_type) { - foreach ($iterator_type->types as $return_type) { + foreach ($iterator_type->types as $iterator_type) { // if it's an empty array, we cannot iterate over it - if ((string) $return_type === 'array') { + if ((string) $iterator_type === 'array') { continue; } - if ($return_type instanceof Type\Atomic\TArray || $return_type instanceof Type\Atomic\TGenericObject) { - $value_index = count($return_type->type_params) - 1; - $value_type_part = $return_type->type_params[$value_index]; - + if ($iterator_type instanceof Type\Atomic\TArray) { if (!$value_type) { - $value_type = $value_type_part; + $value_type = $iterator_type->type_params[1]; } else { - $value_type = Type::combineUnionTypes($value_type, $value_type_part); + $value_type = Type::combineUnionTypes($value_type, $iterator_type->type_params[1]); } - if ($value_index) { - $key_type_part = $return_type->type_params[0]; + $key_type_part = $iterator_type->type_params[0]; - if (!$key_type) { - $key_type = $key_type_part; - } else { - $key_type = Type::combineUnionTypes($key_type, $key_type_part); - } + if (!$key_type) { + $key_type = $key_type_part; + } else { + $key_type = Type::combineUnionTypes($key_type, $key_type_part); } continue; } - if ($return_type instanceof Type\Atomic\Scalar || - $return_type instanceof Type\Atomic\TNull || - $return_type instanceof Type\Atomic\TVoid + if ($iterator_type instanceof Type\Atomic\Scalar || + $iterator_type instanceof Type\Atomic\TNull || + $iterator_type instanceof Type\Atomic\TVoid ) { if (IssueBuffer::accepts( new InvalidIterator( - 'Cannot iterate over ' . $return_type->getKey(), + 'Cannot iterate over ' . $iterator_type->getKey(), new CodeLocation($statements_checker->getSource(), $stmt->expr) ), $statements_checker->getSuppressedIssues() @@ -101,19 +96,18 @@ class ForeachChecker } $value_type = Type::getMixed(); - } elseif ($return_type instanceof Type\Atomic\TArray || - $return_type instanceof Type\Atomic\TObject || - $return_type instanceof Type\Atomic\TMixed || - $return_type instanceof Type\Atomic\TEmpty || - ($return_type instanceof Type\Atomic\TNamedObject && $return_type->value === 'Generator') + } elseif ($iterator_type instanceof Type\Atomic\TArray || + $iterator_type instanceof Type\Atomic\TObject || + $iterator_type instanceof Type\Atomic\TMixed || + $iterator_type instanceof Type\Atomic\TEmpty ) { $value_type = Type::getMixed(); - } elseif ($return_type instanceof Type\Atomic\TNamedObject) { - if ($return_type->value !== 'Traversable' && - $return_type->value !== $statements_checker->getClassName() + } elseif ($iterator_type instanceof Type\Atomic\TNamedObject) { + if ($iterator_type->value !== 'Traversable' && + $iterator_type->value !== $statements_checker->getClassName() ) { if (ClassLikeChecker::checkFullyQualifiedClassLikeName( - $return_type->value, + $iterator_type->value, $statements_checker->getFileChecker(), new CodeLocation($statements_checker->getSource(), $stmt->expr), $statements_checker->getSuppressedIssues() @@ -122,18 +116,47 @@ class ForeachChecker } } + if ($iterator_type instanceof Type\Atomic\TGenericObject && + (strtolower($iterator_type->value) === 'iterable' || + strtolower($iterator_type->value) === 'traversable' || + ClassChecker::classImplements( + $iterator_type->value, + 'Traversable' + )) + ) { + $value_index = count($iterator_type->type_params) - 1; + $value_type_part = $iterator_type->type_params[$value_index]; + + if (!$value_type) { + $value_type = $value_type_part; + } else { + $value_type = Type::combineUnionTypes($value_type, $value_type_part); + } + + if ($value_index) { + $key_type_part = $iterator_type->type_params[0]; + + if (!$key_type) { + $key_type = $key_type_part; + } else { + $key_type = Type::combineUnionTypes($key_type, $key_type_part); + } + } + continue; + } + if (ClassChecker::classImplements( - $return_type->value, + $iterator_type->value, 'Iterator' )) { - $iterator_method = $return_type->value . '::current'; + $iterator_method = $iterator_type->value . '::current'; $iterator_class_type = MethodChecker::getMethodReturnType($iterator_method); if ($iterator_class_type) { $value_type_part = ExpressionChecker::fleshOutTypes( $iterator_class_type, [], - $return_type->value, + $iterator_type->value, $iterator_method ); diff --git a/src/Psalm/Checker/Statements/Expression/CallChecker.php b/src/Psalm/Checker/Statements/Expression/CallChecker.php index e19639b37..3502859d4 100644 --- a/src/Psalm/Checker/Statements/Expression/CallChecker.php +++ b/src/Psalm/Checker/Statements/Expression/CallChecker.php @@ -123,6 +123,9 @@ class CallChecker if ($context->check_functions) { $in_call_map = false; + $is_stubbed = false; + + $function_storage = null; $function_params = null; $code_location = new CodeLocation($statements_checker->getSource(), $stmt); @@ -185,6 +188,7 @@ class CallChecker $method_id = implode('\\', $stmt->name->parts); $in_call_map = FunctionChecker::inCallMap($method_id); + $is_stubbed = isset(FunctionChecker::$stubbed_functions[strtolower($method_id)]); $is_predefined = true; @@ -201,7 +205,7 @@ class CallChecker //$method_id = $statements_checker->getFQCLN() . '::' . $method_id; } - if (!$in_call_map) { + if (!$in_call_map && !$is_stubbed) { if (self::checkFunctionExists( $statements_checker, $method_id, @@ -211,7 +215,9 @@ class CallChecker ) { return false; } + } + if (!$in_call_map || $is_stubbed) { $function_storage = FunctionChecker::getStorage( strtolower($method_id), $statements_checker->getFilePath() @@ -220,12 +226,11 @@ class CallChecker $function_params = $function_storage->params; if (!$is_predefined) { - $defined_constants = FunctionChecker::getDefinedConstants( - $method_id, - $statements_checker->getFilePath() - ); + $defined_constants = $function_storage->defined_constants; } - } else { + } + + if ($in_call_map && !$is_stubbed) { $function_params = FunctionLikeChecker::getFunctionParamsFromCallMapById( $method_id, $stmt->args, @@ -243,19 +248,10 @@ class CallChecker // fall through } - $function_storage = null; - $generic_params = null; if ($stmt->name instanceof PhpParser\Node\Name && $method_id) { - if (!$in_call_map) { - $function_storage = FunctionChecker::getStorage( - strtolower($method_id), - $statements_checker->getFilePath() - ); - - $function_params = $function_storage->params; - } else { + if (!$is_stubbed && $in_call_map) { $function_params = FunctionLikeChecker::getFunctionParamsFromCallMapById( $method_id, $stmt->args, @@ -282,14 +278,7 @@ class CallChecker } if ($stmt->name instanceof PhpParser\Node\Name && $method_id) { - if ($in_call_map) { - $stmt->inferredType = FunctionChecker::getReturnTypeFromCallMapWithArgs( - $method_id, - $stmt->args, - $code_location, - $statements_checker->getSuppressedIssues() - ); - } else { + if (!$in_call_map || $is_stubbed) { try { $stmt->inferredType = FunctionChecker::getFunctionReturnType( $method_id, @@ -300,6 +289,13 @@ class CallChecker // this can happen when the function was defined in the Config startup script $stmt->inferredType = Type::getMixed(); } + } else { + $stmt->inferredType = FunctionChecker::getReturnTypeFromCallMapWithArgs( + $method_id, + $stmt->args, + $code_location, + $statements_checker->getSuppressedIssues() + ); } } @@ -1556,7 +1552,6 @@ class CallChecker } } - $closure_params = $closure_type->params; $closure_return_type = $closure_type->return_type; diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index f1122edc8..6720306a9 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -574,10 +574,26 @@ class Config */ public function visitStubFiles(ProjectChecker $project_checker) { + $project_checker->register_global_functions = true; + + $generic_stubs = realpath(__DIR__ . '/Stubs/CoreGenericFunctions.php'); + + if ($generic_stubs) { + $generic_stub_checker = new FileChecker( + $generic_stubs, + $project_checker + ); + $generic_stub_checker->visit(); + } else { + throw new \UnexpectedValueException('Cannot locate core generic stubs'); + } + foreach ($this->stub_files as $stub_file) { $stub_checker = new FileChecker($stub_file, $project_checker); $stub_checker->visit(); } + + $project_checker->register_global_functions = false; } /** diff --git a/src/Psalm/Stubs/CoreGenericFunctions.php b/src/Psalm/Stubs/CoreGenericFunctions.php new file mode 100644 index 000000000..fe8ddb3e3 --- /dev/null +++ b/src/Psalm/Stubs/CoreGenericFunctions.php @@ -0,0 +1,94 @@ + $arr + * @return array + */ +function array_keys(array $arr) {} + +/** + * @template T + * + * @param array $arr + * @return array + */ +function array_values(array $arr) {} + +/** + * @template T + * + * @param array $arr + * @return array + */ +function array_unique(array $arr) {} + +/** + * @template T + * + * @param array $arr + * @return array + */ +function array_slice(array $arr) {} + +/** + * @template TKey + * @template TValue + * + * @param array $arr + * @param array $arr2 + * @param array|null $arr3 + * @param array|null $arr4 + * @return array + */ +function array_intersect(array $arr, array $arr2, array $arr3 = null, array $arr4 = null) {} + +/** + * @template TKey + * @template TValue + * + * @param array $arr + * @param array $arr2 + * @return array + */ +function array_combine(array $arr, array $arr2) {} + +/** + * @template TKey + * @template TValue + * + * @param array $arr + * @param array $arr2 + * @param array|null $arr3 + * @param array|null $arr4 + * @return array + */ +function array_diff(array $arr, array $arr2, array $arr3 = null, array $arr4 = null) {} + +/** + * @template TKey + * @template TValue + * + * @param array $arr + * @param array $arr2 + * @param array|null $arr3 + * @param array|null $arr4 + * @return array + */ +function array_diff_key(array $arr, array $arr2, array $arr3 = null, array $arr4 = null) {} + +/** + * @template TValue + * + * @param array $arr + * @return TValue + */ +function array_shift(array $arr) {} + +/** + * @template TValue + * + * @param array $arr + * @return TValue + */ +function array_pop(array $arr) {} diff --git a/tests/FunctionCallTest.php b/tests/FunctionCallTest.php index 1acaafecc..3540b54c2 100644 --- a/tests/FunctionCallTest.php +++ b/tests/FunctionCallTest.php @@ -327,21 +327,60 @@ class FunctionCallTest extends PHPUnit_Framework_TestCase /** * @return void */ - public function testArrayFunctions() + public function testArrayKeys() { $stmts = self::$parser->parse(' 1, "b" => 2]); - $b = array_values(["a" => 1, "b" => 2]); - $c = array_combine(["a", "b", "c"], [1, 2, 3]); - $d = array_merge(["a", "b", "c"], [1, 2, 3]); '); $file_checker = new FileChecker('somefile.php', $this->project_checker, $stmts); $context = new Context(); $file_checker->visitAndAnalyzeMethods($context); $this->assertEquals('array', (string) $context->vars_in_scope['$a']); + } + + /** + * @return void + */ + public function testArrayValues() + { + $stmts = self::$parser->parse(' 1, "b" => 2]); + '); + + $file_checker = new FileChecker('somefile.php', $this->project_checker, $stmts); + $context = new Context(); + $file_checker->visitAndAnalyzeMethods($context); $this->assertEquals('array', (string) $context->vars_in_scope['$b']); + } + + /** + * @return void + */ + public function testArrayCombine() + { + $stmts = self::$parser->parse('project_checker, $stmts); + $context = new Context(); + $file_checker->visitAndAnalyzeMethods($context); $this->assertEquals('array', (string) $context->vars_in_scope['$c']); + } + + /** + * @return void + */ + public function testArrayMerge() + { + $stmts = self::$parser->parse('project_checker, $stmts); + $context = new Context(); + $file_checker->visitAndAnalyzeMethods($context); $this->assertEquals('array', (string) $context->vars_in_scope['$d']); } } diff --git a/tests/Php71Test.php b/tests/Php71Test.php index 6fc027aae..dcf7369b7 100644 --- a/tests/Php71Test.php +++ b/tests/Php71Test.php @@ -379,7 +379,7 @@ class Php71Test extends PHPUnit_Framework_TestCase { $stmts = self::$parser->parse(' $iter + * @param iterable $iter */ function iterator(iterable $iter) : void {