From 80e9681680999296f92ed535baf330dbb77c4b79 Mon Sep 17 00:00:00 2001 From: Romain Canon Date: Fri, 7 Jul 2023 13:14:30 +0200 Subject: [PATCH] fix: remove exception inheritance from `UnresolvableType` Because `UnresolvableType` can be applied to any property, parameter or function/method return type, it can be present in a class definition hierarchy. This class definition can then be cached for better application performance, meaning these `UnresolvableType` will be present in the cache entry. This is a problem because an exception contains a lot of information, including reflection instances which can not be serialized and can break caching mechanisms that rely on serialization. `UnresolvableType` extending `LogicException` was not necessary, so it is removed and the code relying on it has been adapted. --- .../Cache/Compiler/TypeCompiler.php | 2 +- .../InvalidConstructorReturnType.php | 4 +-- src/Type/Types/UnresolvableType.php | 29 ++++++++++++------- ...eflectionClassDefinitionRepositoryTest.php | 10 +++---- .../Unit/Type/Types/UnresolvableTypeTest.php | 16 ++++------ 5 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/Definition/Repository/Cache/Compiler/TypeCompiler.php b/src/Definition/Repository/Cache/Compiler/TypeCompiler.php index c4f06c35..a1a75a6d 100644 --- a/src/Definition/Repository/Cache/Compiler/TypeCompiler.php +++ b/src/Definition/Repository/Cache/Compiler/TypeCompiler.php @@ -163,7 +163,7 @@ public function compile(Type $type): string return "new $class($enumName, $pattern, [$cases])"; case $type instanceof UnresolvableType: $raw = var_export($type->toString(), true); - $message = var_export($type->getMessage(), true); + $message = var_export($type->message(), true); return "new $class($raw, $message)"; default: diff --git a/src/Mapper/Object/Exception/InvalidConstructorReturnType.php b/src/Mapper/Object/Exception/InvalidConstructorReturnType.php index 6498a1b0..008c9c3b 100644 --- a/src/Mapper/Object/Exception/InvalidConstructorReturnType.php +++ b/src/Mapper/Object/Exception/InvalidConstructorReturnType.php @@ -16,8 +16,8 @@ public function __construct(FunctionDefinition $function) $returnType = $function->returnType(); if ($returnType instanceof UnresolvableType) { - $message = $returnType->getMessage(); - $previous = $returnType; + $message = $returnType->message(); + $previous = $returnType->previous(); } else { $message = "Invalid return type `{$returnType->toString()}` for constructor `{$function->signature()}`, it must be a valid class name."; $previous = null; diff --git a/src/Type/Types/UnresolvableType.php b/src/Type/Types/UnresolvableType.php index 3d74be3f..c32d5553 100644 --- a/src/Type/Types/UnresolvableType.php +++ b/src/Type/Types/UnresolvableType.php @@ -12,16 +12,13 @@ use Throwable; /** @internal */ -final class UnresolvableType extends LogicException implements Type +final class UnresolvableType implements Type { - private string $rawType; - - public function __construct(string $rawType, string $message, ?Throwable $previous = null) - { - $this->rawType = $rawType; - - parent::__construct($message, 1679578492, $previous); - } + public function __construct( + private string $rawType, + private string $message, + private ?Throwable $previous = null + ) {} public static function forProperty(string $raw, string $signature, InvalidType $exception): self { @@ -79,14 +76,24 @@ public static function forLocalAlias(string $raw, string $name, ClassType $type, ); } + public function message(): string + { + return $this->message; + } + + public function previous(): ?Throwable + { + return $this->previous; + } + public function accepts(mixed $value): bool { - throw $this; + throw new LogicException(); } public function matches(Type $other): bool { - throw $this; + throw new LogicException(); } public function toString(): string diff --git a/tests/Unit/Definition/Repository/Reflection/ReflectionClassDefinitionRepositoryTest.php b/tests/Unit/Definition/Repository/Reflection/ReflectionClassDefinitionRepositoryTest.php index 7ac9aecc..d937050a 100644 --- a/tests/Unit/Definition/Repository/Reflection/ReflectionClassDefinitionRepositoryTest.php +++ b/tests/Unit/Definition/Repository/Reflection/ReflectionClassDefinitionRepositoryTest.php @@ -166,7 +166,7 @@ public function test_invalid_property_type_throws_exception(): void self::assertInstanceOf(UnresolvableType::class, $type); /** @var UnresolvableType $type */ - self::assertMatchesRegularExpression('/^The type `InvalidType` for property `.*` could not be resolved: .*$/', $type->getMessage()); + self::assertMatchesRegularExpression('/^The type `InvalidType` for property `.*` could not be resolved: .*$/', $type->message()); } public function test_invalid_property_default_value_throws_exception(): void @@ -180,7 +180,7 @@ public function test_invalid_property_default_value_throws_exception(): void $type = $class->properties()->get('propertyWithInvalidDefaultValue')->type(); self::assertInstanceOf(UnresolvableType::class, $type); - self::assertMatchesRegularExpression('/Property `.*::\$propertyWithInvalidDefaultValue` of type `string` has invalid default value false/', $type->getMessage()); + 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 @@ -217,7 +217,7 @@ public function publicMethod($parameterWithInvalidType): void {} self::assertInstanceOf(UnresolvableType::class, $type); /** @var UnresolvableType $type */ - self::assertMatchesRegularExpression('/^The type `InvalidTypeWithPendingSpaces` for parameter `.*` could not be resolved: .*$/', $type->getMessage()); + self::assertMatchesRegularExpression('/^The type `InvalidTypeWithPendingSpaces` for parameter `.*` could not be resolved: .*$/', $type->message()); } public function test_invalid_method_return_type_throws_exception(): void @@ -235,7 +235,7 @@ public function publicMethod($parameterWithInvalidType): void {} self::assertInstanceOf(UnresolvableType::class, $type); /** @var UnresolvableType $type */ - self::assertMatchesRegularExpression('/^The type `InvalidType` for return type of method `.*` could not be resolved: .*$/', $type->getMessage()); + self::assertMatchesRegularExpression('/^The type `InvalidType` for return type of method `.*` could not be resolved: .*$/', $type->message()); } public function test_invalid_parameter_default_value_throws_exception(): void @@ -252,7 +252,7 @@ public function publicMethod($parameterWithInvalidDefaultValue = false): void {} $type = $class->methods()->get('publicMethod')->parameters()->get('parameterWithInvalidDefaultValue')->type(); self::assertInstanceOf(UnresolvableType::class, $type); - self::assertMatchesRegularExpression('/Parameter `.*::publicMethod\(\$parameterWithInvalidDefaultValue\)` of type `string` has invalid default value false/', $type->getMessage()); + 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 diff --git a/tests/Unit/Type/Types/UnresolvableTypeTest.php b/tests/Unit/Type/Types/UnresolvableTypeTest.php index 3b355aab..eb6c728e 100644 --- a/tests/Unit/Type/Types/UnresolvableTypeTest.php +++ b/tests/Unit/Type/Types/UnresolvableTypeTest.php @@ -6,31 +6,25 @@ use CuyZ\Valinor\Tests\Fake\Type\FakeType; use CuyZ\Valinor\Type\Types\UnresolvableType; +use LogicException; use PHPUnit\Framework\TestCase; final class UnresolvableTypeTest extends TestCase { - public function test_unresolvable_type_has_correct_code(): void + public function test_call_unresolvable_type_accepts_throws_exception(): void { $type = new UnresolvableType('some-type', 'some message'); - self::assertSame(1679578492, $type->getCode()); - } - - public function test_call_unresolvable_type_accepts_throws_itself(): void - { - $type = new UnresolvableType('some-type', 'some message'); - - $this->expectExceptionObject($type); + $this->expectException(LogicException::class); $type->accepts('foo'); } - public function test_call_unresolvable_type_matches_throws_itself(): void + public function test_call_unresolvable_type_matches_throws_exception(): void { $type = new UnresolvableType('some-type', 'some message'); - $this->expectExceptionObject($type); + $this->expectException(LogicException::class); $type->matches(new FakeType()); }