From 751253ddf030f402453bea2cad035c0db0ad3242 Mon Sep 17 00:00:00 2001 From: Brown Date: Mon, 6 May 2019 16:38:08 -0400 Subject: [PATCH] Fix #1603 - prevent invalid covariant template classes from being passed --- src/Psalm/DocComment.php | 2 +- .../Internal/Analyzer/CommentAnalyzer.php | 84 ++++- .../Internal/Analyzer/MethodAnalyzer.php | 5 + .../Statements/ExpressionAnalyzer.php | 1 + src/Psalm/Internal/Analyzer/TypeAnalyzer.php | 34 +++ src/Psalm/Internal/Codebase/Populator.php | 3 +- .../LanguageServer/Server/TextDocument.php | 4 +- .../Scanner/ClassLikeDocblockComment.php | 2 +- .../Scanner/FunctionDocblockComment.php | 2 +- .../Internal/Stubs/CoreGenericClasses.php | 28 +- src/Psalm/Internal/Type/TypeCombination.php | 288 ++++++++++-------- .../Internal/Visitor/ReflectorVisitor.php | 28 +- src/Psalm/Storage/ClassLikeStorage.php | 5 + src/Psalm/Storage/FunctionLikeStorage.php | 5 + src/Psalm/Type/Atomic/TGenericObject.php | 19 ++ src/Psalm/Type/Union.php | 68 ++++- tests/Template/TemplateExtendsTest.php | 46 +++ tests/Template/TemplateTest.php | 81 +++++ tests/TypeCombinationTest.php | 25 +- 19 files changed, 566 insertions(+), 164 deletions(-) diff --git a/src/Psalm/DocComment.php b/src/Psalm/DocComment.php index a6f0711f78a..8a5268dc194 100644 --- a/src/Psalm/DocComment.php +++ b/src/Psalm/DocComment.php @@ -128,7 +128,7 @@ public static function parse($docblock, $line_number = null, $preserve_format = $special_key, [ 'return', 'param', 'template', 'var', 'type', - 'property', 'method', + 'template-covariant', 'property', 'method', 'assert', 'assert-if-true', 'assert-if-false', 'suppress', 'ignore-nullable-return', 'override-property-visibility', 'override-method-visibility', 'seal-properties', 'seal-methods', diff --git a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php index b5121c6f407..f2385d09df5 100644 --- a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php @@ -401,15 +401,52 @@ public static function extractFunctionDocblockInfo($comment, $line_number) foreach ($all_templates as $template_line) { $template_type = preg_split('/[\s]+/', $template_line); - if (count($template_type) > 2 - && in_array(strtolower($template_type[1]), ['as', 'super', 'of'], true) + $template_name = array_shift($template_type); + + if (count($template_type) > 1 + && in_array(strtolower($template_type[0]), ['as', 'super', 'of'], true) ) { + $template_modifier = strtolower(array_shift($template_type)); $info->templates[] = [ - $template_type[0], - strtolower($template_type[1]), $template_type[2] + $template_name, + $template_modifier, + implode(' ', $template_type), + false ]; } else { - $info->templates[] = [$template_type[0]]; + $info->templates[] = [$template_name, null, null, false]; + } + } + } + + if (isset($comments['specials']['template-covariant']) + || isset($comments['specials']['psalm-template-covariant']) + ) { + $all_templates = + (isset($comments['specials']['template-covariant']) + ? $comments['specials']['template-covariant'] + : []) + + (isset($comments['specials']['psalm-template-covariant']) + ? $comments['specials']['psalm-template-covariant'] + : []); + + foreach ($all_templates as $template_line) { + $template_type = preg_split('/[\s]+/', $template_line); + + $template_name = array_shift($template_type); + + if (count($template_type) > 1 + && in_array(strtolower($template_type[0]), ['as', 'super', 'of'], true) + ) { + $template_modifier = strtolower(array_shift($template_type)); + $info->templates[] = [ + $template_name, + $template_modifier, + implode(' ', $template_type), + true + ]; + } else { + $info->templates[] = [$template_name, null, null, true]; } } } @@ -549,10 +586,43 @@ public static function extractClassLikeDocblockInfo(\PhpParser\Node $node, $comm $info->templates[] = [ $template_name, $template_modifier, - implode(' ', $template_type) + implode(' ', $template_type), + false + ]; + } else { + $info->templates[] = [$template_name, null, null, false]; + } + } + } + + if (isset($comments['specials']['template-covariant']) + || isset($comments['specials']['psalm-template-covariant']) + ) { + $all_templates = + (isset($comments['specials']['template-covariant']) + ? $comments['specials']['template-covariant'] + : []) + + (isset($comments['specials']['psalm-template-covariant']) + ? $comments['specials']['psalm-template-covariant'] + : []); + + foreach ($all_templates as $template_line) { + $template_type = preg_split('/[\s]+/', $template_line); + + $template_name = array_shift($template_type); + + if (count($template_type) > 1 + && in_array(strtolower($template_type[0]), ['as', 'super', 'of'], true) + ) { + $template_modifier = strtolower(array_shift($template_type)); + $info->templates[] = [ + $template_name, + $template_modifier, + implode(' ', $template_type), + true ]; } else { - $info->templates[] = [$template_name]; + $info->templates[] = [$template_name, null, null, true]; } } } diff --git a/src/Psalm/Internal/Analyzer/MethodAnalyzer.php b/src/Psalm/Internal/Analyzer/MethodAnalyzer.php index 553f7725cc7..47e62954e4e 100644 --- a/src/Psalm/Internal/Analyzer/MethodAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/MethodAnalyzer.php @@ -653,6 +653,11 @@ public static function compareMethods( $template_types, $codebase ); + + $guide_method_storage_return_type->replaceTemplateTypesWithArgTypes( + $template_types, + $codebase + ); } // treat void as null when comparing against docblock implementer diff --git a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php index b0a0aba5b64..ccc8738e452 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php @@ -1010,6 +1010,7 @@ public static function fleshOutType( $fleshed_out_type->possibly_undefined = $return_type->possibly_undefined; $fleshed_out_type->by_ref = $return_type->by_ref; $fleshed_out_type->initialized = $return_type->initialized; + $fleshed_out_type->had_template = $return_type->had_template; return $fleshed_out_type; } diff --git a/src/Psalm/Internal/Analyzer/TypeAnalyzer.php b/src/Psalm/Internal/Analyzer/TypeAnalyzer.php index aef77b20ffd..a93b151c53f 100644 --- a/src/Psalm/Internal/Analyzer/TypeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/TypeAnalyzer.php @@ -1605,6 +1605,40 @@ private static function isMatchingTypeContainedBy( $allow_interface_equality )) { $all_types_contain = false; + } elseif (!$input_type_part instanceof TIterable + && !$container_param->had_template + && !$input_param->had_template + && !$container_param->hasTemplate() + && !$input_param->hasTemplate() + && !$input_param->hasLiteralValue() + ) { + $input_storage = $codebase->classlike_storage_provider->get($input_type_part->value); + + if (!($input_storage->template_covariants[$i] ?? false)) { + // Make sure types are basically the same + if (!self::isContainedBy( + $codebase, + $container_param, + $input_param, + $container_param->ignore_nullable_issues, + $container_param->ignore_falsable_issues, + $has_scalar_match, + $type_coerced, + $type_coerced_from_mixed, + $to_string_cast, + $type_coerced_from_scalar, + $allow_interface_equality + ) || $type_coerced + ) { + if ($container_param->hasMixed() || $container_param->isArrayKey()) { + $type_coerced_from_mixed = true; + } else { + $all_types_contain = false; + } + + $type_coerced = false; + } + } } } } diff --git a/src/Psalm/Internal/Codebase/Populator.php b/src/Psalm/Internal/Codebase/Populator.php index 8c5ae213939..ae5a585cd3f 100644 --- a/src/Psalm/Internal/Codebase/Populator.php +++ b/src/Psalm/Internal/Codebase/Populator.php @@ -905,7 +905,7 @@ private function convertPhpStormGenericToPsalmGeneric(Type\Union $candidate, $is { $atomic_types = $candidate->getTypes(); - if (isset($atomic_types['array']) && count($atomic_types) > 1) { + if (isset($atomic_types['array']) && count($atomic_types) > 1 && !isset($atomic_types['null'])) { $iterator_name = null; $generic_params = null; @@ -947,6 +947,7 @@ private function convertPhpStormGenericToPsalmGeneric(Type\Union $candidate, $is } $candidate->removeType('array'); + $candidate->removeType($iterator_name); $candidate->addType($generic_iterator); } } diff --git a/src/Psalm/Internal/LanguageServer/Server/TextDocument.php b/src/Psalm/Internal/LanguageServer/Server/TextDocument.php index 3c3ab531606..7fd4accd3f3 100644 --- a/src/Psalm/Internal/LanguageServer/Server/TextDocument.php +++ b/src/Psalm/Internal/LanguageServer/Server/TextDocument.php @@ -160,7 +160,7 @@ public function didClose(TextDocumentIdentifier $textDocument) * * @param TextDocumentIdentifier $textDocument The text document * @param Position $position The position inside the text document - * @psalm-return Promise + * @psalm-return Promise|Promise */ public function definition(TextDocumentIdentifier $textDocument, Position $position): Promise { @@ -244,7 +244,7 @@ public function hover(TextDocumentIdentifier $textDocument, Position $position): * * @param TextDocumentIdentifier The text document * @param Position $position The position - * @psalm-return Promise + * @psalm-return Promise>|Promise */ public function completion(TextDocumentIdentifier $textDocument, Position $position): Promise { diff --git a/src/Psalm/Internal/Scanner/ClassLikeDocblockComment.php b/src/Psalm/Internal/Scanner/ClassLikeDocblockComment.php index fc2fefae2ae..6de81ae7bb6 100644 --- a/src/Psalm/Internal/Scanner/ClassLikeDocblockComment.php +++ b/src/Psalm/Internal/Scanner/ClassLikeDocblockComment.php @@ -21,7 +21,7 @@ class ClassLikeDocblockComment public $internal = false; /** - * @var array> + * @var array */ public $templates = []; diff --git a/src/Psalm/Internal/Scanner/FunctionDocblockComment.php b/src/Psalm/Internal/Scanner/FunctionDocblockComment.php index 5049a0341cd..f2f6b787125 100644 --- a/src/Psalm/Internal/Scanner/FunctionDocblockComment.php +++ b/src/Psalm/Internal/Scanner/FunctionDocblockComment.php @@ -80,7 +80,7 @@ class FunctionDocblockComment public $return_type_line_number; /** - * @var array> + * @var array */ public $templates = []; diff --git a/src/Psalm/Internal/Stubs/CoreGenericClasses.php b/src/Psalm/Internal/Stubs/CoreGenericClasses.php index 176a3681326..94efc1ac271 100644 --- a/src/Psalm/Internal/Stubs/CoreGenericClasses.php +++ b/src/Psalm/Internal/Stubs/CoreGenericClasses.php @@ -4,8 +4,8 @@ * Interface to detect if a class is traversable using &foreach;. * @link http://php.net/manual/en/class.traversable.php * - * @template TKey - * @template TValue + * @template-covariant TKey + * @template-covariant TValue */ interface Traversable { } @@ -14,8 +14,8 @@ interface Traversable { * Interface to create an external Iterator. * @link http://php.net/manual/en/class.iteratoraggregate.php * - * @template TKey - * @template TValue + * @template-covariant TKey + * @template-covariant TValue * * @template-extends Traversable */ @@ -36,8 +36,8 @@ public function getIterator(); * themselves internally. * @link http://php.net/manual/en/class.iterator.php * - * @template TKey - * @template TValue + * @template-covariant TKey + * @template-covariant TValue * * @template-extends Traversable */ @@ -86,10 +86,10 @@ public function rewind(); } /** - * @template TKey - * @template TValue - * @template TSend - * @template TReturn + * @template-covariant TKey + * @template-covariant TValue + * @template-covariant TSend + * @template-covariant TReturn * * @template-implements Traversable */ @@ -452,8 +452,8 @@ public function getIteratorClass() { } /** * The Seekable iterator. * @link https://php.net/manual/en/class.seekableiterator.php - * @template TKey - * @template TValue + * @template-covariant TKey + * @template-covariant TValue * @template-extends Iterator */ interface SeekableIterator extends Iterator { @@ -725,7 +725,7 @@ public function getElementsByTagNameNS ($namespaceURI, $localName) {} } /** - * @template TNode as DOMNode + * @template-covariant TNode as DOMNode * @template-implements Traversable */ class DOMNodeList implements Traversable, Countable { @@ -1155,7 +1155,7 @@ public function getHash($object) {} } /** - * @template T as object + * @template-covariant T as object * * @property-read class-string $name */ diff --git a/src/Psalm/Internal/Type/TypeCombination.php b/src/Psalm/Internal/Type/TypeCombination.php index 925a2919a02..e1f44e90c50 100644 --- a/src/Psalm/Internal/Type/TypeCombination.php +++ b/src/Psalm/Internal/Type/TypeCombination.php @@ -45,8 +45,14 @@ class TypeCombination /** @var array|null */ private $named_object_types = []; + /** @var array */ + private $array_type_params = []; + + /** @var array> */ + private $builtin_type_params = []; + /** @var array> */ - private $type_params = []; + private $object_type_params = []; /** @var array|null */ private $array_counts = []; @@ -158,7 +164,9 @@ public static function combineTypes( if (count($combination->value_types) === 1 && !count($combination->objectlike_entries) - && !count($combination->type_params) + && !$combination->array_type_params + && !$combination->builtin_type_params + && !$combination->object_type_params && !$combination->named_object_types && !$combination->strings && !$combination->ints @@ -200,17 +208,18 @@ public static function combineTypes( $combination->value_types['bool'] = new TBool(); } - if (isset($combination->type_params['array']) + if ($combination->array_type_params && (isset($combination->named_object_types['Traversable']) - || isset($combination->type_params['Traversable'])) + || isset($combination->builtin_type_params['Traversable'])) && (($codebase && !$codebase->config->allow_phpstorm_generics) - || isset($combination->type_params['Traversable']) + || isset($combination->builtin_type_params['Traversable']) || (isset($combination->named_object_types['Traversable']) && $combination->named_object_types['Traversable']->from_docblock) ) ) { - $array_param_types = $combination->type_params['array']; - $traversable_param_types = $combination->type_params['Traversable'] ?? [Type::getMixed(), Type::getMixed()]; + $array_param_types = $combination->array_type_params; + $traversable_param_types = $combination->builtin_type_params['Traversable'] + ?? [Type::getMixed(), Type::getMixed()]; $combined_param_types = []; @@ -220,6 +229,8 @@ public static function combineTypes( $combination->value_types['iterable'] = new TIterable($combined_param_types); + $combination->array_type_params = []; + /** * @psalm-suppress PossiblyNullArrayOffset * @psalm-suppress PossiblyNullArrayAccess @@ -227,8 +238,7 @@ public static function combineTypes( unset( $combination->value_types['array'], $combination->named_object_types['Traversable'], - $combination->type_params['array'], - $combination->type_params['Traversable'] + $combination->builtin_type_params['Traversable'] ); } @@ -254,25 +264,25 @@ public static function combineTypes( if (count($combination->objectlike_entries)) { if (!$combination->has_mixed || $combination->mixed_from_loop_isset) { - if (isset($combination->type_params['array']) - && $combination->type_params['array'][0]->allStringLiterals() + if ($combination->array_type_params + && $combination->array_type_params[0]->allStringLiterals() ) { - foreach ($combination->type_params['array'][0]->getTypes() as $atomic_key_type) { + foreach ($combination->array_type_params[0]->getTypes() as $atomic_key_type) { if ($atomic_key_type instanceof TLiteralString) { $combination->objectlike_entries[$atomic_key_type->value] - = $combination->type_params['array'][1]; + = $combination->array_type_params[1]; } } - unset($combination->type_params['array']); + $combination->array_type_params = []; } - if (!isset($combination->type_params['array']) - || $combination->type_params['array'][1]->isEmpty() + if (!$combination->array_type_params + || $combination->array_type_params[1]->isEmpty() ) { if (!$overwrite_empty_array - && (isset($combination->type_params['array']) - && $combination->type_params['array'][1]->isEmpty()) + && ($combination->array_type_params + && $combination->array_type_params[1]->isEmpty()) ) { foreach ($combination->objectlike_entries as $objectlike_entry) { $objectlike_entry->possibly_undefined = true; @@ -291,7 +301,7 @@ function (Type\Union $type) : bool { if ($combination->objectlike_entries) { $objectlike = new ObjectLike($combination->objectlike_entries); - if ($combination->objectlike_sealed && !isset($combination->type_params['array'])) { + if ($combination->objectlike_sealed && !$combination->array_type_params) { $objectlike->sealed = true; } @@ -305,89 +315,95 @@ function (Type\Union $type) : bool { } // if we're merging an empty array with an object-like, clobber empty array - unset($combination->type_params['array']); + $combination->array_type_params = []; } } else { - $combination->type_params['array'] = [Type::getMixed(), Type::getMixed()]; + $combination->array_type_params = [Type::getMixed(), Type::getMixed()]; } } - foreach ($combination->type_params as $generic_type => $generic_type_params) { - if ($generic_type === 'array') { - if ($combination->objectlike_entries) { - $objectlike_generic_type = null; + if ($generic_type_params = $combination->array_type_params) { + if ($combination->objectlike_entries) { + $objectlike_generic_type = null; - $objectlike_keys = []; + $objectlike_keys = []; - foreach ($combination->objectlike_entries as $property_name => $property_type) { - if ($objectlike_generic_type) { - $objectlike_generic_type = Type::combineUnionTypes( - $property_type, - $objectlike_generic_type, - $codebase, - $overwrite_empty_array - ); - } else { - $objectlike_generic_type = clone $property_type; - } + foreach ($combination->objectlike_entries as $property_name => $property_type) { + if ($objectlike_generic_type) { + $objectlike_generic_type = Type::combineUnionTypes( + $property_type, + $objectlike_generic_type, + $codebase, + $overwrite_empty_array + ); + } else { + $objectlike_generic_type = clone $property_type; + } - if (is_int($property_name)) { - if (!isset($objectlike_keys['int'])) { - $objectlike_keys['int'] = new TInt; - } - } else { - if (!isset($objectlike_keys['string'])) { - $objectlike_keys['string'] = new TString; - } + if (is_int($property_name)) { + if (!isset($objectlike_keys['int'])) { + $objectlike_keys['int'] = new TInt; + } + } else { + if (!isset($objectlike_keys['string'])) { + $objectlike_keys['string'] = new TString; } } + } - $objectlike_generic_type->possibly_undefined = false; + $objectlike_generic_type->possibly_undefined = false; - $objectlike_key_type = new Type\Union(array_values($objectlike_keys)); + $objectlike_key_type = new Type\Union(array_values($objectlike_keys)); - $generic_type_params[0] = Type::combineUnionTypes( - $generic_type_params[0], - $objectlike_key_type, - $codebase, - $overwrite_empty_array, - $allow_mixed_union - ); - $generic_type_params[1] = Type::combineUnionTypes( - $generic_type_params[1], - $objectlike_generic_type, - $codebase, - $overwrite_empty_array, - $allow_mixed_union - ); - } + $generic_type_params[0] = Type::combineUnionTypes( + $generic_type_params[0], + $objectlike_key_type, + $codebase, + $overwrite_empty_array, + $allow_mixed_union + ); + $generic_type_params[1] = Type::combineUnionTypes( + $generic_type_params[1], + $objectlike_generic_type, + $codebase, + $overwrite_empty_array, + $allow_mixed_union + ); + } - if ($combination->array_always_filled - || ($combination->array_sometimes_filled && $overwrite_empty_array) - || ($combination->objectlike_entries - && $combination->objectlike_sealed - && $overwrite_empty_array) - ) { - if ($combination->array_counts && count($combination->array_counts) === 1) { - $array_type = new TNonEmptyArray($generic_type_params); - $array_type->count = array_keys($combination->array_counts)[0]; - } else { - $array_type = new TNonEmptyArray($generic_type_params); - } + if ($combination->array_always_filled + || ($combination->array_sometimes_filled && $overwrite_empty_array) + || ($combination->objectlike_entries + && $combination->objectlike_sealed + && $overwrite_empty_array) + ) { + if ($combination->array_counts && count($combination->array_counts) === 1) { + $array_type = new TNonEmptyArray($generic_type_params); + $array_type->count = array_keys($combination->array_counts)[0]; } else { - $array_type = new TArray($generic_type_params); + $array_type = new TNonEmptyArray($generic_type_params); } + } else { + $array_type = new TArray($generic_type_params); + } - $new_types[] = $array_type; - } elseif (!isset($combination->value_types[$generic_type])) { - if ($generic_type === 'iterable') { - $new_types[] = new TIterable($generic_type_params); - } else { - $new_types[] = new TGenericObject($generic_type, $generic_type_params); - } + $new_types[] = $array_type; + } + + foreach ($combination->builtin_type_params as $generic_type => $generic_type_params) { + if ($generic_type === 'iterable') { + $new_types[] = new TIterable($generic_type_params); + } else { + $new_types[] = new TGenericObject($generic_type, $generic_type_params); } } + foreach ($combination->object_type_params as $generic_type => $generic_type_params) { + $generic_type = substr($generic_type, 0, (int) strpos($generic_type, '<')); + + $new_types[] = new TGenericObject($generic_type, $generic_type_params); + } + if ($combination->strings) { $new_types = array_merge($new_types, array_values($combination->strings)); } @@ -498,73 +514,75 @@ private static function scrapeTypeProperties( unset($combination->value_types['true']); } - if ($type instanceof TArray && isset($combination->type_params['iterable'])) { + if ($type instanceof TArray && isset($combination->builtin_type_params['iterable'])) { $type_key = 'iterable'; } elseif ($type instanceof TArray && $type->type_params[1]->isMixed() && isset($combination->value_types['iterable']) ) { $type_key = 'iterable'; - $combination->type_params['iterable'] = [Type::getMixed(), Type::getMixed()]; + $combination->builtin_type_params['iterable'] = [Type::getMixed(), Type::getMixed()]; } elseif ($type instanceof TNamedObject && $type->value === 'Traversable' - && (isset($combination->type_params['iterable']) || isset($combination->value_types['iterable'])) + && (isset($combination->builtin_type_params['iterable']) || isset($combination->value_types['iterable'])) ) { $type_key = 'iterable'; - if (!isset($combination->type_params['iterable'])) { - $combination->type_params['iterable'] = [Type::getMixed(), Type::getMixed()]; + if (!isset($combination->builtin_type_params['iterable'])) { + $combination->builtin_type_params['iterable'] = [Type::getMixed(), Type::getMixed()]; } if (!$type instanceof TGenericObject) { $type = new TGenericObject($type->value, [Type::getMixed(), Type::getMixed()]); } + } elseif ($type instanceof TNamedObject && ($type->value === 'Traversable' || $type->value === 'Generator')) { + $type_key = $type->value; } else { $type_key = $type->getKey(); } if ($type instanceof TIterable - && isset($combination->type_params['array']) - && ($type->has_docblock_params || $combination->type_params['array'][1]->isMixed()) + && $combination->array_type_params + && ($type->has_docblock_params || $combination->array_type_params[1]->isMixed()) ) { - if (!isset($combination->type_params['iterable'])) { - $combination->type_params['iterable'] = $combination->type_params['array']; + if (!isset($combination->builtin_type_params['iterable'])) { + $combination->builtin_type_params['iterable'] = $combination->array_type_params; } else { - foreach ($combination->type_params['array'] as $i => $array_type_param) { - $iterable_type_param = $combination->type_params['iterable'][$i]; - $combination->type_params['iterable'][$i] = Type::combineUnionTypes( + foreach ($combination->array_type_params as $i => $array_type_param) { + $iterable_type_param = $combination->builtin_type_params['iterable'][$i]; + $combination->builtin_type_params['iterable'][$i] = Type::combineUnionTypes( $iterable_type_param, $array_type_param ); } } - unset($combination->type_params['array']); + $combination->array_type_params = []; } if ($type instanceof TIterable && (isset($combination->named_object_types['Traversable']) - || isset($combination->type_params['Traversable'])) + || isset($combination->builtin_type_params['Traversable'])) ) { - if (!isset($combination->type_params['iterable'])) { - $combination->type_params['iterable'] - = $combination->type_params['Traversable'] ?? [Type::getMixed(), Type::getMixed()]; - } elseif (isset($combination->type_params['Traversable'])) { - foreach ($combination->type_params['Traversable'] as $i => $array_type_param) { - $iterable_type_param = $combination->type_params['iterable'][$i]; - $combination->type_params['iterable'][$i] = Type::combineUnionTypes( + if (!isset($combination->builtin_type_params['iterable'])) { + $combination->builtin_type_params['iterable'] + = $combination->builtin_type_params['Traversable'] ?? [Type::getMixed(), Type::getMixed()]; + } elseif (isset($combination->builtin_type_params['Traversable'])) { + foreach ($combination->builtin_type_params['Traversable'] as $i => $array_type_param) { + $iterable_type_param = $combination->builtin_type_params['iterable'][$i]; + $combination->builtin_type_params['iterable'][$i] = Type::combineUnionTypes( $iterable_type_param, $array_type_param ); } } else { - $combination->type_params['iterable'] = [Type::getMixed(), Type::getMixed()]; + $combination->builtin_type_params['iterable'] = [Type::getMixed(), Type::getMixed()]; } /** @psalm-suppress PossiblyNullArrayAccess */ unset( $combination->named_object_types['Traversable'], - $combination->type_params['Traversable'] + $combination->builtin_type_params['Traversable'] ); } @@ -615,36 +633,60 @@ private static function scrapeTypeProperties( } } - if ($type instanceof TArray - || $type instanceof TGenericObject - || ($type instanceof TIterable && $type->has_docblock_params) - ) { + if ($type instanceof TArray && $type_key === 'array') { foreach ($type->type_params as $i => $type_param) { - if (isset($combination->type_params[$type_key][$i])) { - $combination->type_params[$type_key][$i] = Type::combineUnionTypes( - $combination->type_params[$type_key][$i], + if (isset($combination->array_type_params[$i])) { + $combination->array_type_params[$i] = Type::combineUnionTypes( + $combination->array_type_params[$i], $type_param, $codebase, $overwrite_empty_array ); } else { - $combination->type_params[$type_key][$i] = $type_param; + $combination->array_type_params[$i] = $type_param; } } - if ($type instanceof TArray) { - if ($type instanceof TNonEmptyArray) { - if ($combination->array_counts !== null) { - if ($type->count === null) { - $combination->array_counts = null; - } else { - $combination->array_counts[$type->count] = true; - } + if ($type instanceof TNonEmptyArray) { + if ($combination->array_counts !== null) { + if ($type->count === null) { + $combination->array_counts = null; + } else { + $combination->array_counts[$type->count] = true; } + } - $combination->array_sometimes_filled = true; + $combination->array_sometimes_filled = true; + } else { + $combination->array_always_filled = false; + } + } elseif (($type instanceof TGenericObject && ($type->value === 'Traversable' || $type->value === 'Generator')) + || ($type instanceof TIterable && $type->has_docblock_params) + || ($type instanceof TArray && $type_key === 'iterable') + ) { + foreach ($type->type_params as $i => $type_param) { + if (isset($combination->builtin_type_params[$type_key][$i])) { + $combination->builtin_type_params[$type_key][$i] = Type::combineUnionTypes( + $combination->builtin_type_params[$type_key][$i], + $type_param, + $codebase, + $overwrite_empty_array + ); + } else { + $combination->builtin_type_params[$type_key][$i] = $type_param; + } + } + } elseif ($type instanceof TGenericObject) { + foreach ($type->type_params as $i => $type_param) { + if (isset($combination->object_type_params[$type_key][$i])) { + $combination->object_type_params[$type_key][$i] = Type::combineUnionTypes( + $combination->object_type_params[$type_key][$i], + $type_param, + $codebase, + $overwrite_empty_array + ); } else { - $combination->array_always_filled = false; + $combination->object_type_params[$type_key][$i] = $type_param; } } } elseif ($type instanceof ObjectLike) { diff --git a/src/Psalm/Internal/Visitor/ReflectorVisitor.php b/src/Psalm/Internal/Visitor/ReflectorVisitor.php index 60a3e5a23f8..85cc7a51716 100644 --- a/src/Psalm/Internal/Visitor/ReflectorVisitor.php +++ b/src/Psalm/Internal/Visitor/ReflectorVisitor.php @@ -878,14 +878,15 @@ private function registerClassLike(PhpParser\Node\Stmt\ClassLike $node) if ($docblock_info->templates) { $storage->template_types = []; - foreach ($docblock_info->templates as $template_type) { - $template_name = $template_type[0]; - if (count($template_type) === 3) { - if (trim($template_type[2])) { + foreach ($docblock_info->templates as $i => $template_map) { + $template_name = $template_map[0]; + + if ($template_map[1] !== null && $template_map[2] !== null) { + if (trim($template_map[2])) { try { $template_type = Type::parseTokens( Type::fixUpLocalType( - $template_type[2], + $template_map[2], $this->aliases, null, $this->type_aliases @@ -919,6 +920,7 @@ private function registerClassLike(PhpParser\Node\Stmt\ClassLike $node) } } else { $storage->template_types[$template_name][$fq_classlike_name] = [Type::getMixed()]; + $storage->template_covariants[$i] = $template_map[3]; } } @@ -1755,8 +1757,10 @@ private function registerFunctionLike(PhpParser\Node\FunctionLike $stmt, $fake_m if ($docblock_info->templates) { $storage->template_types = []; - foreach ($docblock_info->templates as $template_map) { - if (count($template_map) === 3) { + foreach ($docblock_info->templates as $i => $template_map) { + $template_name = $template_map[0]; + + if ($template_map[1] !== null && $template_map[2] !== null) { if (trim($template_map[2])) { $template_type = Type::parseTokens( Type::fixUpLocalType( @@ -1769,7 +1773,7 @@ private function registerFunctionLike(PhpParser\Node\FunctionLike $stmt, $fake_m } else { if (IssueBuffer::accepts( new InvalidDocblock( - 'Template missing as type', + 'Template ' . $template_name . ' missing as type', new CodeLocation($this->file_scanner, $stmt, null, true) ) )) { @@ -1781,10 +1785,10 @@ private function registerFunctionLike(PhpParser\Node\FunctionLike $stmt, $fake_m $template_type = Type::getMixed(); } - if (isset($template_types[$template_map[0]])) { + if (isset($template_types[$template_name])) { if (IssueBuffer::accepts( new InvalidDocblock( - 'Duplicate template param in docblock for ' + 'Duplicate template param ' . $template_name . ' in docblock for ' . $cased_function_id, new CodeLocation($this->file_scanner, $stmt, null, true) ) @@ -1793,9 +1797,11 @@ private function registerFunctionLike(PhpParser\Node\FunctionLike $stmt, $fake_m $storage->has_docblock_issues = true; } else { - $storage->template_types[$template_map[0]] = [ + $storage->template_types[$template_name] = [ '' => [$template_type] ]; + + $storage->template_covariants[$i] = $template_map[3]; } } diff --git a/src/Psalm/Storage/ClassLikeStorage.php b/src/Psalm/Storage/ClassLikeStorage.php index fbb8f6776c0..dc93f3b3eb6 100644 --- a/src/Psalm/Storage/ClassLikeStorage.php +++ b/src/Psalm/Storage/ClassLikeStorage.php @@ -277,6 +277,11 @@ class ClassLikeStorage */ public $template_types; + /** + * @var array|null + */ + public $template_covariants; + /** * @var array>|null */ diff --git a/src/Psalm/Storage/FunctionLikeStorage.php b/src/Psalm/Storage/FunctionLikeStorage.php index dea496166df..ff783e7b177 100644 --- a/src/Psalm/Storage/FunctionLikeStorage.php +++ b/src/Psalm/Storage/FunctionLikeStorage.php @@ -109,6 +109,11 @@ class FunctionLikeStorage */ public $template_types; + /** + * @var array|null + */ + public $template_covariants; + /** * @var bool */ diff --git a/src/Psalm/Type/Atomic/TGenericObject.php b/src/Psalm/Type/Atomic/TGenericObject.php index 4b1dc0d0fad..d74c1eb4fa2 100644 --- a/src/Psalm/Type/Atomic/TGenericObject.php +++ b/src/Psalm/Type/Atomic/TGenericObject.php @@ -25,6 +25,25 @@ public function __construct($value, array $type_params) $this->type_params = $type_params; } + /** + * @return string + */ + public function getKey() + { + $s = ''; + foreach ($this->type_params as $type_param) { + $s .= $type_param->getKey() . ', '; + } + + $extra_types = ''; + + if ($this instanceof TNamedObject && $this->extra_types) { + $extra_types = '&' . implode('&', $this->extra_types); + } + + return $this->value . '<' . substr($s, 0, -2) . '>' . $extra_types; + } + /** * @return bool */ diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php index e5682e0df3e..6bf31a6aa43 100644 --- a/src/Psalm/Type/Union.php +++ b/src/Psalm/Type/Union.php @@ -92,6 +92,12 @@ class Union */ public $possibly_undefined_from_try = false; + /** + * Whether or not this union had a template, since replaced + * @var bool + */ + public $had_template = false; + /** * @var array */ @@ -264,6 +270,47 @@ public function __toString() return substr($s, 0, -1) ?: ''; } + public function getKey() : string + { + if (empty($this->types)) { + return ''; + } + $s = ''; + + $printed_int = false; + $printed_float = false; + $printed_string = false; + + foreach ($this->types as $type) { + if ($type instanceof TLiteralFloat) { + if ($printed_float) { + continue; + } + + $s .= 'float|'; + $printed_float = true; + } elseif ($type instanceof TLiteralString) { + if ($printed_string) { + continue; + } + + $s .= 'string|'; + $printed_string = true; + } elseif ($type instanceof TLiteralInt) { + if ($printed_int) { + continue; + } + + $s .= 'int|'; + $printed_int = true; + } else { + $s .= $type->getKey() . '|'; + } + } + + return substr($s, 0, -1) ?: ''; + } + /** * @return string */ @@ -785,7 +832,9 @@ public function isNever() */ public function isGenerator() { - return count($this->types) === 1 && isset($this->types['Generator']); + return count($this->types) === 1 + && (($single_type = reset($this->types)) instanceof TNamedObject) + && ($single_type->value === 'Generator'); } /** @@ -898,6 +947,10 @@ public function replaceTemplateTypesWithStandins( $keys_to_unset = []; foreach ($this->types as $key => $atomic_type) { + if ($bracket_pos = strpos($key, '<')) { + $key = substr($key, 0, $bracket_pos); + } + if ($atomic_type instanceof Type\Atomic\TTemplateParam && isset($template_types[$key][$atomic_type->defining_class ?: '']) ) { @@ -925,6 +978,8 @@ public function replaceTemplateTypesWithStandins( } } + $this->had_template = true; + if ($input_type) { $generic_param = clone $input_type; $generic_param->setFromDocblock(); @@ -971,6 +1026,8 @@ public function replaceTemplateTypesWithStandins( $this->types[$class_string->getKey()] = $class_string; + $this->had_template = true; + if ($input_type) { $valid_input_atomic_types = []; @@ -1017,6 +1074,10 @@ public function replaceTemplateTypesWithStandins( if ($input_type && $codebase) { foreach ($input_type->types as $input_key => $atomic_input_type) { + if ($bracket_pos = strpos($input_key, '<')) { + $input_key = substr($input_key, 0, $bracket_pos); + } + if ($input_key === $key) { $matching_atomic_type = $atomic_input_type; break; @@ -1352,6 +1413,11 @@ public function allStringLiterals() : bool return true; } + public function hasLiteralValue() : bool + { + return $this->literal_int_types || $this->literal_string_types || $this->literal_float_types; + } + /** * @return bool true if this is a int literal with only one possible value */ diff --git a/tests/Template/TemplateExtendsTest.php b/tests/Template/TemplateExtendsTest.php index fa85d71f22e..e08146fb953 100644 --- a/tests/Template/TemplateExtendsTest.php +++ b/tests/Template/TemplateExtendsTest.php @@ -1547,6 +1547,7 @@ protected function createFrom(array $elements) ' */ interface Collection extends \IteratorAggregate { @@ -1767,6 +1768,51 @@ protected function validate($value): void } }', ], + 'allowPassingToCovariantCollection' => [ + ' + */ + class Collection extends \ArrayObject { + /** + * @param array $kv + */ + public function __construct(array $kv) { + parent::__construct($kv); + } + } + + /** + * @param Collection $list + */ + function getSounds(Traversable $list) : void { + foreach ($list as $l) { + $l->getSound(); + } + } + + /** + * @param Collection $list + */ + function takesDogList(Collection $list) : void { + getSounds($list); // this probably should not be an error + }' + ], ]; } diff --git a/tests/Template/TemplateTest.php b/tests/Template/TemplateTest.php index 5f4cff47936..0dc65939138 100644 --- a/tests/Template/TemplateTest.php +++ b/tests/Template/TemplateTest.php @@ -2098,6 +2098,56 @@ public function __construct(array $elements = []) '$c' => 'ArrayCollection', ], ], + 'doNotCombineTypes' => [ + 't = $t; + } + + /** + * @return T + */ + public function get() { + return $this->t; + } + } + + /** + * @param C $a + * @param C $b + * @return C|C + */ + function randomCollection(C $a, C $b) : C { + if (rand(0, 1)) { + return $a; + } + + return $b; + } + + $random_collection = randomCollection(new C(new A), new C(new B)); + + $a_or_b = $random_collection->get();', + [ + '$random_collection' => 'C|C', + '$a_or_b' => 'B|A', + ], + ], ]; } @@ -2654,6 +2704,37 @@ public function __construct() }', 'error_message' => 'InvalidPropertyAssignmentValue' ], + 'preventDogCatSnafu' => [ + ' $list + */ + function addAnimal(Collection $list) : void { + $list->add(new Cat()); + } + + /** + * @param Collection $list + */ + function takesDogList(Collection $list) : void { + addAnimal($list); // this should be an error + }', + 'error_message' => 'InvalidArgument', + ], ]; } } diff --git a/tests/TypeCombinationTest.php b/tests/TypeCombinationTest.php index eb92af52d41..5ee17917ff0 100644 --- a/tests/TypeCombinationTest.php +++ b/tests/TypeCombinationTest.php @@ -276,7 +276,7 @@ public function providerTestValidTypeCombination() ], ], 'arrayObjectAndParamsWithEmptyArray' => [ - 'ArrayObject|array', + 'array|ArrayObject', [ 'ArrayObject', 'array', @@ -316,7 +316,7 @@ public function providerTestValidTypeCombination() ], ], 'aAndAOfB' => [ - 'A', + 'A|A', [ 'A', 'A', @@ -379,6 +379,27 @@ public function providerTestValidTypeCombination() 'string', ], ], + 'traversableAorB' => [ + 'Traversable', + [ + 'Traversable', + 'Traversable', + ], + ], + 'iterableAorB' => [ + 'iterable', + [ + 'iterable', + 'iterable', + ], + ], + 'FooAorB' => [ + 'Foo|Foo', + [ + 'Foo', + 'Foo', + ], + ], ]; }