diff --git a/library/Message/Formatter/NestedArrayFormatter.php b/library/Message/Formatter/NestedArrayFormatter.php index 936432c5e..250dc0719 100644 --- a/library/Message/Formatter/NestedArrayFormatter.php +++ b/library/Message/Formatter/NestedArrayFormatter.php @@ -13,8 +13,11 @@ use Respect\Validation\Message\Renderer; use Respect\Validation\Result; +use function array_reduce; use function count; use function current; +use function is_array; +use function is_numeric; final readonly class NestedArrayFormatter implements ArrayFormatter { @@ -25,31 +28,117 @@ */ public function format(Result $result, Renderer $renderer, array $templates): array { - if (count($result->children) === 0) { - return [ - $result->path->value ?? $result->id->value => $renderer->render($result, $templates), - ]; + if ($result->children === []) { + return [$this->getKey($result) => $renderer->render($result, $templates)]; } - $messages = []; - foreach ($result->children as $child) { - $key = $child->path->value ?? $child->id->value; - $messages[$key] = $this->format( - $child->withoutName(), + [$children, $hasString] = $this->prepareChildren($result->children); + + $messages = array_reduce( + $children, + fn(array $messages, array $item) => $this->appendMessage( + $messages, + $item['key'], + $item['child'], $renderer, $templates, - ); - if (count($messages[$key]) !== 1) { - continue; + $hasString, + ), + [], + ); + + if (count($messages) > 1) { + return ['__root__' => $renderer->render($result, $templates)] + $messages; + } + + return $messages; + } + + /** + * @param array $children + * + * @return array{0: array, 1: bool} + */ + private function prepareChildren(array $children): array + { + $mapped = []; + $hasString = false; + + foreach ($children as $child) { + $key = $this->getKey($child); + if (!is_numeric($key)) { + $hasString = true; } - $messages[$key] = current($messages[$key]); + $mapped[] = ['key' => $key, 'child' => $child]; } - if (count($messages) > 1) { - return ['__root__' => $renderer->render($result, $templates)] + $messages; + return [$mapped, $hasString]; + } + + /** + * @param array $messages + * @param array $templates + * + * @return array + */ + private function appendMessage( + array $messages, + string|int $key, + Result $child, + Renderer $renderer, + array $templates, + bool $hasString, + ): array { + if ($hasString && is_numeric($key)) { + $key = $child->id->value; } + $message = $this->renderChild($child, $renderer, $templates); + + if (!$hasString) { + $messages[] = $message; + + return $messages; + } + + if (isset($messages[$key])) { + if (!is_array($messages[$key])) { + $messages[$key] = [$messages[$key]]; + } + + $messages[$key][] = $message; + + return $messages; + } + + $messages[$key] = $message; + return $messages; } + + /** + * @param array> $templates + * + * @return array|string + */ + private function renderChild(Result $child, Renderer $renderer, array $templates): array|string + { + $formatted = $this->format( + $child->withoutName(), + $renderer, + $templates, + ); + + if (count($formatted) === 1) { + return current($formatted); + } + + return $formatted; + } + + private function getKey(Result $result): string|int + { + return $result->path->value ?? $result->id->value; + } } diff --git a/tests/feature/Issues/Issue1289Test.php b/tests/feature/Issues/Issue1289Test.php index d6f220ad0..ac8f517f2 100644 --- a/tests/feature/Issues/Issue1289Test.php +++ b/tests/feature/Issues/Issue1289Test.php @@ -60,7 +60,10 @@ ->and($messages)->toBe([ 0 => [ '__root__' => '`.0` must pass the rules', - 'default' => '`.0.default` must be a boolean', + 'default' => [ + '`.0.default` must be a string', + '`.0.default` must be a boolean', + ], 'description' => '`.0.description` must be a string value', ], ]), diff --git a/tests/feature/Issues/Issue1376Test.php b/tests/feature/Issues/Issue1376Test.php index cae865e76..fbd5190fe 100644 --- a/tests/feature/Issues/Issue1376Test.php +++ b/tests/feature/Issues/Issue1376Test.php @@ -29,7 +29,10 @@ '__root__' => '`stdClass { +$author="foo" }` must pass all the rules', 'title' => '`.title` must be present', 'description' => '`.description` must be present', - 'author' => 'The length of `.author` must be between 1 and 2', + 'author' => [ + '`.author` must be an integer', + 'The length of `.author` must be between 1 and 2', + ], 'user' => '`.user` must be present', ]), )); diff --git a/tests/feature/Validators/AttributesTest.php b/tests/feature/Validators/AttributesTest.php index b5627f8a0..aa03288a5 100644 --- a/tests/feature/Validators/AttributesTest.php +++ b/tests/feature/Validators/AttributesTest.php @@ -57,7 +57,10 @@ ->and($messages)->toBe([ '__root__' => '`Respect\Validation\Test\Stubs\WithAttributes { +$name="" +$birthdate="not a date" #$phone="not a phone number" + ... }` must pass the rules', 'name' => '`.name` must be defined', - 'birthdate' => 'For comparison with now, `.birthdate` must be a valid datetime', + 'birthdate' => [ + '`.birthdate` must be a valid date in the format "2005-12-30"', + 'For comparison with now, `.birthdate` must be a valid datetime', + ], 'phone' => '`.phone` must be a valid telephone number or must be null', 'email' => '`.email` must be a valid email address or must be null', ]), diff --git a/tests/feature/Validators/EachTest.php b/tests/feature/Validators/EachTest.php index 427f42abb..58cad5753 100644 --- a/tests/feature/Validators/EachTest.php +++ b/tests/feature/Validators/EachTest.php @@ -283,13 +283,21 @@ FULL_MESSAGE) ->and($messages)->toBe([ '__root__' => 'Each item in `[2, 4]` must be valid', - 0 => '`.0` must be an odd number', - 1 => '`.1` must be an odd number', + 0 => [ + '__root__' => '`.0` must pass all the rules', + 0 => '`.0` must be between 5 and 7', + 1 => '`.0` must be an odd number', + ], + 1 => [ + '__root__' => '`.1` must pass all the rules', + 0 => '`.1` must be between 5 and 7', + 1 => '`.1` must be an odd number', + ], ]), )); test('Multiple nested rules', catchAll( - fn() => v::each(v::arrayType()->key('my_int', v::intType()->odd()))->assert([['not_int' => 'wrong'], ['my_int' => 2], 'not an array']), + fn() => v::each(v::arrayType()->key('my_int', v::intType()->odd())->length(v::equals(1)))->assert([['not_int' => 'wrong'], ['my_int' => 2], 'not an array']), fn(string $message, string $fullMessage, array $messages) => expect() ->and($message)->toBe('`.0.my_int` must be present') ->and($fullMessage)->toBe(<<<'FULL_MESSAGE' @@ -301,6 +309,7 @@ - `.2` must pass all the rules - `.2` must be an array - `.2.my_int` must be present + - The length of `.2` must be equal to 1 FULL_MESSAGE) ->and($messages)->toBe([ '__root__' => 'Each item in `[["not_int": "wrong"], ["my_int": 2], "not an array"]` must be valid', @@ -308,8 +317,9 @@ 1 => '`.1.my_int` must be an odd number', 2 => [ '__root__' => '`.2` must pass all the rules', - 2 => '`.2` must be an array', + 'arrayType' => '`.2` must be an array', 'my_int' => '`.2.my_int` must be present', + 'lengthEquals' => 'The length of `.2` must be equal to 1', ], ]), )); diff --git a/tests/unit/Message/Formatter/NestedArrayFormatterTest.php b/tests/unit/Message/Formatter/NestedArrayFormatterTest.php index 1ba18d4c9..44dc599d7 100644 --- a/tests/unit/Message/Formatter/NestedArrayFormatterTest.php +++ b/tests/unit/Message/Formatter/NestedArrayFormatterTest.php @@ -13,6 +13,7 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use Respect\Validation\Message\StandardFormatter\ResultCreator; +use Respect\Validation\Path; use Respect\Validation\Result; use Respect\Validation\Test\Builders\ResultBuilder; use Respect\Validation\Test\Message\TestingMessageRenderer; @@ -37,7 +38,7 @@ public function itShouldFormatArrayMessage(Result $result, array $expected, arra self::assertSame($expected, $formatter->format($result, $renderer, $templates)); } - /** @return array, 2?: array}> */ + /** @return array, 2?: array}> */ public static function provideForArray(): array { return [ @@ -76,6 +77,65 @@ public static function provideForArray(): array '3rd' => '__3rd_original__', ], ], + 'with string key collision' => [ + (new ResultBuilder())->id('root')->template('root_msg') + ->children( + (new ResultBuilder())->id('c1')->template('msg1')->withPath(new Path('foo'))->build(), + (new ResultBuilder())->id('c2')->template('msg2')->withPath(new Path('foo'))->build(), + )->build(), + [ + 'foo' => ['msg1', 'msg2'], + ], + ], + 'with numeric key collision (list)' => [ + (new ResultBuilder())->id('root')->template('root_msg') + ->children( + (new ResultBuilder())->id('c1')->template('msg1')->withPath(new Path(0))->build(), + (new ResultBuilder())->id('c2')->template('msg2')->withPath(new Path(0))->build(), + )->build(), + [ + '__root__' => 'root_msg', + 0 => 'msg1', + 1 => 'msg2', + ], + ], + 'with mixed keys replacement' => [ + (new ResultBuilder())->id('root')->template('root_msg') + ->children( + (new ResultBuilder())->id('c1')->template('msg1')->withPath(new Path('foo'))->build(), + (new ResultBuilder())->id('c2')->template('msg2')->withPath(new Path(0))->build(), + )->build(), + [ + '__root__' => 'root_msg', + 'foo' => 'msg1', + 'c2' => 'msg2', + ], + ], + 'with mixed keys and ID collision' => [ + (new ResultBuilder())->id('root')->template('root_msg') + ->children( + (new ResultBuilder())->id('c1')->template('msg1')->withPath(new Path('foo'))->build(), + (new ResultBuilder())->id('sameId')->template('msg2')->withPath(new Path(0))->build(), + (new ResultBuilder())->id('sameId')->template('msg3')->withPath(new Path(1))->build(), + )->build(), + [ + '__root__' => 'root_msg', + 'foo' => 'msg1', + 'sameId' => ['msg2', 'msg3'], + ], + ], + 'with pure numeric keys' => [ + (new ResultBuilder())->id('root')->template('root_msg') + ->children( + (new ResultBuilder())->id('c1')->template('msg1')->withPath(new Path(10))->build(), + (new ResultBuilder())->id('c2')->template('msg2')->withPath(new Path(20))->build(), + )->build(), + [ + '__root__' => 'root_msg', + 0 => 'msg1', + 1 => 'msg2', + ], + ], ]; } }