Skip to content

Commit

Permalink
v2
Browse files Browse the repository at this point in the history
  • Loading branch information
henriquemoody committed Dec 27, 2024
1 parent ae48959 commit 50cfd46
Show file tree
Hide file tree
Showing 12 changed files with 114 additions and 75 deletions.
41 changes: 29 additions & 12 deletions library/Message/StandardFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use function array_filter;
use function array_key_exists;
use function array_map;
use function array_reduce;
use function array_values;
use function count;
Expand Down Expand Up @@ -42,11 +43,7 @@ public function main(Result $result, array $templates, Translator $translator):
$selectedTemplates = $this->selectTemplates($result, $templates);
if (!$this->isFinalTemplate($result, $selectedTemplates)) {
foreach ($this->extractDeduplicatedChildren($result) as $child) {
if ($result->path !== null && $child->path !== null && $child->path !== $result->path) {
$child = $child->withPath($result->path);
} elseif ($result->path !== null && $child->path === null) {
$child = $child->withPath($result->path);
}
$child = $this->resultWithPath($result, $child);

return $this->main($child, $selectedTemplates, $translator);
}
Expand All @@ -63,7 +60,6 @@ public function full(
array $templates,
Translator $translator,
int $depth = 0,
?Result $parent = null,
Result ...$siblings
): string {
$selectedTemplates = $this->selectTemplates($result, $templates);
Expand All @@ -75,20 +71,25 @@ public function full(
$rendered .= sprintf(
'%s- %s' . PHP_EOL,
$indentation,
$this->renderer->render($this->getTemplated($result, $selectedTemplates), $translator),
$this->renderer->render(
$this->getTemplated($depth > 0 ? $result->withDeepestPath() : $result, $selectedTemplates),
$translator
),
);
$depth++;
}

if (!$isFinalTemplate) {
$results = $this->extractDeduplicatedChildren($result);
$results = array_map(
fn(Result $child) => $this->resultWithPath($result, $child),
$this->extractDeduplicatedChildren($result)
);
foreach ($results as $child) {
$rendered .= $this->full(
$child,
$selectedTemplates,
$translator,
$depth,
$result,
...array_filter($results, static fn (Result $sibling) => $sibling !== $child)
);
$rendered .= PHP_EOL;
Expand All @@ -110,7 +111,7 @@ public function array(Result $result, array $templates, Translator $translator):
if (count($deduplicatedChildren) === 0 || $this->isFinalTemplate($result, $selectedTemplates)) {
return [
$result->path ?? $result->id => $this->renderer->render(
$this->getTemplated($result, $selectedTemplates),
$this->getTemplated($result->withDeepestPath(), $selectedTemplates),
$translator
),
];
Expand All @@ -120,7 +121,7 @@ public function array(Result $result, array $templates, Translator $translator):
foreach ($deduplicatedChildren as $child) {
$key = $child->path ?? $child->id;
$messages[$key] = $this->array(
$child,
$this->resultWithPath($result, $child),
$this->selectTemplates($child, $selectedTemplates),
$translator
);
Expand All @@ -133,7 +134,10 @@ public function array(Result $result, array $templates, Translator $translator):

if (count($messages) > 1) {
$self = [
'__root__' => $this->renderer->render($this->getTemplated($result, $selectedTemplates), $translator),
'__root__' => $this->renderer->render(
$this->getTemplated($result->withDeepestPath(), $selectedTemplates),
$translator
),
];

return $self + $messages;
Expand All @@ -142,6 +146,19 @@ public function array(Result $result, array $templates, Translator $translator):
return $messages;
}

public function resultWithPath(Result $parent, Result $child): Result
{
if ($parent->path !== null && $child->path !== null && $child->path !== $parent->path) {
return $child->withPath($parent->path);
}

if ($parent->path !== null && $child->path === null) {
return $child->withPath($parent->path);
}

return $child;
}

private function isAlwaysVisible(Result $result, Result ...$siblings): bool
{
if ($result->isValid) {
Expand Down
11 changes: 6 additions & 5 deletions library/Message/StandardRenderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,12 @@ public function __construct(

public function render(Result $result, Translator $translator, ?string $template = null): string
{
$parameters = $result->parameters + [
'path' => $result->path !== null ? Quoted::fromPath($result->path) : null,
'input' => $result->input,
];
$parameters['name'] ??= $result->name ?? $parameters['path'] ?? $this->placeholder('input', $result->input, $translator);
$parameters = $result->parameters;
$parameters['path'] = $result->path !== null ? Quoted::fromPath($result->path) : null;
$parameters['input'] = $result->input;

$builtName = $result->name ?? $parameters['path'] ?? $this->placeholder('input', $result->input, $translator);
$parameters['name'] ??= $builtName;

$rendered = (string) preg_replace_callback(
'/{{(\w+)(\|([^}]+))?}}/',
Expand Down
22 changes: 14 additions & 8 deletions library/Result.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@
namespace Respect\Validation;

use Respect\Validation\Rules\Core\Nameable;
use Respect\Validation\Rules\Core\Renameable;

use function array_filter;
use function array_map;
use function count;
use function end;
use function explode;
use function lcfirst;
use function preg_match;
use function strrchr;
Expand Down Expand Up @@ -112,17 +113,22 @@ public function withIdFrom(Rule $rule): self

public function withPath(string|int $path): self
{
if ($this->path === $path) {
return $this->clone(
adjacent: $this->adjacent?->withPath($path),
path: $this->path === null ? $path : $path . '.' . $this->path,
);
}

public function withDeepestPath(): self
{
$paths = explode('.', (string) $this->path);
if (count($paths) === 1) {
return $this;
}

return $this->clone(
adjacent: $this->adjacent?->withPath($path),
path: $this->path === null ? $path : $path . '.' . $this->path,
// children: array_map(
// static fn (Result $child) => $child->path === null ? $child->withPath($child->name ?? $path) : $child,
// $this->children
// ),
adjacent: $this->adjacent?->withPath(end($paths)),
path: end($paths),
);
}

Expand Down
1 change: 0 additions & 1 deletion library/Rules/KeyExists.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use Respect\Validation\Message\Template;
use Respect\Validation\Result;
use Respect\Validation\Rules\Core\KeyRelated;
use Respect\Validation\Rules\Core\Renameable;
use Respect\Validation\Rules\Core\Standard;

use function array_key_exists;
Expand Down
1 change: 0 additions & 1 deletion library/Rules/PropertyExists.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
use ReflectionObject;
use Respect\Validation\Message\Template;
use Respect\Validation\Result;
use Respect\Validation\Rules\Core\Renameable;
use Respect\Validation\Rules\Core\Standard;

use function is_object;
Expand Down
8 changes: 4 additions & 4 deletions tests/feature/Issues/Issue1289Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,17 @@
<<<'FULL_MESSAGE'
- `.0` must pass the rules
- `.default` must pass one of the rules
- 2 must be a string
- 2 must be a boolean
- `.default` must be a string
- `.default` must be a boolean
- `.description` must be a string value
FULL_MESSAGE,
[
0 => [
'__root__' => '`.0` must pass the rules',
'default' => [
'__root__' => '`.default` must pass one of the rules',
'stringType' => '2 must be a string',
'boolType' => '2 must be a boolean',
'stringType' => '`.default` must be a string',
'boolType' => '`.default` must be a boolean',
],
'description' => '`.description` must be a string value',
],
Expand Down
12 changes: 6 additions & 6 deletions tests/feature/Issues/Issue1334Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,22 @@ function (): void {
- `.0` must pass the rules
- `.street` must be present
- `.other` must pass the rules
- 123 must be a string or must be null
- `.other` must be a string or must be null
- `.1` must pass the rules
- "" must not be empty
- `.street` must not be empty
- `.2` must pass the rules
- 123 must be a string
- `.street` must be a string
FULL_MESSAGE,
[
'each' => [
'__root__' => 'Each item in `[["region": "Oregon", "country": "USA", "other": 123], ["street": "", "region": "Oregon", "country": "USA"], ["s ... ]` must be valid',
0 => [
'__root__' => '`.0` must pass the rules',
'street' => '`.street` must be present',
'other' => '123 must be a string or must be null',
'other' => '`.other` must be a string or must be null',
],
1 => '"" must not be empty',
2 => '123 must be a string',
1 => '`.street` must not be empty',
2 => '`.street` must be a string',
],
],
));
8 changes: 4 additions & 4 deletions tests/feature/Issues/Issue1376Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
- `.title` must be present
- `.description` must be present
- `.author` must pass all the rules
- "foo" must be an integer
- The length of "foo" must be between 1 and 2
- `.author` must be an integer
- The length of `.author` must be between 1 and 2
- `.user` must be present
FULL_MESSAGE,
[
Expand All @@ -30,8 +30,8 @@
'description' => '`.description` must be present',
'author' => [
'__root__' => '`.author` must pass all the rules',
'intType' => '"foo" must be an integer',
'lengthBetween' => 'The length of "foo" must be between 1 and 2',
'intType' => '`.author` must be an integer',
'lengthBetween' => 'The length of `.author` must be between 1 and 2',
],
'user' => '`.user` must be present',
],
Expand Down
59 changes: 39 additions & 20 deletions tests/feature/Issues/Issue1469Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,35 @@

test('https://github.com/Respect/Validation/issues/1469', expectAll(
function (): void {
$data = [
'order_items' => [
[
'product_title' => 'test',
'quantity' => 'test',
v::create()
->arrayVal()
->keySet(
v::key(
'order_items',
v::create()
->arrayVal()
->each(
v::keySet(
v::key('product_title', v::stringVal()->notEmpty()),
v::key('quantity', v::intVal()->notEmpty()),
)
)
->notEmpty()
),
)
->assert([
'order_items' => [
[
'product_title' => 'test',
'quantity' => 'test',
],
[
'product_title2' => 'test',
],
],
[
'product_title2' => 'test',
],
],
];

v::arrayVal()->keySet(
v::key('order_items', v::arrayVal()->each(v::keySet(
v::key('product_title', v::stringVal()->notEmpty()),
v::key('quantity', v::intVal()->notEmpty()),
))->notEmpty()),
)->assert($data);
]);
},
'`.0.quantity` must be an integer value',
'`.order_items.0.quantity` must be an integer value',
<<<'FULL_MESSAGE'
- Each item in `.order_items` must be valid
- `.0` validation failed
Expand All @@ -45,9 +54,19 @@ function (): void {
1 => [
'__root__' => '`.order_items` contains both missing and extra keys',
'product_title' => '`.product_title` must be present',
'quantity' => '`.quantity must` be present',
'quantity' => '`.quantity` must be present',
'product_title2' => '`.product_title2` must not be present',
],
],
],
));
))->skip(<<<TEST_SKIP
This test is skipped because the issue is not fixed yet.
When changing the path of a `Result` we don't change the path of its children. I took this approach because we don't
want to duplicated `.path1.path1` in the message (`.parent.child`).
However, that means that when one has a rule/result in-between paths (`KeySet` in this case), the children will be
totally unaware of the path of the parent. Although that's partially the intended, it causes problems like these.
I'm not sure how to fix this yet.
TEST_SKIP);
4 changes: 2 additions & 2 deletions tests/feature/Rules/AttributesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@
- `.phone` must be a valid telephone number or must be null
FULL_MESSAGE,
[
'__root__' => '`Respect\Validation\Test\Stubs\WithAttributes{ +$name="" +$email="not an email" +$birthdate="not a date" +$phone ... }` must pass all the rules',
'__root__' => '`Respect\Validation\Test\Stubs\WithAttributes { +$name="" +$email="not an email" +$birthdate="not a date" +$phone ... }` must pass all the rules',
'name' => '`.name` must not be empty',
'email' => '`.email` must be a valid email address',
'birthdate' => [
'__root__' => 'birthdate must pass all the rules',
'__root__' => '`.birthdate` must pass all the rules',
'date' => '`.birthdate` must be a valid date in the format "2005-12-30"',
'dateTimeDiffLessThanOrEqual' => 'For comparison with now, `.birthdate` must be a valid datetime',
],
Expand Down
18 changes: 9 additions & 9 deletions tests/feature/Rules/EachTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -256,13 +256,13 @@
'__root__' => 'Each item in `[2, 4]` must be valid',
0 => [
'__root__' => '`.0` must pass all the rules',
'between' => '2 must be between 5 and 7',
'odd' => '2 must be an odd number',
'between' => '`.0` must be between 5 and 7',
'odd' => '`.0` must be an odd number',
],
1 => [
'__root__' => '`.1` must pass all the rules',
'between' => '4 must be between 5 and 7',
'odd' => '4 must be an odd number',
'between' => '`.1` must be between 5 and 7',
'odd' => '`.1` must be an odd number',
],
],
));
Expand All @@ -282,12 +282,12 @@
FULL_MESSAGE,
[
'__root__' => 'Each item in `[["not_int": "wrong"], ["my_int": 2], "not an array"]` must be valid',
0 => 'my_int must be present',
1 => 'my_int must be an odd number',
0 => '`.my_int` must be present',
1 => '`.my_int` must be an odd number',
2 => [
'__root__' => '"not an array" must pass all the rules',
'arrayType' => '"not an array" must be an array',
'my_int' => 'my_int must be present',
'__root__' => '`.2` must pass all the rules',
'arrayType' => '`.2` must be an array',
'my_int' => '`.my_int` must be present',
],
],
));
Loading

0 comments on commit 50cfd46

Please sign in to comment.