From 14bab7bd59fbaee0e767681bc1d587cf62c73ee4 Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Fri, 19 Jan 2024 12:51:55 +0100 Subject: [PATCH] Allow specifying a custom default template type --- docs/annotating_code/templated_annotations.md | 48 +++++++++++++++++++ .../Expression/Call/NewAnalyzer.php | 14 ++---- .../Reflector/ClassLikeNodeScanner.php | 41 +++++++++++++++- src/Psalm/Storage/ClassLikeStorage.php | 11 +++++ stubs/SPL.phpstub | 4 +- tests/Template/ClassTemplateTest.php | 22 +++++++++ 6 files changed, 128 insertions(+), 12 deletions(-) diff --git a/docs/annotating_code/templated_annotations.md b/docs/annotating_code/templated_annotations.md index 117c43fe274..fb2cb8d0d8f 100644 --- a/docs/annotating_code/templated_annotations.md +++ b/docs/annotating_code/templated_annotations.md @@ -113,6 +113,54 @@ function array_combine(array $arr, array $arr2) {} - `@template` tag order matters for class docblocks, as they dictate the order in which those generic parameters are referenced in docblocks. - The names of your templated types (e.g. `TKey`, `TValue`) don't matter outside the scope of the class or function in which they're declared. +## Default template values + +By default, if a class template parameter is not initialized in the constructor or by a `@var` annotation, Psalm will initialize it to `mixed`. + +However, a custom default initial type can also by provided using the following syntax: + +```php + */ + private array $values = []; + + /** @param T $value */ + public function addValue($value): void { + $this->values []= $value; + } + + /** @return list */ + public function getValue(): array { + return $this->values; + } +} + +$t1 = new MyContainer; + +/** @psalm-trace $t1 */; // MyContainer + +// InvalidArgument: Argument 1 of MyContainer::addValue expects never, but 123 provided +$t1->addValue(123); + + +/** @var MyContainer Always specify template parameters! */ +$t2 = new MyContainer; + +/** @psalm-trace $t2 */; // MyContainer + +// OK! +$t2->addValue(123); +``` + +This can be often useful, like in the above example, to always force specification of a template type when constructing generic objects by specifying `never` as default type: `never` is the [bottom type](https://psalm.dev/docs/annotating_code/type_syntax/top_bottom_types/#never), and thus all usages of methods relying on an *uninitialized* template type will use `never` and will emit Psalm issues, essentially warning the user to explicitly specify a template parameter when constructing the class. + ## @param class-string<T> Psalm also allows you to parameterize class types diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php index 1a9daef9fb5..156db0116c4 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php @@ -490,8 +490,8 @@ private static function analyzeNamedConstructor( $generic_param_types = null; - if ($storage->template_types) { - foreach ($storage->template_types as $template_name => $base_type) { + if ($storage->default_template_types) { + foreach ($storage->default_template_types as $template_name => $base_type) { if (isset($template_result->lower_bounds[$template_name][$fq_class_name])) { $generic_param_type = TemplateStandinTypeReplacer::getMostSpecificTypeFromBounds( $template_result->lower_bounds[$template_name][$fq_class_name], @@ -515,11 +515,7 @@ private static function analyzeNamedConstructor( ), ); } else { - if ($fq_class_name === 'SplObjectStorage') { - $generic_param_type = Type::getNever(); - } else { - $generic_param_type = array_values($base_type)[0]; - } + $generic_param_type = array_values($base_type)[0]; } $generic_param_types[] = $generic_param_type->setProperties([ @@ -550,13 +546,13 @@ private static function analyzeNamedConstructor( ), $statements_analyzer->getSuppressedIssues(), ); - } elseif ($storage->template_types) { + } elseif ($storage->default_template_types) { $result_atomic_type = new TGenericObject( $fq_class_name, array_values( array_map( static fn($map) => reset($map), - $storage->template_types, + $storage->default_template_types, ), ), false, diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php index 87da361b2e1..c7e8676798c 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php @@ -431,8 +431,16 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool if ($template_map[1] !== null && $template_map[2] !== null) { if (trim($template_map[2])) { $type_string = $template_map[2]; + $default_type_string = null; try { - $type_string = CommentAnalyzer::splitDocLine($type_string)[0]; + $type_string_split = CommentAnalyzer::splitDocLine($type_string); + if (isset($type_string_split[1]) + && isset($type_string_split[2]) + && $type_string_split[1] === '=' + ) { + $default_type_string = $type_string_split[2]; + } + $type_string = $type_string_split[0]; } catch (DocblockParseException $e) { throw new DocblockParseException( $type_string . ' is not a valid type: ' . $e->getMessage(), @@ -463,6 +471,37 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool $storage->template_types[$template_name] = [ $fq_classlike_name => $template_type, ]; + + if ($default_type_string !== null) { + $default_type_string = CommentAnalyzer::sanitizeDocblockType($default_type_string); + try { + $default_template_type = TypeParser::parseTokens( + TypeTokenizer::getFullyQualifiedTokens( + $default_type_string, + $this->aliases, + $storage->template_types, + $this->type_aliases, + ), + null, + $storage->template_types, + $this->type_aliases, + ); + } catch (TypeParseTreeException $e) { + $storage->docblock_issues[] = new InvalidDocblock( + $e->getMessage() . ' in docblock for ' . $fq_classlike_name, + $name_location ?? $class_location, + ); + + continue; + } + + $storage->default_template_types[$template_name] = [ + $fq_classlike_name => $default_template_type, + ]; + } else { + $storage->default_template_types[$template_name] = + $storage->template_types[$template_name]; + } } else { $storage->docblock_issues[] = new InvalidDocblock( 'Template missing as type', diff --git a/src/Psalm/Storage/ClassLikeStorage.php b/src/Psalm/Storage/ClassLikeStorage.php index 6aec220047d..4ce65683c6b 100644 --- a/src/Psalm/Storage/ClassLikeStorage.php +++ b/src/Psalm/Storage/ClassLikeStorage.php @@ -346,6 +346,17 @@ final class ClassLikeStorage implements HasAttributesInterface */ public $template_types; + /** + * An array holding the class template default types. + * + * The name of the template is the first key. The nested array is keyed by the defining class + * (i.e. the same as the class name). This allows operations with the same-named template defined + * across multiple classes to not run into trouble. + * + * @var array>|null + */ + public $default_template_types; + /** * @var array|null */ diff --git a/stubs/SPL.phpstub b/stubs/SPL.phpstub index 7a623c933a0..51b3e1cc06b 100644 --- a/stubs/SPL.phpstub +++ b/stubs/SPL.phpstub @@ -735,8 +735,8 @@ class SplPriorityQueue implements Iterator, Countable { * cases involving the need to uniquely identify objects. * @link https://php.net/manual/en/class.splobjectstorage.php * - * @template TObject as object - * @template TArrayValue + * @template TObject as object = never + * @template TArrayValue as mixed = never * @template-implements ArrayAccess * @template-implements Iterator */ diff --git a/tests/Template/ClassTemplateTest.php b/tests/Template/ClassTemplateTest.php index 12c0025461a..fb775377597 100644 --- a/tests/Template/ClassTemplateTest.php +++ b/tests/Template/ClassTemplateTest.php @@ -4209,6 +4209,28 @@ public function aggregate(...$values): null|int|float 'ignored_issues' => [], 'php_version' => '8.0', ], + 'defaultTemplateType' => [ + 'code' => ' [ + '$a===' => 'a', + ], + ], + 'initializeSplObjectStorage' => [ + 'code' => ' [ + '$a===' => 'SplObjectStorage', + ], + ], ]; }