From 9ef1ce153503ad76e1dfe19a465372363d5d2fd5 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Sat, 12 Jan 2019 18:18:23 -0500 Subject: [PATCH] Make @template-extends more robust --- .../Expression/Call/MethodCallAnalyzer.php | 21 ++ .../Expression/Call/NewAnalyzer.php | 40 ++++ .../Statements/Expression/CallAnalyzer.php | 91 +++++--- src/Psalm/Internal/Codebase/Populator.php | 8 + src/Psalm/Type.php | 4 + tests/TemplateTest.php | 216 ++++++++++++++++++ 6 files changed, 351 insertions(+), 29 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php index 966cc48f3..7761ad76e 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php @@ -1054,6 +1054,25 @@ class MethodCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\ $type_extends->param_name, array_keys($calling_class_storage->template_types) ); + $class_template_params[$type_name] = [ + $lhs_type_part->type_params[(int) $mapped_offset], + $class_storage->name, + ]; + } elseif ($type_extends->defining_class + && isset( + $calling_class_storage + ->template_type_extends + [strtolower($type_extends->defining_class)] + [$type_extends->param_name] + ) + ) { + $mapped_offset = array_search( + $type_extends->param_name, + array_keys($calling_class_storage + ->template_type_extends + [strtolower($type_extends->defining_class)]) + ); + $class_template_params[$type_name] = [ $lhs_type_part->type_params[(int) $mapped_offset], $class_storage->name, @@ -1073,6 +1092,8 @@ class MethodCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\ $i++; } + + //var_dump($class_template_params); } else { foreach ($class_storage->template_types as $type_name => list($type)) { if ($class_storage !== $calling_class_storage diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php index 5bc7d4ac8..6ce27eb04 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php @@ -357,6 +357,12 @@ class NewAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\CallAna foreach ($storage->template_types as $template_name => $_) { if (isset($found_generic_params[$template_name])) { $generic_params[] = $found_generic_params[$template_name]; + } elseif ($storage->template_type_extends && $found_generic_params) { + $generic_params[] = self::getGenericParamForOffset( + $template_name, + $storage->template_type_extends, + $found_generic_params + ); } else { $generic_params[] = [Type::getMixed(), null]; } @@ -444,4 +450,38 @@ class NewAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\CallAna return null; } + + /** + * @param string $template_name + * @param array> $template_type_extends + * @param array $found_generic_params + * @return array{Type\Union, ?string} + */ + private static function getGenericParamForOffset( + string $template_name, + array $template_type_extends, + array $found_generic_params + ) { + if (isset($found_generic_params[$template_name])) { + return $found_generic_params[$template_name]; + } + + foreach ($template_type_extends as $type_map) { + //var_dump($template_name, $type_map); + foreach ($type_map as $extended_template_name => $extended_type) { + if (is_string($extended_template_name) + && $extended_type instanceof Type\Atomic\TGenericParam + && $extended_type->param_name === $template_name + ) { + return self::getGenericParamForOffset( + $extended_template_name, + $template_type_extends, + $found_generic_params + ); + } + } + } + + return [Type::getMixed(), null]; + } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php index 075d0603c..638109d22 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php @@ -869,35 +869,11 @@ class CallAnalyzer $template_types = null; if ($function_storage) { - $template_types = []; - - if ($function_storage->template_types) { - $template_types = $function_storage->template_types; - } - if ($class_storage) { - if ($calling_class_storage - && $class_storage !== $calling_class_storage - && $calling_class_storage->template_type_extends - ) { - foreach ($calling_class_storage->template_type_extends as $class_name_lc => $type_map) { - foreach ($type_map as $template_name => $type) { - if (is_string($template_name) - && $class_name_lc === strtolower($class_storage->name) - ) { - $template_types[$template_name] = [new Type\Union([$type]), null]; - } - } - } - } elseif ($class_storage->template_types) { - foreach ($class_storage->template_types as $template_name => $type) { - $template_types[$template_name] = $type; - } - } - } - - foreach ($template_types as $key => $type) { - $template_types[$key][0] = clone $type[0]; - } + $template_types = self::getTemplateTypesForFunction( + $function_storage, + $class_storage, + $calling_class_storage + ); } $existing_generic_params = $generic_params ?: []; @@ -1239,6 +1215,63 @@ class CallAnalyzer } } + /** + * @return array + */ + private static function getTemplateTypesForFunction( + FunctionLikeStorage $function_storage, + ClassLikeStorage $class_storage = null, + ClassLikeStorage $calling_class_storage = null + ) : array { + $template_types = []; + + if ($function_storage->template_types) { + $template_types = $function_storage->template_types; + } + + if ($class_storage) { + if ($calling_class_storage + && $class_storage !== $calling_class_storage + && $calling_class_storage->template_type_extends + ) { + foreach ($calling_class_storage->template_type_extends as $class_name_lc => $type_map) { + foreach ($type_map as $template_name => $type) { + if (is_string($template_name) + && $class_name_lc === strtolower($class_storage->name) + ) { + if ($type instanceof Type\Atomic\TGenericParam + && $type->defining_class + && isset( + $calling_class_storage + ->template_type_extends + [strtolower($type->defining_class)] + [$type->param_name] + ) + ) { + $type = $calling_class_storage + ->template_type_extends + [strtolower($type->defining_class)] + [$type->param_name]; + } + + $template_types[$template_name] = [new Type\Union([$type]), $class_storage->name]; + } + } + } + } elseif ($class_storage->template_types) { + foreach ($class_storage->template_types as $template_name => $type) { + $template_types[$template_name] = $type; + } + } + } + + foreach ($template_types as $key => $type) { + $template_types[$key][0] = clone $type[0]; + } + + return $template_types; + } + /** * @param StatementsAnalyzer $statements_analyzer * @param array $args diff --git a/src/Psalm/Internal/Codebase/Populator.php b/src/Psalm/Internal/Codebase/Populator.php index e5dab070b..6067d1ceb 100644 --- a/src/Psalm/Internal/Codebase/Populator.php +++ b/src/Psalm/Internal/Codebase/Populator.php @@ -334,6 +334,7 @@ class Populator $storage->parent_classes = array_merge($storage->parent_classes, $parent_storage->parent_classes); if ($parent_storage->template_types + && $parent_storage_class && isset($storage->template_type_extends[$parent_storage_class]) ) { foreach ($storage->template_type_extends[$parent_storage_class] as $i => $type) { @@ -345,6 +346,13 @@ class Populator $storage->template_type_extends[$parent_storage_class][$mapped_name] = $type; } } + + if ($parent_storage->template_type_extends) { + $storage->template_type_extends = array_merge( + $parent_storage->template_type_extends, + $storage->template_type_extends + ); + } } $this->inheritMethodsFromParent($storage, $parent_storage); diff --git a/src/Psalm/Type.php b/src/Psalm/Type.php index d352bc93d..8b3d23a95 100644 --- a/src/Psalm/Type.php +++ b/src/Psalm/Type.php @@ -423,6 +423,10 @@ abstract class Type } if ($parse_tree instanceof ParseTree\NullableTree) { + if (!isset($parse_tree->children[0])) { + throw new TypeParseTreeException('Misplaced question mark'); + } + $non_nullable_type = self::getTypeFromTree($parse_tree->children[0], false, $template_type_map); if ($non_nullable_type instanceof Union) { diff --git a/tests/TemplateTest.php b/tests/TemplateTest.php index f52833fb5..a0420f682 100644 --- a/tests/TemplateTest.php +++ b/tests/TemplateTest.php @@ -1672,6 +1672,96 @@ class TemplateTest extends TestCase '$id' => 'array-key', ] ], + 'extendsTwiceSameName' => [ + 'v = $v; + } + /** + * @return T + */ + public function getValue() + { + return $this->v; + } + } + + /** + * @template T + * @template-extends Container + */ + class ChildContainer extends Container {} + + /** + * @template T + * @template-extends ChildContainer + */ + class GrandChildContainer extends ChildContainer {} + + $fc = new GrandChildContainer(5); + $a = $fc->getValue();', + [ + '$a' => 'int', + ] + ], + 'extendsTwiceDifferentName' => [ + 'v = $v; + } + /** + * @return T1 + */ + public function getValue() + { + return $this->v; + } + } + + /** + * @template T2 + * @template-extends Container + */ + class ChildContainer extends Container {} + + /** + * @template T3 + * @template-extends ChildContainer + */ + class GrandChildContainer extends ChildContainer {} + + $fc = new GrandChildContainer(5); + $a = $fc->getValue();', + [ + '$a' => 'int', + ] + ], ]; } @@ -2051,6 +2141,132 @@ class TemplateTest extends TestCase $au = new AppUser("string");', 'error_message' => 'InvalidScalarArgument', ], + 'extendsTwiceDifferentNameBrokenChain' => [ + 'v = $v; + } + /** + * @return T1 + */ + public function getValue() + { + return $this->v; + } + } + + /** + * @template T2 + */ + class ChildContainer extends Container {} + + /** + * @template T3 + * @template-extends ChildContainer + */ + class GrandChildContainer extends ChildContainer {} + + $fc = new GrandChildContainer(5); + $a = $fc->getValue();', + 'error_message' => 'MixedAssignment', + ], + 'extendsTwiceSameNameBrokenChain' => [ + 'v = $v; + } + /** + * @return T + */ + public function getValue() + { + return $this->v; + } + } + + /** + * @template T + */ + class ChildContainer extends Container {} + + /** + * @template T + * @template-extends ChildContainer + */ + class GrandChildContainer extends ChildContainer {} + + $fc = new GrandChildContainer(5); + $a = $fc->getValue();', + 'error_message' => 'MixedAssignment', + ], + 'extendsTwiceSameNameLastDoesNotExtend' => [ + 'v = $v; + } + /** + * @return T + */ + public function getValue() + { + return $this->v; + } + } + + /** + * @template T + * @template-extends Container + */ + class ChildContainer extends Container {} + + /** + * @template T + */ + class GrandChildContainer extends ChildContainer {} + + $fc = new GrandChildContainer(5); + $a = $fc->getValue();', + 'error_message' => 'MixedAssignment', + ], ]; } }