From 081ba4b204863921e86ad7a4f0c408d66143b3e9 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Sun, 27 Jan 2019 23:12:40 -0500 Subject: [PATCH] Fix #1072 - add support for @use SomeTrait --- src/Psalm/Internal/Analyzer/ClassAnalyzer.php | 123 +++++++++++------- .../Internal/Analyzer/MethodAnalyzer.php | 28 ++++ src/Psalm/Internal/Codebase/Populator.php | 47 +++++++ src/Psalm/Internal/Codebase/Reflection.php | 1 + .../Internal/Visitor/ReflectorVisitor.php | 117 ++++++++++++++++- src/Psalm/Storage/ClassLikeStorage.php | 12 +- src/Psalm/Storage/MethodStorage.php | 5 + tests/Template/TemplateExtendsTest.php | 52 ++++++++ 8 files changed, 333 insertions(+), 52 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php index 8ef673766..5cf8232ef 100644 --- a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php @@ -211,7 +211,15 @@ class ClassAnalyzer extends ClassLikeAnalyzer true ); - $this->checkTemplateParams($codebase, $storage, $parent_class_storage, $code_location); + if ($storage->template_type_extends_count !== null) { + $this->checkTemplateParams( + $codebase, + $storage, + $parent_class_storage, + $code_location, + $storage->template_type_extends_count + ); + } } catch (\InvalidArgumentException $e) { // do nothing } @@ -259,7 +267,17 @@ class ClassAnalyzer extends ClassLikeAnalyzer true ); - $this->checkTemplateParams($codebase, $storage, $interface_storage, $code_location); + if (isset($storage->template_type_implements_count[strtolower($fq_interface_name)])) { + $expected_param_count = $storage->template_type_implements_count[strtolower($fq_interface_name)]; + + $this->checkTemplateParams( + $codebase, + $storage, + $interface_storage, + $code_location, + $expected_param_count + ); + } } if ($storage->template_type_extends) { @@ -607,6 +625,22 @@ class ClassAnalyzer extends ClassLikeAnalyzer $trait_aliases ); + if (isset($storage->template_type_uses_count[strtolower($fq_trait_name)])) { + $trait_storage = $codebase->classlike_storage_provider->get($fq_trait_name); + $expected_param_count = $storage->template_type_uses_count[strtolower($fq_trait_name)]; + + $this->checkTemplateParams( + $codebase, + $storage, + $trait_storage, + new CodeLocation( + $this, + $trait + ), + $expected_param_count + ); + } + foreach ($trait_node->stmts as $trait_stmt) { if ($trait_stmt instanceof PhpParser\Node\Stmt\Property) { $this->checkForMissingPropertyType($trait_analyzer, $trait_stmt); @@ -1327,57 +1361,56 @@ class ClassAnalyzer extends ClassLikeAnalyzer Codebase $codebase, ClassLikeStorage $storage, ClassLikeStorage $parent_storage, - CodeLocation $code_location + CodeLocation $code_location, + int $expected_param_count ) { $template_type_count = $parent_storage->template_types === null ? 0 : count($parent_storage->template_types); - if ($storage->template_type_extends_count !== null) { - if ($template_type_count > $storage->template_type_extends_count) { - if (IssueBuffer::accepts( - new MissingTemplateParam( - $storage->name . ' has missing template params, expecting ' - . $template_type_count, - $code_location - ), - array_merge($storage->suppressed_issues, $this->getSuppressedIssues()) - )) { - // fall through - } - } elseif ($template_type_count < $storage->template_type_extends_count) { - if (IssueBuffer::accepts( - new TooManyTemplateParams( - $storage->name . ' has too many template params, expecting ' - . $template_type_count, - $code_location - ), - array_merge($storage->suppressed_issues, $this->getSuppressedIssues()) - )) { - // fall through - } + if ($template_type_count > $expected_param_count) { + if (IssueBuffer::accepts( + new MissingTemplateParam( + $storage->name . ' has missing template params, expecting ' + . $template_type_count, + $code_location + ), + array_merge($storage->suppressed_issues, $this->getSuppressedIssues()) + )) { + // fall through } + } elseif ($template_type_count < $expected_param_count) { + if (IssueBuffer::accepts( + new TooManyTemplateParams( + $storage->name . ' has too many template params, expecting ' + . $template_type_count, + $code_location + ), + array_merge($storage->suppressed_issues, $this->getSuppressedIssues()) + )) { + // fall through + } + } - if ($parent_storage->template_types && $storage->template_type_extends) { - foreach ($parent_storage->template_types as $i => $template_type) { - if (!$template_type[0]->isMixed() - && isset($storage->template_type_extends[strtolower($parent_storage->name)][$i]) - ) { - $extended_type = new Type\Union([ - $storage->template_type_extends[strtolower($parent_storage->name)][$i] - ]); + if ($parent_storage->template_types && $storage->template_type_extends) { + foreach ($parent_storage->template_types as $i => $template_type) { + if (!$template_type[0]->isMixed() + && isset($storage->template_type_extends[strtolower($parent_storage->name)][$i]) + ) { + $extended_type = new Type\Union([ + $storage->template_type_extends[strtolower($parent_storage->name)][$i] + ]); - if (!TypeAnalyzer::isContainedBy($codebase, $extended_type, $template_type[0])) { - if (IssueBuffer::accepts( - new InvalidTemplateParam( - 'Extended template param ' . $i . ' expects type ' . $template_type[0]->getId() - . ', type ' . $extended_type->getId() . ' given', - $code_location - ), - array_merge($storage->suppressed_issues, $this->getSuppressedIssues()) - )) { - // fall through - } + if (!TypeAnalyzer::isContainedBy($codebase, $extended_type, $template_type[0])) { + if (IssueBuffer::accepts( + new InvalidTemplateParam( + 'Extended template param ' . $i . ' expects type ' . $template_type[0]->getId() + . ', type ' . $extended_type->getId() . ' given', + $code_location + ), + array_merge($storage->suppressed_issues, $this->getSuppressedIssues()) + )) { + // fall through } } } diff --git a/src/Psalm/Internal/Analyzer/MethodAnalyzer.php b/src/Psalm/Internal/Analyzer/MethodAnalyzer.php index 3719e96c5..5bde3c19a 100644 --- a/src/Psalm/Internal/Analyzer/MethodAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/MethodAnalyzer.php @@ -582,6 +582,34 @@ class MethodAnalyzer extends FunctionLikeAnalyzer ); } + $guide_trait_name_lc = null; + + if ($guide_classlike_storage === $implementer_classlike_storage) { + $guide_trait_name_lc = strtolower($implementer_method_storage->defining_fqcln); + } + + if ($guide_trait_name_lc + && isset($implementer_classlike_storage->template_type_extends[$guide_trait_name_lc]) + ) { + $map = $implementer_classlike_storage->template_type_extends[$guide_trait_name_lc]; + + $template_types = []; + + foreach ($map as $key => $atomic_type) { + if (is_string($key)) { + $template_types[$key] = [ + new Type\Union([$atomic_type]), + $implementer_method_storage->defining_fqcln + ]; + } + } + + $implementer_method_storage_return_type->replaceTemplateTypesWithArgTypes( + $template_types, + $codebase + ); + } + // treat void as null when comparing against docblock implementer if ($implementer_method_storage_return_type->isVoid()) { $implementer_method_storage_return_type = Type::getNull(); diff --git a/src/Psalm/Internal/Codebase/Populator.php b/src/Psalm/Internal/Codebase/Populator.php index 9cdfb9462..2f5828ffd 100644 --- a/src/Psalm/Internal/Codebase/Populator.php +++ b/src/Psalm/Internal/Codebase/Populator.php @@ -301,6 +301,53 @@ class Populator $this->inheritMethodsFromParent($storage, $trait_storage); $this->inheritPropertiesFromParent($storage, $trait_storage); + + if ($trait_storage->template_types) { + if (isset($storage->template_type_extends[$used_trait_lc])) { + foreach ($storage->template_type_extends[$used_trait_lc] as $i => $type) { + $trait_template_type_names = array_keys($trait_storage->template_types); + + $mapped_name = $trait_template_type_names[$i] ?? null; + + if ($mapped_name) { + $storage->template_type_extends[$used_trait_lc][$mapped_name] = $type; + } + } + + if ($trait_storage->template_type_extends) { + foreach ($trait_storage->template_type_extends as $t_storage_class => $type_map) { + foreach ($type_map as $i => $type) { + if (isset($storage->template_type_extends[$t_storage_class][$i]) + || is_int($i) + ) { + continue; + } + + if ($type instanceof Type\Atomic\TGenericParam + && $type->defining_class + && ($referenced_type + = $storage->template_type_extends + [strtolower($type->defining_class)] + [$type->param_name] + ?? null) + && (!$referenced_type instanceof Type\Atomic\TGenericParam) + ) { + $storage->template_type_extends[$t_storage_class][$i] = $referenced_type; + } else { + $storage->template_type_extends[$t_storage_class][$i] = $type; + } + } + } + } + } else { + $storage->template_type_extends[$used_trait_lc] = []; + + foreach ($trait_storage->template_types as $template_name => $template_type) { + $storage->template_type_extends[$used_trait_lc][$template_name] + = array_values($template_type[0]->getTypes())[0]; + } + } + } } } diff --git a/src/Psalm/Internal/Codebase/Reflection.php b/src/Psalm/Internal/Codebase/Reflection.php index 05027b00d..71a2f5c42 100644 --- a/src/Psalm/Internal/Codebase/Reflection.php +++ b/src/Psalm/Internal/Codebase/Reflection.php @@ -226,6 +226,7 @@ class Reflection $storage = $class_storage->methods[strtolower($method_name)] = new MethodStorage(); $storage->cased_name = $method->name; + $storage->defining_fqcln = $method->class; if (strtolower((string)$method->name) === strtolower((string)$method->class)) { $this->codebase->methods->setDeclaringMethodId( diff --git a/src/Psalm/Internal/Visitor/ReflectorVisitor.php b/src/Psalm/Internal/Visitor/ReflectorVisitor.php index 66b8cca04..307eab308 100644 --- a/src/Psalm/Internal/Visitor/ReflectorVisitor.php +++ b/src/Psalm/Internal/Visitor/ReflectorVisitor.php @@ -443,6 +443,23 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse $storage->used_traits[strtolower($trait_fqcln)] = $trait_fqcln; $this->file_storage->required_classes[strtolower($trait_fqcln)] = $trait_fqcln; } + + if ($node_comment = $node->getDocComment()) { + $comments = DocComment::parse((string) $node_comment, 0); + + if (isset($comments['specials']['template-use']) + || isset($comments['specials']['use']) + ) { + $all_inheritance = array_merge( + $comments['specials']['template-use'] ?? [], + $comments['specials']['use'] ?? [] + ); + + foreach ($all_inheritance as $template_line) { + $this->useTemplatedType($storage, $node, $template_line); + } + } + } } elseif ($node instanceof PhpParser\Node\Expr\Include_) { $this->visitInclude($node); } elseif ($node instanceof PhpParser\Node\Expr\Assign @@ -1097,7 +1114,7 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse $implemented_type_parameters = []; - $storage->template_type_implements_count = count($atomic_type->type_params); + $storage->template_type_implements_count[$generic_class_lc] = count($atomic_type->type_params); foreach ($atomic_type->type_params as $type_param) { if (!$type_param->isSingle()) { @@ -1122,6 +1139,101 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse } } + /** + * @return void + */ + private function useTemplatedType( + ClassLikeStorage $storage, + PhpParser\Node\Stmt\TraitUse $node, + string $used_class_name + ) { + try { + $used_union_type = Type::parseTokens( + Type::fixUpLocalType( + $used_class_name, + $this->aliases, + $this->class_template_types, + $this->type_aliases + ), + false, + $this->class_template_types + ); + } catch (TypeParseTreeException $e) { + if (IssueBuffer::accepts( + new InvalidDocblock( + $e->getMessage() . ' in docblock for ' . implode('.', $this->fq_classlike_names), + new CodeLocation($this->file_scanner, $node, null, true) + ) + )) { + } + + $storage->has_docblock_issues = true; + return; + } + + if (!$used_union_type->isSingle()) { + if (IssueBuffer::accepts( + new InvalidDocblock( + '@template-use cannot be a union type', + new CodeLocation($this->file_scanner, $node, null, true) + ) + )) { + } + } + + foreach ($used_union_type->getTypes() as $atomic_type) { + if (!$atomic_type instanceof Type\Atomic\TGenericObject) { + if (IssueBuffer::accepts( + new InvalidDocblock( + '@template-use has invalid class ' . $atomic_type->getId(), + new CodeLocation($this->file_scanner, $node, null, true) + ) + )) { + } + + return; + } + + $generic_class_lc = strtolower($atomic_type->value); + + if (!isset($storage->used_traits[$generic_class_lc])) { + if (IssueBuffer::accepts( + new InvalidDocblock( + '@template-use must include the name of an used class,' + . ' got ' . $atomic_type->getId(), + new CodeLocation($this->file_scanner, $node, null, true) + ) + )) { + } + } + + $used_type_parameters = []; + + $storage->template_type_uses_count[$generic_class_lc] = count($atomic_type->type_params); + + foreach ($atomic_type->type_params as $type_param) { + if (!$type_param->isSingle()) { + if (IssueBuffer::accepts( + new InvalidDocblock( + '@template-uses type parameter cannot be a union type', + new CodeLocation($this->file_scanner, $node, null, true) + ) + )) { + } + return; + } + + foreach ($type_param->getTypes() as $type_param_atomic) { + $used_type_parameters[] = $type_param_atomic; + } + } + + if ($used_type_parameters) { + $storage->template_type_extends[$generic_class_lc] = $used_type_parameters; + } + } + } + /** * @param PhpParser\Node\FunctionLike $stmt * @param bool $fake_method in the case of @method annotations we do something a little strange @@ -1137,6 +1249,7 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse $cased_function_id = '@method ' . $stmt->name->name; $storage = new MethodStorage(); + $storage->defining_fqcln = ''; $storage->is_static = (bool) $stmt->isStatic(); } elseif ($stmt instanceof PhpParser\Node\Stmt\Function_) { $cased_function_id = @@ -1251,6 +1364,8 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse $storage = $class_storage->methods[strtolower($stmt->name->name)] = new MethodStorage(); } + $storage->defining_fqcln = $fq_classlike_name; + $class_name_parts = explode('\\', $fq_classlike_name); $class_name = array_pop($class_name_parts); diff --git a/src/Psalm/Storage/ClassLikeStorage.php b/src/Psalm/Storage/ClassLikeStorage.php index f82bc7840..45b8bb0af 100644 --- a/src/Psalm/Storage/ClassLikeStorage.php +++ b/src/Psalm/Storage/ClassLikeStorage.php @@ -281,15 +281,15 @@ class ClassLikeStorage public $template_type_extends_count; /** - * @var array>|null - */ - public $template_type_implements; - - /** - * @var ?int + * @var array|null */ public $template_type_implements_count; + /** + * @var array|null + */ + public $template_type_uses_count; + /** * @var array>|null */ diff --git a/src/Psalm/Storage/MethodStorage.php b/src/Psalm/Storage/MethodStorage.php index cd2ab743b..7f7641d18 100644 --- a/src/Psalm/Storage/MethodStorage.php +++ b/src/Psalm/Storage/MethodStorage.php @@ -49,4 +49,9 @@ class MethodStorage extends FunctionLikeStorage * @var bool */ public $inheritdoc = false; + + /** + * @var string + */ + public $defining_fqcln; } diff --git a/tests/Template/TemplateExtendsTest.php b/tests/Template/TemplateExtendsTest.php index 94c15f08c..1c518d4b3 100644 --- a/tests/Template/TemplateExtendsTest.php +++ b/tests/Template/TemplateExtendsTest.php @@ -844,6 +844,43 @@ class TemplateExtendsTest extends TestCase public function valid(): bool { return false; } }', ], + 'traitUse' => [ + ' + */ + abstract function elements() : array; + + /** + * @return T|null + */ + public function first() + { + return $this->elements()[0] ?? null; + } + } + + class Service + { + /** + * @use CollectionTrait + */ + use CollectionTrait; + + /** + * @return array + */ + public function elements(): array + { + return [1, 2, 3, 4]; + } + }', + ], ]; } @@ -1126,6 +1163,21 @@ class TemplateExtendsTest extends TestCase class CC extends A {}', 'error_message' => 'MissingTemplateParam' ], + 'templateImplementsWithoutAllParams' => [ + ' + */ + class CC implements I {}', + 'error_message' => 'MissingTemplateParam' + ], 'extendsTemplateButLikeBadly' => [ '