fix: properly handle nested unresolvable type during mapping

This change brings two enhancements: - When mapping to a structure that contains a nested unresolvable type, for instance a doc-block type that does not match the native type, the mapper will detect it and throw a proper exception (it used to just crash on a badly designed assertion). - Unresolvable types that are not used during mapping, for instance parameters of methods that are not used by the mapper will no longer throw an exception even if not needed by the library. Instead, an unresolvable type is assigned to these invalid properties/parameters/ return types.
CuyZ · Mar 27, 2024 · 1947061 · 1947061
1 parent 0f5e96e
commit 1947061
Show file tree

Hide file tree

Showing 12 changed files with 168 additions and 78 deletions.
diff --git a/rector.php b/rector.php
@@ -35,6 +35,7 @@
         ReadOnlyPropertyRector::class,
         MixedTypeRector::class => [
             __DIR__ . '/tests/Unit/Definition/Repository/Reflection/ReflectionClassDefinitionRepositoryTest',
+            __DIR__ . '/tests/Integration/Mapping/TypeErrorDuringMappingTest.php',
         ],
         NullToStrictStringFuncCallArgRector::class => [
             __DIR__ . '/tests/Traits/TestIsSingleton.php',

diff --git a/src/Definition/Exception/TypesDoNotMatch.php b/src/Definition/Exception/TypesDoNotMatch.php
diff --git a/src/Definition/Repository/Reflection/ReflectionTypeResolver.php b/src/Definition/Repository/Reflection/ReflectionTypeResolver.php
@@ -4,7 +4,6 @@
 
 namespace CuyZ\Valinor\Definition\Repository\Reflection;
 
-use CuyZ\Valinor\Definition\Exception\TypesDoNotMatch;
 use CuyZ\Valinor\Type\GenericType;
 use CuyZ\Valinor\Type\Parser\Exception\InvalidType;
 use CuyZ\Valinor\Type\Parser\TypeParser;
@@ -51,7 +50,7 @@ public function resolveType(ReflectionProperty|ReflectionParameter|ReflectionFun
         }
 
         if (! $typeFromDocBlock->matches($nativeType)) {
-            throw new TypesDoNotMatch($reflection, $typeFromDocBlock, $nativeType);
+            return UnresolvableType::forDocBlockTypeNotMatchingNative($reflection, $typeFromDocBlock, $nativeType);
         }
 
         return $typeFromDocBlock;

diff --git a/src/Mapper/Exception/TypeErrorDuringArgumentsMapping.php b/src/Mapper/Exception/TypeErrorDuringArgumentsMapping.php
@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace CuyZ\Valinor\Mapper\Exception;
+
+use CuyZ\Valinor\Definition\FunctionDefinition;
+use CuyZ\Valinor\Mapper\Tree\Exception\UnresolvableShellType;
+use LogicException;
+
+/** @internal */
+final class TypeErrorDuringArgumentsMapping extends LogicException
+{
+    public function __construct(FunctionDefinition $function, UnresolvableShellType $exception)
+    {
+        parent::__construct(
+            "Could not map arguments of `$function->signature`: {$exception->getMessage()}",
+            1711534351,
+            $exception,
+        );
+    }
+}
diff --git a/src/Mapper/Exception/TypeErrorDuringMapping.php b/src/Mapper/Exception/TypeErrorDuringMapping.php
@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace CuyZ\Valinor\Mapper\Exception;
+
+use CuyZ\Valinor\Mapper\Tree\Exception\UnresolvableShellType;
+use CuyZ\Valinor\Type\Type;
+use LogicException;
+
+/** @internal */
+final class TypeErrorDuringMapping extends LogicException
+{
+    public function __construct(Type $type, UnresolvableShellType $exception)
+    {
+        parent::__construct(
+            "Error while trying to map to `{$type->toString()}`: {$exception->getMessage()}",
+            1711526329,
+            $exception,
+        );
+    }
+}
diff --git a/src/Mapper/Tree/Exception/UnresolvableShellType.php b/src/Mapper/Tree/Exception/UnresolvableShellType.php
@@ -0,0 +1,17 @@
+<?php
+
+declare(strict_types=1);
+
+namespace CuyZ\Valinor\Mapper\Tree\Exception;
+
+use CuyZ\Valinor\Type\Types\UnresolvableType;
+use LogicException;
+
+/** @internal */
+final class UnresolvableShellType extends LogicException
+{
+    public function __construct(UnresolvableType $type)
+    {
+        parent::__construct($type->message());
+    }
+}
diff --git a/src/Mapper/Tree/Shell.php b/src/Mapper/Tree/Shell.php
@@ -5,6 +5,7 @@
 namespace CuyZ\Valinor\Mapper\Tree;
 
 use CuyZ\Valinor\Definition\Attributes;
+use CuyZ\Valinor\Mapper\Tree\Exception\UnresolvableShellType;
 use CuyZ\Valinor\Type\Type;
 use CuyZ\Valinor\Type\Types\UnresolvableType;
 
@@ -27,7 +28,9 @@ final class Shell
 
     private function __construct(private Type $type)
     {
-        assert(! $type instanceof UnresolvableType);
+        if ($type instanceof UnresolvableType) {
+            throw new UnresolvableShellType($type);
+        }
     }
 
     public static function root(Type $type, mixed $value): self

diff --git a/src/Mapper/TypeArgumentsMapper.php b/src/Mapper/TypeArgumentsMapper.php
@@ -6,7 +6,9 @@
 
 use CuyZ\Valinor\Definition\ParameterDefinition;
 use CuyZ\Valinor\Definition\Repository\FunctionDefinitionRepository;
+use CuyZ\Valinor\Mapper\Exception\TypeErrorDuringArgumentsMapping;
 use CuyZ\Valinor\Mapper\Tree\Builder\RootNodeBuilder;
+use CuyZ\Valinor\Mapper\Tree\Exception\UnresolvableShellType;
 use CuyZ\Valinor\Mapper\Tree\Shell;
 use CuyZ\Valinor\Type\Types\ShapedArrayElement;
 use CuyZ\Valinor\Type\Types\ShapedArrayType;
@@ -42,7 +44,11 @@ public function mapArguments(callable $callable, mixed $source): array
         $type = new ShapedArrayType(...$elements);
         $shell = Shell::root($type, $source);
 
-        $node = $this->nodeBuilder->build($shell);
+        try {
+            $node = $this->nodeBuilder->build($shell);
+        } catch (UnresolvableShellType $exception) {
+            throw new TypeErrorDuringArgumentsMapping($function, $exception);
+        }
 
         if (! $node->isValid()) {
             throw new ArgumentsMapperError($function, $node->node());

diff --git a/src/Mapper/TypeTreeMapper.php b/src/Mapper/TypeTreeMapper.php
@@ -5,8 +5,10 @@
 namespace CuyZ\Valinor\Mapper;
 
 use CuyZ\Valinor\Mapper\Exception\InvalidMappingTypeSignature;
+use CuyZ\Valinor\Mapper\Exception\TypeErrorDuringMapping;
 use CuyZ\Valinor\Mapper\Tree\Builder\RootNodeBuilder;
 use CuyZ\Valinor\Mapper\Tree\Builder\TreeNode;
+use CuyZ\Valinor\Mapper\Tree\Exception\UnresolvableShellType;
 use CuyZ\Valinor\Mapper\Tree\Shell;
 use CuyZ\Valinor\Type\Parser\Exception\InvalidType;
 use CuyZ\Valinor\Type\Parser\TypeParser;
@@ -41,6 +43,10 @@ private function node(string $signature, mixed $source): TreeNode
 
         $shell = Shell::root($type, $source);
 
-        return $this->nodeBuilder->build($shell);
+        try {
+            return $this->nodeBuilder->build($shell);
+        } catch (UnresolvableShellType $exception) {
+            throw new TypeErrorDuringMapping($type, $exception);
+        }
     }
 }
diff --git a/src/Type/Types/UnresolvableType.php b/src/Type/Types/UnresolvableType.php
@@ -7,8 +7,12 @@
 use CuyZ\Valinor\Type\ClassType;
 use CuyZ\Valinor\Type\Parser\Exception\InvalidType;
 use CuyZ\Valinor\Type\Type;
+use CuyZ\Valinor\Utility\Reflection\Reflection;
 use CuyZ\Valinor\Utility\ValueDumper;
 use LogicException;
+use ReflectionFunctionAbstract;
+use ReflectionParameter;
+use ReflectionProperty;
 
 /** @internal */
 final class UnresolvableType implements Type
@@ -62,6 +66,21 @@ public static function forInvalidParameterDefaultValue(string $signature, Type $
         );
     }
 
+    public static function forDocBlockTypeNotMatchingNative(ReflectionProperty|ReflectionParameter|ReflectionFunctionAbstract $reflection, Type $typeFromDocBlock, Type $typeFromReflection): self
+    {
+        $signature = Reflection::signature($reflection);
+
+        if ($reflection instanceof ReflectionProperty) {
+            $message = "Types for property `$signature` do not match: `{$typeFromDocBlock->toString()}` (docblock) does not accept `{$typeFromReflection->toString()}` (native).";
+        } elseif ($reflection instanceof ReflectionParameter) {
+            $message = "Types for parameter `$signature` do not match: `{$typeFromDocBlock->toString()}` (docblock) does not accept `{$typeFromReflection->toString()}` (native).";
+        } else {
+            $message = "Return types for method `$signature` do not match: `{$typeFromDocBlock->toString()}` (docblock) does not accept `{$typeFromReflection->toString()}` (native).";
+        }
+
+        return new self($typeFromDocBlock->toString(), $message);
+    }
+
     public static function forLocalAlias(string $raw, string $name, ClassType $type, InvalidType $exception): self
     {
         return new self(

diff --git a/tests/Integration/Mapping/TypeErrorDuringMappingTest.php b/tests/Integration/Mapping/TypeErrorDuringMappingTest.php
@@ -0,0 +1,61 @@
+<?php
+
+declare(strict_types=1);
+
+namespace CuyZ\Valinor\Tests\Integration\Mapping;
+
+use CuyZ\Valinor\Mapper\Exception\TypeErrorDuringArgumentsMapping;
+use CuyZ\Valinor\Mapper\Exception\TypeErrorDuringMapping;
+use CuyZ\Valinor\Tests\Integration\IntegrationTestCase;
+
+final class TypeErrorDuringMappingTest extends IntegrationTestCase
+{
+    public function test_property_with_non_matching_types_throws_exception(): void
+    {
+        $class = (new class () {
+            /**
+             * @phpstan-ignore-next-line
+             * @var string
+             */
+            public bool $propertyWithNotMatchingTypes;
+        })::class;
+
+        $this->expectException(TypeErrorDuringMapping::class);
+        $this->expectExceptionCode(1711526329);
+        $this->expectExceptionMessage("Error while trying to map to `$class`: Types for property `$class::\$propertyWithNotMatchingTypes` do not match: `string` (docblock) does not accept `bool` (native).");
+
+        $this->mapperBuilder()->mapper()->map($class, ['propertyWithNotMatchingTypes' => true]);
+    }
+
+    public function test_parameter_with_non_matching_types_throws_exception(): void
+    {
+        $class = (new class (true) {
+            /**
+             * @param string $parameterWithNotMatchingTypes
+             * @phpstan-ignore-next-line
+             */
+            public function __construct(public bool $parameterWithNotMatchingTypes) {}
+        })::class;
+
+        $this->expectException(TypeErrorDuringMapping::class);
+        $this->expectExceptionCode(1711526329);
+        $this->expectExceptionMessage("Error while trying to map to `$class`: Types for parameter `$class::__construct(\$parameterWithNotMatchingTypes)` do not match: `string` (docblock) does not accept `bool` (native).");
+
+        $this->mapperBuilder()->mapper()->map($class, ['parameterWithNotMatchingTypes' => true]);
+    }
+
+    public function test_function_parameter_with_non_matching_types_throws_exception(): void
+    {
+        $function =
+            /**
+             * @param string $parameterWithNotMatchingTypes
+             */
+            fn (bool $parameterWithNotMatchingTypes): string => 'foo';
+
+        $this->expectException(TypeErrorDuringArgumentsMapping::class);
+        $this->expectExceptionCode(1711534351);
+        $this->expectExceptionMessageMatches("/Could not map arguments of `.*`: Types for parameter `.*` do not match: `string` \(docblock\) does not accept `bool` \(native\)\./");
+
+        $this->mapperBuilder()->argumentsMapper()->mapArguments($function, ['parameterWithNotMatchingTypes' => true]);
+    }
+}
diff --git a/tests/Unit/Definition/Repository/Reflection/ReflectionClassDefinitionRepositoryTest.php b/tests/Unit/Definition/Repository/Reflection/ReflectionClassDefinitionRepositoryTest.php
@@ -11,7 +11,6 @@
 use CuyZ\Valinor\Definition\Exception\InvalidTypeAliasImportClass;
 use CuyZ\Valinor\Definition\Exception\InvalidTypeAliasImportClassType;
 use CuyZ\Valinor\Definition\Exception\SeveralExtendTagsFound;
-use CuyZ\Valinor\Definition\Exception\TypesDoNotMatch;
 use CuyZ\Valinor\Definition\Exception\UnknownTypeAliasImport;
 use CuyZ\Valinor\Definition\Repository\Reflection\ReflectionClassDefinitionRepository;
 use CuyZ\Valinor\Tests\Fake\Type\FakeType;
@@ -177,7 +176,6 @@ public function test_invalid_property_type_throws_exception(): void
         $type = $class->properties->get('propertyWithInvalidType')->type;
 
         self::assertInstanceOf(UnresolvableType::class, $type);
-        /** @var UnresolvableType $type */
         self::assertMatchesRegularExpression('/^The type `InvalidType` for property `.*` could not be resolved: .*$/', $type->message());
     }
 
@@ -195,23 +193,6 @@ public function test_invalid_property_default_value_throws_exception(): void
         self::assertMatchesRegularExpression('/Property `.*::\$propertyWithInvalidDefaultValue` of type `string` has invalid default value false/', $type->message());
     }
 
-    public function test_property_with_non_matching_types_throws_exception(): void
-    {
-        $class = (new class () {
-            /**
-             * @phpstan-ignore-next-line
-             * @var string
-             */
-            public bool $propertyWithNotMatchingTypes;
-        })::class;
-
-        $this->expectException(TypesDoNotMatch::class);
-        $this->expectExceptionCode(1638471381);
-        $this->expectExceptionMessage("Types for property `$class::\$propertyWithNotMatchingTypes` do not match: `string` (docblock) does not accept `bool` (native).");
-
-        $this->repository->for(new NativeClassType($class));
-    }
-
     public function test_invalid_parameter_type_throws_exception(): void
     {
         $class = (new class () {
@@ -228,7 +209,6 @@ public function publicMethod($parameterWithInvalidType): void {}
         $type = $class->methods->get('publicMethod')->parameters->get('parameterWithInvalidType')->type;
 
         self::assertInstanceOf(UnresolvableType::class, $type);
-        /** @var UnresolvableType $type */
         self::assertMatchesRegularExpression('/^The type `InvalidTypeWithPendingSpaces` for parameter `.*` could not be resolved: .*$/', $type->message());
     }
 
@@ -246,7 +226,6 @@ public function publicMethod($parameterWithInvalidType): void {}
         $type = $class->methods->get('publicMethod')->returnType;
 
         self::assertInstanceOf(UnresolvableType::class, $type);
-        /** @var UnresolvableType $type */
         self::assertMatchesRegularExpression('/^The type `InvalidType` for return type of method `.*` could not be resolved: .*$/', $type->message());
     }
 
@@ -267,23 +246,6 @@ public function publicMethod($parameterWithInvalidDefaultValue = false): void {}
         self::assertMatchesRegularExpression('/Parameter `.*::publicMethod\(\$parameterWithInvalidDefaultValue\)` of type `string` has invalid default value false/', $type->message());
     }
 
-    public function test_parameter_with_non_matching_types_throws_exception(): void
-    {
-        $class = (new class () {
-            /**
-             * @param string $parameterWithNotMatchingTypes
-             * @phpstan-ignore-next-line
-             */
-            public function publicMethod(bool $parameterWithNotMatchingTypes): void {}
-        })::class;
-
-        $this->expectException(TypesDoNotMatch::class);
-        $this->expectExceptionCode(1638471381);
-        $this->expectExceptionMessage("Types for parameter `$class::publicMethod(\$parameterWithNotMatchingTypes)` do not match: `string` (docblock) does not accept `bool` (native).");
-
-        $this->repository->for(new NativeClassType($class));
-    }
-
     public function test_method_with_non_matching_return_types_throws_exception(): void
     {
         $class = (new class () {
@@ -297,11 +259,14 @@ public function publicMethod(): string
             }
         })::class;
 
-        $this->expectException(TypesDoNotMatch::class);
-        $this->expectExceptionCode(1638471381);
-        $this->expectExceptionMessage("Return types for method `$class::publicMethod()` do not match: `bool` (docblock) does not accept `string` (native).");
+        $returnType = $this->repository
+            ->for(new NativeClassType($class))
+            ->methods
+            ->get('publicMethod')
+            ->returnType;
 
-        $this->repository->for(new NativeClassType($class));
+        self::assertInstanceOf(UnresolvableType::class, $returnType);
+        self::assertMatchesRegularExpression('/^Return types for method `.*` do not match: `bool` \(docblock\) does not accept `string` \(native\).$/', $returnType->message());
     }
 
     public function test_class_with_local_type_alias_name_duplication_throws_exception(): void