From 60e0329e295774f0e488e22b3ec8a5218f0e265a Mon Sep 17 00:00:00 2001 From: Romain Canon Date: Sun, 7 Apr 2024 22:37:26 +0200 Subject: [PATCH] fix: properly handle nested local type aliases The following annotation now works properly: ```php /** * @phpstan-type Address = array{street?: string, city: string} * @phpstan-type User = array{name: non-empty-string, address: Address} */ final class SomeClass { public function __construct( /** @var User */ public $value, ) {} } (new \CuyZ\Valinor\MapperBuilder()) ->allowSuperfluousKeys() ->mapper() ->map(SomeClass::class, [ 'name' => 'John Doe', 'address' => [ 'street' => 'Bron-Yr-Aur', 'city' => 'SY20 8QA, Machynlleth', ], ]); ``` --- .../ClassImportedTypeAliasResolver.php | 2 +- .../ClassLocalTypeAliasResolver.php | 8 ++-- .../TypeResolver/ClassParentTypeResolver.php | 2 +- .../TypeResolver/ClassTemplatesResolver.php | 5 +-- .../FunctionReturnTypeResolver.php | 2 +- .../TypeResolver/ParameterTypeResolver.php | 2 +- .../TypeResolver/PropertyTypeResolver.php | 2 +- src/Type/Parser/Lexer/Annotations.php | 40 +++++++++++++----- src/Type/Parser/Lexer/TokenizedAnnotation.php | 10 +++++ .../ClassLocalTypeAliasResolverTest.php | 42 +++++++++++++++++++ .../TypeResolver/PropertyTypeResolverTest.php | 10 +++++ .../Type/Parser/Lexer/AnnotationsTest.php | 2 +- 12 files changed, 103 insertions(+), 24 deletions(-) diff --git a/src/Definition/Repository/Reflection/TypeResolver/ClassImportedTypeAliasResolver.php b/src/Definition/Repository/Reflection/TypeResolver/ClassImportedTypeAliasResolver.php index d951200c..84ae4212 100644 --- a/src/Definition/Repository/Reflection/TypeResolver/ClassImportedTypeAliasResolver.php +++ b/src/Definition/Repository/Reflection/TypeResolver/ClassImportedTypeAliasResolver.php @@ -77,7 +77,7 @@ private function extractImportedAliasesFromDocBlock(string $className): array $importedAliases = []; - $annotations = (new Annotations($docBlock))->allOf( + $annotations = (new Annotations($docBlock))->filteredByPriority( '@phpstan-import-type', '@psalm-import-type', ); diff --git a/src/Definition/Repository/Reflection/TypeResolver/ClassLocalTypeAliasResolver.php b/src/Definition/Repository/Reflection/TypeResolver/ClassLocalTypeAliasResolver.php index 84d07529..dd57537f 100644 --- a/src/Definition/Repository/Reflection/TypeResolver/ClassLocalTypeAliasResolver.php +++ b/src/Definition/Repository/Reflection/TypeResolver/ClassLocalTypeAliasResolver.php @@ -32,12 +32,12 @@ public function resolveLocalTypeAliases(ObjectType $type): array return []; } - $typeParser = $this->typeParserFactory->buildAdvancedTypeParserForClass($type); - $types = []; foreach ($localAliases as $name => $raw) { try { + $typeParser = $this->typeParserFactory->buildAdvancedTypeParserForClass($type, $types); + $types[$name] = $typeParser->parse($raw); } catch (InvalidType $exception) { $types[$name] = UnresolvableType::forLocalAlias($raw, $name, $type, $exception); @@ -61,7 +61,7 @@ private function extractLocalAliasesFromDocBlock(string $className): array $aliases = []; - $annotations = (new Annotations($docBlock))->allOf( + $annotations = (new Annotations($docBlock))->filteredInOrder( '@phpstan-type', '@psalm-type', ); @@ -80,7 +80,7 @@ private function extractLocalAliasesFromDocBlock(string $className): array $key = key($tokens); if ($key !== null) { - $aliases[$name] ??= $annotation->allAfter($key); + $aliases[$name] = $annotation->allAfter($key); } } diff --git a/src/Definition/Repository/Reflection/TypeResolver/ClassParentTypeResolver.php b/src/Definition/Repository/Reflection/TypeResolver/ClassParentTypeResolver.php index f6b8ec31..e2afd92f 100644 --- a/src/Definition/Repository/Reflection/TypeResolver/ClassParentTypeResolver.php +++ b/src/Definition/Repository/Reflection/TypeResolver/ClassParentTypeResolver.php @@ -72,7 +72,7 @@ private function extractParentTypeFromDocBlock(ReflectionClass $reflection): arr return []; } - $annotations = (new Annotations($docBlock))->allOf( + $annotations = (new Annotations($docBlock))->filteredByPriority( '@phpstan-extends', '@psalm-extends', '@extends', diff --git a/src/Definition/Repository/Reflection/TypeResolver/ClassTemplatesResolver.php b/src/Definition/Repository/Reflection/TypeResolver/ClassTemplatesResolver.php index cc2d36c6..6f90973b 100644 --- a/src/Definition/Repository/Reflection/TypeResolver/ClassTemplatesResolver.php +++ b/src/Definition/Repository/Reflection/TypeResolver/ClassTemplatesResolver.php @@ -10,7 +10,6 @@ use function array_key_exists; use function array_keys; -use function array_reverse; use function current; use function key; @@ -40,7 +39,7 @@ public function resolveTemplatesFrom(string $className): array $templates = []; - $annotations = (new Annotations($docBlock))->allOf( + $annotations = (new Annotations($docBlock))->filteredByPriority( '@phpstan-template', '@psalm-template', '@template', @@ -72,6 +71,6 @@ public function resolveTemplatesFrom(string $className): array } } - return array_reverse($templates); + return $templates; } } diff --git a/src/Definition/Repository/Reflection/TypeResolver/FunctionReturnTypeResolver.php b/src/Definition/Repository/Reflection/TypeResolver/FunctionReturnTypeResolver.php index 9d4bf274..7750af39 100644 --- a/src/Definition/Repository/Reflection/TypeResolver/FunctionReturnTypeResolver.php +++ b/src/Definition/Repository/Reflection/TypeResolver/FunctionReturnTypeResolver.php @@ -37,6 +37,6 @@ private function extractReturnTypeFromDocBlock(ReflectionFunctionAbstract $refle '@phpstan-return', '@psalm-return', '@return', - ); + )?->raw(); } } diff --git a/src/Definition/Repository/Reflection/TypeResolver/ParameterTypeResolver.php b/src/Definition/Repository/Reflection/TypeResolver/ParameterTypeResolver.php index 836ddb50..bab24b08 100644 --- a/src/Definition/Repository/Reflection/TypeResolver/ParameterTypeResolver.php +++ b/src/Definition/Repository/Reflection/TypeResolver/ParameterTypeResolver.php @@ -58,7 +58,7 @@ private function extractTypeFromDocBlock(ReflectionParameter $reflection): ?stri return null; } - $annotations = (new Annotations($docBlock))->allOf( + $annotations = (new Annotations($docBlock))->filteredByPriority( '@phpstan-param', '@psalm-param', '@param', diff --git a/src/Definition/Repository/Reflection/TypeResolver/PropertyTypeResolver.php b/src/Definition/Repository/Reflection/TypeResolver/PropertyTypeResolver.php index 864156a2..f2212e8d 100644 --- a/src/Definition/Repository/Reflection/TypeResolver/PropertyTypeResolver.php +++ b/src/Definition/Repository/Reflection/TypeResolver/PropertyTypeResolver.php @@ -37,6 +37,6 @@ public function extractTypeFromDocBlock(ReflectionProperty $reflection): ?string '@phpstan-var', '@psalm-var', '@var', - ); + )?->raw(); } } diff --git a/src/Type/Parser/Lexer/Annotations.php b/src/Type/Parser/Lexer/Annotations.php index 3105d8cc..8cc7fa65 100644 --- a/src/Type/Parser/Lexer/Annotations.php +++ b/src/Type/Parser/Lexer/Annotations.php @@ -4,14 +4,16 @@ namespace CuyZ\Valinor\Type\Parser\Lexer; +use function array_filter; use function array_merge; use function current; +use function in_array; use function trim; /** @internal */ final class Annotations { - /** @var array> */ + /** @var list */ private array $annotations = []; public function __construct(string $docBlock) @@ -27,7 +29,7 @@ public function __construct(string $docBlock) $current = $this->trimArrayTips($current); if ($current !== []) { - $this->annotations[$token][] = new TokenizedAnnotation($current); + array_unshift($this->annotations, new TokenizedAnnotation($token, $current)); } $current = []; @@ -37,31 +39,47 @@ public function __construct(string $docBlock) } } - public function firstOf(string ...$annotations): ?string + public function firstOf(string ...$annotations): ?TokenizedAnnotation { foreach ($annotations as $annotation) { - if (isset($this->annotations[$annotation])) { - return $this->annotations[$annotation][0]->raw(); + foreach ($this->annotations as $tokenizedAnnotation) { + if ($tokenizedAnnotation->name() === $annotation) { + return $tokenizedAnnotation; + } } } return null; } + /** + * @return array + */ + public function filteredInOrder(string ...$annotations): array + { + return array_filter( + $this->annotations, + static fn (TokenizedAnnotation $tokenizedAnnotation) => in_array($tokenizedAnnotation->name(), $annotations, true), + ); + } + /** * @return list */ - public function allOf(string ...$annotations): array + public function filteredByPriority(string ...$annotations): array { - $all = []; + $result = []; foreach ($annotations as $annotation) { - if (isset($this->annotations[$annotation])) { - $all = array_merge($all, $this->annotations[$annotation]); - } + $filtered = array_filter( + $this->annotations, + static fn (TokenizedAnnotation $tokenizedAnnotation) => $tokenizedAnnotation->name() === $annotation, + ); + + $result = array_merge($result, $filtered); } - return $all; + return $result; } private function sanitizeDocComment(string $value): string diff --git a/src/Type/Parser/Lexer/TokenizedAnnotation.php b/src/Type/Parser/Lexer/TokenizedAnnotation.php index 3f40c4bc..4e4c403c 100644 --- a/src/Type/Parser/Lexer/TokenizedAnnotation.php +++ b/src/Type/Parser/Lexer/TokenizedAnnotation.php @@ -11,10 +11,20 @@ final class TokenizedAnnotation { public function __construct( + /** @var non-empty-string */ + private string $name, /** @var non-empty-list> */ private array $tokens, ) {} + /** + * @return non-empty-string + */ + public function name(): string + { + return $this->name; + } + public function splice(int $length): string { return implode('', array_splice($this->tokens, 0, $length)); diff --git a/tests/Functional/Definition/Repository/Reflection/TypeResolver/ClassLocalTypeAliasResolverTest.php b/tests/Functional/Definition/Repository/Reflection/TypeResolver/ClassLocalTypeAliasResolverTest.php index f162f10c..b32b87d3 100644 --- a/tests/Functional/Definition/Repository/Reflection/TypeResolver/ClassLocalTypeAliasResolverTest.php +++ b/tests/Functional/Definition/Repository/Reflection/TypeResolver/ClassLocalTypeAliasResolverTest.php @@ -81,5 +81,47 @@ public static function local_type_alias_is_resolved_properly_data_provider(): it 'SomeType' => 'int<42, 1337>', ] ]; + + yield 'types can be nested' => [ + 'className' => ( + /** + * @phpstan-type SomeType = non-empty-string + * @phpstan-type SomeNestedType = array + */ + new class () {} + )::class, + [ + 'SomeType' => 'non-empty-string', + 'SomeNestedType' => 'array', + ] + ]; + + yield 'PHPStan type can use a Psalm type' => [ + 'className' => ( + /** + * @psalm-type SomeType = non-empty-string + * @phpstan-type SomeNestedType = array + */ + new class () {} + )::class, + [ + 'SomeType' => 'non-empty-string', + 'SomeNestedType' => 'array', + ] + ]; + + yield 'Psalm type can use a PHPStan type' => [ + 'className' => ( + /** + * @phpstan-type SomeType = non-empty-string + * @psalm-type SomeNestedType = array + */ + new class () {} + )::class, + [ + 'SomeType' => 'non-empty-string', + 'SomeNestedType' => 'array', + ] + ]; } } diff --git a/tests/Functional/Definition/Repository/Reflection/TypeResolver/PropertyTypeResolverTest.php b/tests/Functional/Definition/Repository/Reflection/TypeResolver/PropertyTypeResolverTest.php index 4a37f902..0aa0dfb0 100644 --- a/tests/Functional/Definition/Repository/Reflection/TypeResolver/PropertyTypeResolverTest.php +++ b/tests/Functional/Definition/Repository/Reflection/TypeResolver/PropertyTypeResolverTest.php @@ -139,5 +139,15 @@ public static function property_type_is_resolved_properly_data_provider(): itera }, 'foo'), 'non-empty-string', ]; + + yield 'docBlock present but no @var annotation' => [ + new ReflectionProperty(new class () { + /** + * Some comment + */ + public string $foo; + }, 'foo'), + 'string', + ]; } } diff --git a/tests/Unit/Type/Parser/Lexer/AnnotationsTest.php b/tests/Unit/Type/Parser/Lexer/AnnotationsTest.php index 579d3a44..f6cec9b4 100644 --- a/tests/Unit/Type/Parser/Lexer/AnnotationsTest.php +++ b/tests/Unit/Type/Parser/Lexer/AnnotationsTest.php @@ -24,7 +24,7 @@ public function test_annotations_are_parsed_properly(string $docBlock, array $ex foreach ($expectedAnnotations as $name => $expected) { $result = array_map( fn (TokenizedAnnotation $annotation) => $annotation->raw(), - $annotations->allOf($name) + $annotations->filteredByPriority($name) ); self::assertSame($expected, $result);