Skip to content

Commit

Permalink
fix: remove exception inheritance from UnresolvableType
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
romm committed Jul 7, 2023
1 parent d358e83 commit 80e9681
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 30 deletions.
2 changes: 1 addition & 1 deletion src/Definition/Repository/Cache/Compiler/TypeCompiler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions src/Mapper/Object/Exception/InvalidConstructorReturnType.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
29 changes: 18 additions & 11 deletions src/Type/Types/UnresolvableType.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
16 changes: 5 additions & 11 deletions tests/Unit/Type/Types/UnresolvableTypeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down

0 comments on commit 80e9681

Please sign in to comment.