From 11240eb1935a936148cb1346d22dc42343d5ff36 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Tue, 19 Jun 2018 13:19:34 -0400 Subject: [PATCH] Fix #826 allow better template replacements --- src/Psalm/Checker/CommentChecker.php | 22 +++++--- .../Statements/Expression/CallChecker.php | 14 +---- src/Psalm/Codebase/Reflection.php | 2 +- .../Scanner/ClassLikeDocblockComment.php | 2 +- src/Psalm/Scanner/FunctionDocblockComment.php | 2 +- src/Psalm/Storage/ClassLikeStorage.php | 2 +- src/Psalm/Storage/FunctionLikeStorage.php | 2 +- src/Psalm/Type.php | 56 +++++++++---------- src/Psalm/Type/Atomic.php | 8 +-- src/Psalm/Type/Atomic/CallableTrait.php | 2 +- src/Psalm/Type/Atomic/GenericTrait.php | 2 +- src/Psalm/Type/Union.php | 11 ++-- src/Psalm/Visitor/DependencyFinderVisitor.php | 29 ++++++---- tests/BinaryOperationTest.php | 1 + tests/TemplateTest.php | 28 ++++++++++ 15 files changed, 106 insertions(+), 77 deletions(-) diff --git a/src/Psalm/Checker/CommentChecker.php b/src/Psalm/Checker/CommentChecker.php index 08e338c1a..55194fa08 100644 --- a/src/Psalm/Checker/CommentChecker.php +++ b/src/Psalm/Checker/CommentChecker.php @@ -18,7 +18,7 @@ class CommentChecker /** * @param string $comment * @param Aliases $aliases - * @param array|null $template_types + * @param array|null $template_type_names * @param int|null $var_line_number * @param int|null $came_from_line_number what line number in $source that $comment came from * @@ -31,7 +31,7 @@ class CommentChecker $comment, FileSource $source, Aliases $aliases, - array $template_types = null, + array $template_type_names = null, $var_line_number = null, $came_from_line_number = null ) { @@ -74,7 +74,7 @@ class CommentChecker $var_type_tokens = Type::fixUpLocalType( $line_parts[0], $aliases, - $template_types + $template_type_names ); } catch (TypeParseTreeException $e) { throw new DocblockParseException($line_parts[0] . ' is not a valid type'); @@ -94,7 +94,7 @@ class CommentChecker } try { - $defined_type = Type::parseTokens($var_type_tokens, false, $template_types ?: []); + $defined_type = Type::parseTokens($var_type_tokens, false, $template_type_names ?: []); } catch (TypeParseTreeException $e) { if (is_int($came_from_line_number)) { throw new DocblockParseException( @@ -277,9 +277,12 @@ class CommentChecker $template_type = preg_split('/[\s]+/', $template_line); if (count($template_type) > 2 && in_array(strtolower($template_type[1]), ['as', 'super'], true)) { - $info->template_types[] = [$template_type[0], strtolower($template_type[1]), $template_type[2]]; + $info->template_type_names[] = [ + $template_type[0], + strtolower($template_type[1]), $template_type[2] + ]; } else { - $info->template_types[] = [$template_type[0]]; + $info->template_type_names[] = [$template_type[0]]; } } } @@ -371,9 +374,12 @@ class CommentChecker $template_type = preg_split('/[\s]+/', $template_line); if (count($template_type) > 2 && in_array(strtolower($template_type[1]), ['as', 'super'], true)) { - $info->template_types[] = [$template_type[0], strtolower($template_type[1]), $template_type[2]]; + $info->template_type_names[] = [ + $template_type[0], + strtolower($template_type[1]), $template_type[2] + ]; } else { - $info->template_types[] = [$template_type[0]]; + $info->template_type_names[] = [$template_type[0]]; } } } diff --git a/src/Psalm/Checker/Statements/Expression/CallChecker.php b/src/Psalm/Checker/Statements/Expression/CallChecker.php index 0761e2296..ea0c5c89a 100644 --- a/src/Psalm/Checker/Statements/Expression/CallChecker.php +++ b/src/Psalm/Checker/Statements/Expression/CallChecker.php @@ -661,19 +661,7 @@ class CallChecker } } - if ($generic_params) { - $existing_generic_params_to_strings = array_map( - /** - * @return string - */ - function (Type\Union $type) { - return (string) $type; - }, - $generic_params - ); - } else { - $existing_generic_params_to_strings = []; - } + $existing_generic_params_to_strings = $generic_params ?: []; foreach ($args as $argument_offset => $arg) { $function_param = count($function_params) > $argument_offset diff --git a/src/Psalm/Codebase/Reflection.php b/src/Psalm/Codebase/Reflection.php index f02155b8e..c31765959 100644 --- a/src/Psalm/Codebase/Reflection.php +++ b/src/Psalm/Codebase/Reflection.php @@ -159,7 +159,7 @@ class Reflection ); if ($class_name_lower === 'generator') { - $storage->template_types = ['TKey' => 'mixed', 'TValue' => 'mixed']; + $storage->template_types = ['TKey' => Type::getMixed(), 'TValue' => Type::getMixed()]; } $interfaces = $reflected_class->getInterfaces(); diff --git a/src/Psalm/Scanner/ClassLikeDocblockComment.php b/src/Psalm/Scanner/ClassLikeDocblockComment.php index 5edd5121c..1495900ce 100644 --- a/src/Psalm/Scanner/ClassLikeDocblockComment.php +++ b/src/Psalm/Scanner/ClassLikeDocblockComment.php @@ -13,7 +13,7 @@ class ClassLikeDocblockComment /** * @var array> */ - public $template_types = []; + public $template_type_names = []; /** * @var array diff --git a/src/Psalm/Scanner/FunctionDocblockComment.php b/src/Psalm/Scanner/FunctionDocblockComment.php index a9c87b256..4f3f1d1e3 100644 --- a/src/Psalm/Scanner/FunctionDocblockComment.php +++ b/src/Psalm/Scanner/FunctionDocblockComment.php @@ -57,7 +57,7 @@ class FunctionDocblockComment /** * @var array> */ - public $template_types = []; + public $template_type_names = []; /** * @var array diff --git a/src/Psalm/Storage/ClassLikeStorage.php b/src/Psalm/Storage/ClassLikeStorage.php index 0c24910e5..835c76a0d 100644 --- a/src/Psalm/Storage/ClassLikeStorage.php +++ b/src/Psalm/Storage/ClassLikeStorage.php @@ -201,7 +201,7 @@ class ClassLikeStorage public $overridden_property_ids = []; /** - * @var array|null + * @var array|null */ public $template_types; diff --git a/src/Psalm/Storage/FunctionLikeStorage.php b/src/Psalm/Storage/FunctionLikeStorage.php index ba2686a69..1502ba14b 100644 --- a/src/Psalm/Storage/FunctionLikeStorage.php +++ b/src/Psalm/Storage/FunctionLikeStorage.php @@ -87,7 +87,7 @@ class FunctionLikeStorage public $global_types = []; /** - * @var array|null + * @var array|null */ public $template_types; diff --git a/src/Psalm/Type.php b/src/Psalm/Type.php index 3fa7fe3a5..be6b22fa9 100644 --- a/src/Psalm/Type.php +++ b/src/Psalm/Type.php @@ -74,13 +74,13 @@ abstract class Type * * @param string $type_string * @param bool $php_compatible - * @param array $template_types + * @param array $template_type_names * * @return Union */ - public static function parseString($type_string, $php_compatible = false, array $template_types = []) + public static function parseString($type_string, $php_compatible = false, array $template_type_names = []) { - return self::parseTokens(self::tokenize($type_string), $php_compatible, $template_types); + return self::parseTokens(self::tokenize($type_string), $php_compatible, $template_type_names); } /** @@ -88,11 +88,11 @@ abstract class Type * * @param array $type_tokens * @param bool $php_compatible - * @param array $template_types + * @param array $template_type_names * * @return Union */ - public static function parseTokens(array $type_tokens, $php_compatible = false, array $template_types = []) + public static function parseTokens(array $type_tokens, $php_compatible = false, array $template_type_names = []) { if (count($type_tokens) === 1) { $only_token = $type_tokens[0]; @@ -104,12 +104,12 @@ abstract class Type $only_token = self::fixScalarTerms($only_token, $php_compatible); - return new Union([Atomic::create($only_token, $php_compatible, $template_types)]); + return new Union([Atomic::create($only_token, $php_compatible, $template_type_names)]); } try { $parse_tree = ParseTree::createFromTokens($type_tokens); - $parsed_type = self::getTypeFromTree($parse_tree, $php_compatible, $template_types); + $parsed_type = self::getTypeFromTree($parse_tree, $php_compatible, $template_type_names); } catch (TypeParseTreeException $e) { throw $e; } @@ -168,11 +168,11 @@ abstract class Type /** * @param ParseTree $parse_tree * @param bool $php_compatible - * @param array $template_types + * @param array $template_type_names * * @return Atomic|TArray|TGenericObject|ObjectLike|Union */ - private static function getTypeFromTree(ParseTree $parse_tree, $php_compatible, array $template_types) + private static function getTypeFromTree(ParseTree $parse_tree, $php_compatible, array $template_type_names) { if ($parse_tree instanceof ParseTree\GenericTree) { $generic_type = $parse_tree->value; @@ -181,8 +181,8 @@ abstract class Type /** * @return Union */ - function (ParseTree $child_tree) use ($template_types) { - $tree_type = self::getTypeFromTree($child_tree, false, $template_types); + function (ParseTree $child_tree) use ($template_type_names) { + $tree_type = self::getTypeFromTree($child_tree, false, $template_type_names); return $tree_type instanceof Union ? $tree_type : new Union([$tree_type]); }, @@ -215,10 +215,10 @@ abstract class Type foreach ($parse_tree->children as $child_tree) { if ($child_tree instanceof ParseTree\NullableTree) { - $atomic_type = self::getTypeFromTree($child_tree->children[0], false, $template_types); + $atomic_type = self::getTypeFromTree($child_tree->children[0], false, $template_type_names); $has_null = true; } else { - $atomic_type = self::getTypeFromTree($child_tree, false, $template_types); + $atomic_type = self::getTypeFromTree($child_tree, false, $template_type_names); } if (!$atomic_type instanceof Atomic) { @@ -242,8 +242,8 @@ abstract class Type /** * @return Atomic */ - function (ParseTree $child_tree) use ($template_types) { - $atomic_type = self::getTypeFromTree($child_tree, false, $template_types); + function (ParseTree $child_tree) use ($template_type_names) { + $atomic_type = self::getTypeFromTree($child_tree, false, $template_type_names); if (!$atomic_type instanceof Atomic) { throw new TypeParseTreeException( @@ -277,11 +277,11 @@ abstract class Type foreach ($parse_tree->children as $i => $property_branch) { if (!$property_branch instanceof ParseTree\ObjectLikePropertyTree) { - $property_type = self::getTypeFromTree($property_branch, false, $template_types); + $property_type = self::getTypeFromTree($property_branch, false, $template_type_names); $property_maybe_undefined = false; $property_key = (string)$i; } elseif (count($property_branch->children) === 1) { - $property_type = self::getTypeFromTree($property_branch->children[0], false, $template_types); + $property_type = self::getTypeFromTree($property_branch->children[0], false, $template_type_names); $property_maybe_undefined = $property_branch->possibly_undefined; $property_key = $property_branch->value; } else { @@ -313,7 +313,7 @@ abstract class Type } if ($parse_tree instanceof ParseTree\CallableWithReturnTypeTree) { - $callable_type = self::getTypeFromTree($parse_tree->children[0], false, $template_types); + $callable_type = self::getTypeFromTree($parse_tree->children[0], false, $template_type_names); if (!$callable_type instanceof TCallable && !$callable_type instanceof Type\Atomic\Fn) { throw new \InvalidArgumentException('Parsing callable tree node should return TCallable'); @@ -323,7 +323,7 @@ abstract class Type throw new TypeParseTreeException('Invalid return type'); } - $return_type = self::getTypeFromTree($parse_tree->children[1], false, $template_types); + $return_type = self::getTypeFromTree($parse_tree->children[1], false, $template_type_names); $callable_type->return_type = $return_type instanceof Union ? $return_type : new Union([$return_type]); @@ -335,16 +335,16 @@ abstract class Type /** * @return FunctionLikeParameter */ - function (ParseTree $child_tree) use ($template_types) { + function (ParseTree $child_tree) use ($template_type_names) { $is_variadic = false; $is_optional = false; if ($child_tree instanceof ParseTree\CallableParamTree) { - $tree_type = self::getTypeFromTree($child_tree->children[0], false, $template_types); + $tree_type = self::getTypeFromTree($child_tree->children[0], false, $template_type_names); $is_variadic = $child_tree->variadic; $is_optional = $child_tree->has_default; } else { - $tree_type = self::getTypeFromTree($child_tree, false, $template_types); + $tree_type = self::getTypeFromTree($child_tree, false, $template_type_names); } $tree_type = $tree_type instanceof Union ? $tree_type : new Union([$tree_type]); @@ -371,11 +371,11 @@ abstract class Type } if ($parse_tree instanceof ParseTree\EncapsulationTree) { - return self::getTypeFromTree($parse_tree->children[0], false, $template_types); + return self::getTypeFromTree($parse_tree->children[0], false, $template_type_names); } if ($parse_tree instanceof ParseTree\NullableTree) { - $atomic_type = self::getTypeFromTree($parse_tree->children[0], false, $template_types); + $atomic_type = self::getTypeFromTree($parse_tree->children[0], false, $template_type_names); if (!$atomic_type instanceof Atomic) { throw new \UnexpectedValueException( @@ -412,7 +412,7 @@ abstract class Type $atomic_type = self::fixScalarTerms($parse_tree->value, $php_compatible); - return Atomic::create($atomic_type, $php_compatible, $template_types); + return Atomic::create($atomic_type, $php_compatible, $template_type_names); } /** @@ -570,14 +570,14 @@ abstract class Type /** * @param string $string_type * @param Aliases $aliases - * @param array|null $template_types + * @param array|null $template_type_names * * @return array */ public static function fixUpLocalType( $string_type, Aliases $aliases, - array $template_types = null + array $template_type_names = null ) { $type_tokens = self::tokenize($string_type); @@ -614,7 +614,7 @@ abstract class Type continue; } - if (isset($template_types[$string_type_token])) { + if (isset($template_type_names[$string_type_token])) { continue; } diff --git a/src/Psalm/Type/Atomic.php b/src/Psalm/Type/Atomic.php index 0b5e97fd3..82b1c54a6 100644 --- a/src/Psalm/Type/Atomic.php +++ b/src/Psalm/Type/Atomic.php @@ -53,11 +53,11 @@ abstract class Atomic /** * @param string $value * @param bool $php_compatible - * @param array $template_types + * @param array $template_type_names * * @return Atomic */ - public static function create($value, $php_compatible = false, array $template_types = []) + public static function create($value, $php_compatible = false, array $template_type_names = []) { switch ($value) { case 'int': @@ -126,7 +126,7 @@ abstract class Atomic throw new \Psalm\Exception\TypeParseTreeException('First character of type cannot be numeric'); } - if (isset($template_types[$value])) { + if (isset($template_type_names[$value])) { return new TGenericParam($value); } @@ -358,7 +358,7 @@ abstract class Atomic } /** - * @param array $template_types + * @param array $template_types * @param array $generic_params * @param Type\Atomic|null $input_type * diff --git a/src/Psalm/Type/Atomic/CallableTrait.php b/src/Psalm/Type/Atomic/CallableTrait.php index db2eb08a9..ceb03fe51 100644 --- a/src/Psalm/Type/Atomic/CallableTrait.php +++ b/src/Psalm/Type/Atomic/CallableTrait.php @@ -133,7 +133,7 @@ trait CallableTrait } /** - * @param array $template_types + * @param array $template_types * @param array $generic_params * @param Atomic|null $input_type * diff --git a/src/Psalm/Type/Atomic/GenericTrait.php b/src/Psalm/Type/Atomic/GenericTrait.php index 66c6cc74a..be6429dfe 100644 --- a/src/Psalm/Type/Atomic/GenericTrait.php +++ b/src/Psalm/Type/Atomic/GenericTrait.php @@ -132,7 +132,7 @@ trait GenericTrait } /** - * @param array $template_types + * @param array $template_types * @param array $generic_params * @param Atomic|null $input_type * diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php index bc460ca6a..db7fb1e6a 100644 --- a/src/Psalm/Type/Union.php +++ b/src/Psalm/Type/Union.php @@ -727,9 +727,9 @@ class Union } /** - * @param array $template_types - * @param array $generic_params - * @param Type\Union|null $input_type + * @param array $template_types + * @param array $generic_params + * @param Type\Union|null $input_type * * @return void */ @@ -743,9 +743,10 @@ class Union foreach ($this->types as $key => $atomic_type) { if (isset($template_types[$key])) { - if ($template_types[$key] !== $key) { + if ($template_types[$key]->getId() !== $key) { $keys_to_unset[] = $key; - $this->types[$template_types[$key]] = Atomic::create($template_types[$key]); + $first_atomic_type = array_values($template_types[$key]->getTypes())[0]; + $this->types[$first_atomic_type->getKey()] = clone $first_atomic_type; if ($input_type) { $generic_params[$key] = clone $input_type; diff --git a/src/Psalm/Visitor/DependencyFinderVisitor.php b/src/Psalm/Visitor/DependencyFinderVisitor.php index 812b3acb2..227e15260 100644 --- a/src/Psalm/Visitor/DependencyFinderVisitor.php +++ b/src/Psalm/Visitor/DependencyFinderVisitor.php @@ -225,18 +225,19 @@ class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements P } if ($docblock_info) { - if ($docblock_info->template_types) { + if ($docblock_info->template_type_names) { $storage->template_types = []; - foreach ($docblock_info->template_types as $template_type) { + foreach ($docblock_info->template_type_names as $template_type) { if (count($template_type) === 3) { - $as_type_string = Type::getFQCLNFromString( - $template_type[2], - $this->aliases + $storage->template_types[$template_type[0]] = Type::parseTokens( + Type::fixUpLocalType( + $template_type[2], + $this->aliases + ) ); - $storage->template_types[$template_type[0]] = $as_type_string; } else { - $storage->template_types[$template_type[0]] = 'mixed'; + $storage->template_types[$template_type[0]] = Type::getMixed(); } } @@ -1029,15 +1030,19 @@ class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements P $template_types = $class_storage && $class_storage->template_types ? $class_storage->template_types : null; - if ($docblock_info->template_types) { + if ($docblock_info->template_type_names) { $storage->template_types = []; - foreach ($docblock_info->template_types as $template_type) { + foreach ($docblock_info->template_type_names as $template_type) { if (count($template_type) === 3) { - $as_type_string = Type::getFQCLNFromString($template_type[2], $this->aliases); - $storage->template_types[$template_type[0]] = $as_type_string; + $storage->template_types[$template_type[0]] = Type::parseTokens( + Type::fixUpLocalType( + $template_type[2], + $this->aliases + ) + ); } else { - $storage->template_types[$template_type[0]] = 'mixed'; + $storage->template_types[$template_type[0]] = Type::getMixed(); } } diff --git a/tests/BinaryOperationTest.php b/tests/BinaryOperationTest.php index e5cb3778a..b378560fa 100644 --- a/tests/BinaryOperationTest.php +++ b/tests/BinaryOperationTest.php @@ -138,6 +138,7 @@ class BinaryOperationTest extends TestCase $b += 1;', 'assertions' => [ '$a' => 'float', + '$b' => 'float', ], ], ]; diff --git a/tests/TemplateTest.php b/tests/TemplateTest.php index 6c6bdf87c..2a2d75dc7 100644 --- a/tests/TemplateTest.php +++ b/tests/TemplateTest.php @@ -656,6 +656,34 @@ class TemplateTest extends TestCase class A {} class B {}' ], + 'collectionOfClosure' => [ + ' + */ + public function filter(Closure $p); + } + class I {} + + /** @var Collection> $c */ + $c = new Collection; + + $c->filter( + /** @param Collection $elt */ + function(Collection $elt): bool { return (bool) rand(0,1); } + ); + + $c->filter( + /** @param Collection $elt */ + function(Collection $elt): bool { return true; } + );', + ], ]; }