From 93af1e73ea5311b47e9bb7e6811cf2eafbe6ac15 Mon Sep 17 00:00:00 2001 From: Ivan Kurnosov Date: Wed, 25 Sep 2024 14:39:10 +1200 Subject: [PATCH 1/3] Added different message to distinguish between two cases: a) the value at the path is of the wrong type b) the path does not exist . --- src/Psl/Type/Exception/AssertException.php | 32 ++++++++++++++++---- src/Psl/Type/Exception/CoercionException.php | 31 ++++++++++++++++--- src/Psl/Type/Internal/ShapeType.php | 4 +-- tests/unit/Type/ShapeTypeTest.php | 8 ++--- 4 files changed, 58 insertions(+), 17 deletions(-) diff --git a/src/Psl/Type/Exception/AssertException.php b/src/Psl/Type/Exception/AssertException.php index 19335c71..a5a31c8f 100644 --- a/src/Psl/Type/Exception/AssertException.php +++ b/src/Psl/Type/Exception/AssertException.php @@ -17,18 +17,28 @@ final class AssertException extends Exception /** * @param list $paths */ - private function __construct(string $actual, string $expected, array $paths = [], ?Throwable $previous = null) + private function __construct(?string $actual, string $expected, array $paths = [], ?Throwable $previous = null) { $first = $previous instanceof Exception ? $previous->getFirstFailingActualType() : $actual; - parent::__construct( - Str\format( + if ($first !== null) { + $message = Str\format( 'Expected "%s", got "%s"%s.', $expected, $first, - $paths ? ' at path "' . Str\join($paths, '.') . '"' : '' - ), - $actual, + $paths ? ' at path "' . Str\join($paths, '.') . '"' : '', + ); + } else { + $message = Str\format( + 'Expected "%s", received no value at path "%s".', + $expected, + Str\join($paths, '.'), + ); + } + + parent::__construct( + $message, + $actual ?? 'null', $paths, $previous ); @@ -51,4 +61,14 @@ public static function withValue( return new self(get_debug_type($value), $expected_type, Vec\filter_nulls($paths), $previous); } + + public static function withoutValue( + string $expected_type, + ?string $path = null, + ?Throwable $previous = null + ): self { + $paths = $previous instanceof Exception ? [$path, ...$previous->getPaths()] : [$path]; + + return new self(null, $expected_type, Vec\filter_nulls($paths), $previous); + } } diff --git a/src/Psl/Type/Exception/CoercionException.php b/src/Psl/Type/Exception/CoercionException.php index 3b8b5c3a..6c5f2275 100644 --- a/src/Psl/Type/Exception/CoercionException.php +++ b/src/Psl/Type/Exception/CoercionException.php @@ -17,19 +17,30 @@ final class CoercionException extends Exception /** * @param list $paths */ - private function __construct(string $actual, string $target, array $paths = [], ?Throwable $previous = null) + private function __construct(?string $actual, string $target, array $paths = [], ?Throwable $previous = null) { $first = $previous instanceof Exception ? $previous->getFirstFailingActualType() : $actual; - parent::__construct( - Str\format( + if ($first !== null) { + $message = Str\format( 'Could not coerce "%s" to type "%s"%s%s.', $first, $target, $paths ? ' at path "' . Str\join($paths, '.') . '"' : '', $previous && !$previous instanceof self ? ': ' . $previous->getMessage() : '', - ), - $actual, + ); + } else { + $message = Str\format( + 'Could not coerce to type "%s" at path "%s" as the value was not passed%s.', + $target, + Str\join($paths, '.'), + $previous && !$previous instanceof self ? ': ' . $previous->getMessage() : '', + ); + } + + parent::__construct( + $message, + $actual ?? 'null', $paths, $previous ); @@ -52,4 +63,14 @@ public static function withValue( return new self(get_debug_type($value), $target, Vec\filter_nulls($paths), $previous); } + + public static function withoutValue( + string $target, + ?string $path = null, + ?Throwable $previous = null + ): self { + $paths = $previous instanceof Exception ? [$path, ...$previous->getPaths()] : [$path]; + + return new self(null, $target, Vec\filter_nulls($paths), $previous); + } } diff --git a/src/Psl/Type/Internal/ShapeType.php b/src/Psl/Type/Internal/ShapeType.php index fd24039a..ead79632 100644 --- a/src/Psl/Type/Internal/ShapeType.php +++ b/src/Psl/Type/Internal/ShapeType.php @@ -144,7 +144,7 @@ private function coerceIterable(mixed $value): array continue; } - throw CoercionException::withValue(null, $this->toString(), PathExpression::path($element)); + throw CoercionException::withoutValue($this->toString(), PathExpression::path($element)); } } catch (CoercionException $e) { throw match (true) { @@ -196,7 +196,7 @@ public function assert(mixed $value): array continue; } - throw AssertException::withValue(null, $this->toString(), PathExpression::path($element)); + throw AssertException::withoutValue($this->toString(), PathExpression::path($element)); } } catch (AssertException $e) { throw match (true) { diff --git a/tests/unit/Type/ShapeTypeTest.php b/tests/unit/Type/ShapeTypeTest.php index ac6a2339..e7c1512f 100644 --- a/tests/unit/Type/ShapeTypeTest.php +++ b/tests/unit/Type/ShapeTypeTest.php @@ -216,7 +216,7 @@ public static function provideAssertExceptionExpectations(): iterable 'name' => Type\string(), ]), [], - 'Expected "array{\'name\': string}", got "null" at path "name".' + 'Expected "array{\'name\': string}", received no value at path "name".' ]; yield 'invalid key' => [ Type\shape([ @@ -247,7 +247,7 @@ public static function provideCoerceExceptionExpectations(): iterable 'name' => Type\string(), ]), [], - 'Could not coerce "null" to type "array{\'name\': string}" at path "name".' + 'Could not coerce to type "array{\'name\': string}" at path "name" as the value was not passed.' ]; yield 'invalid key' => [ Type\shape([ @@ -295,7 +295,7 @@ public static function provideCoerceExceptionExpectations(): iterable (static function () { yield null => 'nope'; })(), - 'Could not coerce "null" to type "array{\'id\': int}" at path "id".' + 'Could not coerce to type "array{\'id\': int}" at path "id" as the value was not passed.' ]; yield 'iterator yielding object key' => [ Type\shape([ @@ -305,7 +305,7 @@ public static function provideCoerceExceptionExpectations(): iterable yield (new class () { }) => 'nope'; })(), - 'Could not coerce "null" to type "array{\'id\': int}" at path "id".' + 'Could not coerce to type "array{\'id\': int}" at path "id" as the value was not passed.' ]; } From 6b02ca3c6b14c83662e2336e8ae8f34ecb0495f0 Mon Sep 17 00:00:00 2001 From: Ivan Kurnosov Date: Tue, 29 Oct 2024 09:58:21 +1300 Subject: [PATCH 2/3] Moved exception message generation to named constructors in AssertException and CoercionException --- src/Psl/Type/Exception/AssertException.php | 42 +++++++++--------- src/Psl/Type/Exception/CoercionException.php | 46 ++++++++++---------- 2 files changed, 42 insertions(+), 46 deletions(-) diff --git a/src/Psl/Type/Exception/AssertException.php b/src/Psl/Type/Exception/AssertException.php index a5a31c8f..336757e7 100644 --- a/src/Psl/Type/Exception/AssertException.php +++ b/src/Psl/Type/Exception/AssertException.php @@ -17,25 +17,8 @@ final class AssertException extends Exception /** * @param list $paths */ - private function __construct(?string $actual, string $expected, array $paths = [], ?Throwable $previous = null) + private function __construct(?string $actual, string $expected, string $message, array $paths = [], ?Throwable $previous = null) { - $first = $previous instanceof Exception ? $previous->getFirstFailingActualType() : $actual; - - if ($first !== null) { - $message = Str\format( - 'Expected "%s", got "%s"%s.', - $expected, - $first, - $paths ? ' at path "' . Str\join($paths, '.') . '"' : '', - ); - } else { - $message = Str\format( - 'Expected "%s", received no value at path "%s".', - $expected, - Str\join($paths, '.'), - ); - } - parent::__construct( $message, $actual ?? 'null', @@ -57,9 +40,18 @@ public static function withValue( ?string $path = null, ?Throwable $previous = null ): self { - $paths = $previous instanceof Exception ? [$path, ...$previous->getPaths()] : [$path]; + $paths = Vec\filter_nulls($previous instanceof Exception ? [$path, ...$previous->getPaths()] : [$path]); + $actual = get_debug_type($value); + $first = $previous instanceof Exception ? $previous->getFirstFailingActualType() : $actual; - return new self(get_debug_type($value), $expected_type, Vec\filter_nulls($paths), $previous); + $message = Str\format( + 'Expected "%s", got "%s"%s.', + $expected_type, + $first, + $paths ? ' at path "' . Str\join($paths, '.') . '"' : '', + ); + + return new self($actual, $expected_type, $message, $paths, $previous); } public static function withoutValue( @@ -67,8 +59,14 @@ public static function withoutValue( ?string $path = null, ?Throwable $previous = null ): self { - $paths = $previous instanceof Exception ? [$path, ...$previous->getPaths()] : [$path]; + $paths = Vec\filter_nulls($previous instanceof Exception ? [$path, ...$previous->getPaths()] : [$path]); + + $message = Str\format( + 'Expected "%s", received no value at path "%s".', + $expected_type, + Str\join($paths, '.'), + ); - return new self(null, $expected_type, Vec\filter_nulls($paths), $previous); + return new self(null, $expected_type, $message, $paths, $previous); } } diff --git a/src/Psl/Type/Exception/CoercionException.php b/src/Psl/Type/Exception/CoercionException.php index 6c5f2275..cf2286cb 100644 --- a/src/Psl/Type/Exception/CoercionException.php +++ b/src/Psl/Type/Exception/CoercionException.php @@ -17,27 +17,8 @@ final class CoercionException extends Exception /** * @param list $paths */ - private function __construct(?string $actual, string $target, array $paths = [], ?Throwable $previous = null) + private function __construct(?string $actual, string $target, string $message, array $paths = [], ?Throwable $previous = null) { - $first = $previous instanceof Exception ? $previous->getFirstFailingActualType() : $actual; - - if ($first !== null) { - $message = Str\format( - 'Could not coerce "%s" to type "%s"%s%s.', - $first, - $target, - $paths ? ' at path "' . Str\join($paths, '.') . '"' : '', - $previous && !$previous instanceof self ? ': ' . $previous->getMessage() : '', - ); - } else { - $message = Str\format( - 'Could not coerce to type "%s" at path "%s" as the value was not passed%s.', - $target, - Str\join($paths, '.'), - $previous && !$previous instanceof self ? ': ' . $previous->getMessage() : '', - ); - } - parent::__construct( $message, $actual ?? 'null', @@ -59,9 +40,19 @@ public static function withValue( ?string $path = null, ?Throwable $previous = null ): self { - $paths = $previous instanceof Exception ? [$path, ...$previous->getPaths()] : [$path]; + $paths = Vec\filter_nulls($previous instanceof Exception ? [$path, ...$previous->getPaths()] : [$path]); + $actual = get_debug_type($value); + $first = $previous instanceof Exception ? $previous->getFirstFailingActualType() : $actual; - return new self(get_debug_type($value), $target, Vec\filter_nulls($paths), $previous); + $message = Str\format( + 'Could not coerce "%s" to type "%s"%s%s.', + $first, + $target, + $paths ? ' at path "' . Str\join($paths, '.') . '"' : '', + $previous && !$previous instanceof self ? ': ' . $previous->getMessage() : '', + ); + + return new self($actual, $target, $message, $paths, $previous); } public static function withoutValue( @@ -69,8 +60,15 @@ public static function withoutValue( ?string $path = null, ?Throwable $previous = null ): self { - $paths = $previous instanceof Exception ? [$path, ...$previous->getPaths()] : [$path]; + $paths = Vec\filter_nulls($previous instanceof Exception ? [$path, ...$previous->getPaths()] : [$path]); + + $message = Str\format( + 'Could not coerce to type "%s" at path "%s" as the value was not passed%s.', + $target, + Str\join($paths, '.'), + $previous && !$previous instanceof self ? ': ' . $previous->getMessage() : '', + ); - return new self(null, $target, Vec\filter_nulls($paths), $previous); + return new self(null, $target, $message, $paths, $previous); } } From 891509619c6dc57b18044661cdf664d53edf0d17 Mon Sep 17 00:00:00 2001 From: Ivan Kurnosov Date: Thu, 7 Nov 2024 09:54:38 +1300 Subject: [PATCH 3/3] Fix nested missing values --- src/Psl/Type/Exception/CoercionException.php | 21 +++++++++++++++++--- src/Psl/Type/Internal/ShapeType.php | 2 +- tests/unit/Type/ShapeTypeTest.php | 11 ++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/Psl/Type/Exception/CoercionException.php b/src/Psl/Type/Exception/CoercionException.php index cf2286cb..128b10fc 100644 --- a/src/Psl/Type/Exception/CoercionException.php +++ b/src/Psl/Type/Exception/CoercionException.php @@ -14,10 +14,12 @@ final class CoercionException extends Exception { private string $target; + private bool $withValue; + /** * @param list $paths */ - private function __construct(?string $actual, string $target, string $message, array $paths = [], ?Throwable $previous = null) + private function __construct(?string $actual, string $target, string $message, bool $withValue, array $paths = [], ?Throwable $previous = null) { parent::__construct( $message, @@ -27,6 +29,7 @@ private function __construct(?string $actual, string $target, string $message, a ); $this->target = $target; + $this->withValue = $withValue; } public function getTargetType(): string @@ -52,7 +55,7 @@ public static function withValue( $previous && !$previous instanceof self ? ': ' . $previous->getMessage() : '', ); - return new self($actual, $target, $message, $paths, $previous); + return new self($actual, $target, $message, true, $paths, $previous); } public static function withoutValue( @@ -69,6 +72,18 @@ public static function withoutValue( $previous && !$previous instanceof self ? ': ' . $previous->getMessage() : '', ); - return new self(null, $target, $message, $paths, $previous); + return new self(null, $target, $message, false, $paths, $previous); + } + + public function wrap( + mixed $value, + string $target, + ?string $path = null + ): self { + if ($this->withValue) { + return self::withValue($value, $target, $path, $this); + } + + return self::withoutValue($target, $path, $this); } } diff --git a/src/Psl/Type/Internal/ShapeType.php b/src/Psl/Type/Internal/ShapeType.php index ead79632..6116861a 100644 --- a/src/Psl/Type/Internal/ShapeType.php +++ b/src/Psl/Type/Internal/ShapeType.php @@ -148,7 +148,7 @@ private function coerceIterable(mixed $value): array } } catch (CoercionException $e) { throw match (true) { - $element_value_found => CoercionException::withValue($array[$element] ?? null, $this->toString(), PathExpression::path($element), $e), + $element_value_found => $e->wrap($array[$element] ?? null, $this->toString(), PathExpression::path($element)), default => $e }; } diff --git a/tests/unit/Type/ShapeTypeTest.php b/tests/unit/Type/ShapeTypeTest.php index e7c1512f..10cf077d 100644 --- a/tests/unit/Type/ShapeTypeTest.php +++ b/tests/unit/Type/ShapeTypeTest.php @@ -249,6 +249,17 @@ public static function provideCoerceExceptionExpectations(): iterable [], 'Could not coerce to type "array{\'name\': string}" at path "name" as the value was not passed.' ]; + yield 'missing nested key' => [ + Type\shape([ + 'a' => Type\shape([ + 'b' => Type\shape([ + 'c' => Type\mixed(), + ]), + ]), + ]), + ['a' => ['b' => []]], + 'Could not coerce to type "array{\'a\': array{\'b\': array{\'c\': mixed}}}" at path "a.b.c" as the value was not passed.', + ]; yield 'invalid key' => [ Type\shape([ 'name' => Type\string(),