Skip to content

Commit 7f4a4c2

Browse files
committed
Fix misplaced Outer/Inner names when using named() on Each
The Each validator was using "Outer" names (the name applied to the parent) in child results and vice-versa. This commit fixes it, and also streamlines the Result class, introducing helper `mapChildren` and `mapAdjacent` methods and removing unecessary recursive array_maps. Named children now also display their names, allowing users to create more meaningful messages that do not spam nested `.0`...`.0`...etc numeric keys which could be confusing. If the user does not name them, the previous behavior is kept.
1 parent 0381718 commit 7f4a4c2

File tree

6 files changed

+66
-76
lines changed

6 files changed

+66
-76
lines changed

src/Message/Formatter/NestedArrayFormatter.php

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ public function format(Result $result, Renderer $renderer, array $templates): ar
4545
$renderer,
4646
$templates,
4747
$hasString,
48+
$result,
49+
isParentVisible: count($children) > 1,
4850
),
4951
[],
5052
);
@@ -91,12 +93,14 @@ private function appendMessage(
9193
Renderer $renderer,
9294
array $templates,
9395
bool $hasString,
96+
Result $parent,
97+
bool $isParentVisible,
9498
): array {
9599
if ($hasString && is_numeric($key)) {
96100
$key = $child->id->value;
97101
}
98102

99-
$message = $this->renderChild($child, $renderer, $templates);
103+
$message = $this->renderChild($child, $renderer, $templates, $parent, $isParentVisible);
100104

101105
if (!$hasString) {
102106
$messages[] = $message;
@@ -124,10 +128,15 @@ private function appendMessage(
124128
*
125129
* @return array<string>|string
126130
*/
127-
private function renderChild(Result $child, Renderer $renderer, array $templates): array|string
128-
{
131+
private function renderChild(
132+
Result $child,
133+
Renderer $renderer,
134+
array $templates,
135+
Result $parent,
136+
bool $isParentVisible,
137+
): array|string {
129138
$formatted = $this->format(
130-
$child->withoutName(),
139+
$isParentVisible && $child->name === $parent->name ? $child->withoutName() : $child,
131140
$renderer,
132141
$templates,
133142
);

src/Result.php

Lines changed: 21 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public function asAdjacentOf(Result $result, string $prefix): Result
7777
if ($this->allowsAdjacent()) {
7878
return clone ($result, [
7979
'id' => $this->id->withPrefix($prefix),
80-
'adjacent' => $this->withInput($result->input),
80+
'adjacent' => clone($this, ['input' => $result->input]),
8181
]);
8282
}
8383

@@ -112,12 +112,17 @@ public function withIdFrom(Validator $validator): self
112112
return clone($this, ['id' => Id::fromValidator($validator)]);
113113
}
114114

115-
public function withPath(Path $path): self
115+
public function withPrecedentName(bool $hasPrecedentName): self
116116
{
117-
if ($this->path === $path) {
118-
return $this;
119-
}
117+
return clone($this, [
118+
'hasPrecedentName' => $hasPrecedentName,
119+
'adjacent' => $this->adjacent?->withPrecedentName($hasPrecedentName),
120+
'children' => $this->mapChildren(static fn(Result $r) => $r->withPrecedentName($hasPrecedentName)),
121+
]);
122+
}
120123

124+
public function withPath(Path $path): self
125+
{
121126
if ($this->path !== null) {
122127
$this->path->parent = $path;
123128

@@ -127,11 +132,7 @@ public function withPath(Path $path): self
127132
return clone($this, [
128133
'path' => $path,
129134
'adjacent' => $this->adjacent?->withPath($path),
130-
'hasPrecedentName' => $this->name !== null,
131-
'children' => array_map(
132-
static fn(Result $child) => $child->withPath($path),
133-
$this->children,
134-
),
135+
'children' => $this->mapChildren(static fn(Result $r) => $r->withPath($path)),
135136
]);
136137
}
137138

@@ -144,10 +145,7 @@ public function withoutName(): self
144145
return clone ($this, [
145146
'name' => null,
146147
'adjacent' => $this->adjacent?->withoutName(),
147-
'children' => array_map(
148-
fn(Result $child) => $child->name === $this->name ? $child->withoutName() : $child,
149-
$this->children,
150-
),
148+
'children' => $this->mapChildren(fn(Result $r) => $r->name === $this->name ? $r->withoutName() : $r),
151149
]);
152150
}
153151

@@ -164,11 +162,9 @@ public function withName(Name $name): self
164162

165163
return clone($this, [
166164
'name' => $name,
165+
'hasPrecedentName' => $this->path === null,
167166
'adjacent' => $this->adjacent?->withName($name),
168-
'children' => array_map(
169-
static fn(Result $child) => $child->withName($name),
170-
$this->children,
171-
),
167+
'children' => $this->mapChildren(static fn(Result $r) => $r->name === null ? $r->withName($name) : $r),
172168
]);
173169
}
174170

@@ -180,25 +176,7 @@ public function withNameFrom(Validator $validator): self
180176

181177
return clone($this, [
182178
'name' => $this->name ?? $validator->getName(),
183-
'hasPrecedentName' => true,
184179
'adjacent' => $this->adjacent?->withNameFrom($validator),
185-
'children' => array_map(
186-
static fn(Result $child) => $child->withNameFrom($validator),
187-
$this->children,
188-
),
189-
]);
190-
}
191-
192-
public function withInput(mixed $input): self
193-
{
194-
$currentInput = $this->input;
195-
196-
return clone($this, [
197-
'input' => $input,
198-
'children' => array_map(
199-
static fn(Result $child) => $child->input === $currentInput ? $child->withInput($input) : $child,
200-
$this->children,
201-
),
202180
]);
203181
}
204182

@@ -212,7 +190,6 @@ public function withToggledValidation(): self
212190
return clone($this, [
213191
'hasPassed' => !$this->hasPassed,
214192
'adjacent' => $this->adjacent?->withToggledValidation(),
215-
'children' => array_map(static fn(Result $child) => $child->withToggledValidation(), $this->children),
216193
]);
217194
}
218195

@@ -222,10 +199,7 @@ public function withToggledModeAndValidation(): self
222199
'hasPassed' => !$this->hasPassed,
223200
'hasInvertedMode' => !$this->hasInvertedMode,
224201
'adjacent' => $this->adjacent?->withToggledModeAndValidation(),
225-
'children' => array_map(
226-
static fn(Result $child) => $child->withToggledModeAndValidation(),
227-
$this->children,
228-
),
202+
'children' => $this->mapChildren(static fn(Result $r) => $r->withToggledModeAndValidation()),
229203
]);
230204
}
231205

@@ -247,4 +221,10 @@ public function allowsAdjacent(): bool
247221

248222
return count($childrenThatAllowAdjacent) === 1;
249223
}
224+
225+
/** @return array<Result> */
226+
private function mapChildren(callable $callback): array
227+
{
228+
return $this->children === [] ? [] : array_map($callback, $this->children);
229+
}
250230
}

src/Validators/Core/FilteredNonEmptyArray.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public function evaluate(mixed $input): Result
2828
{
2929
$iterableResult = (new IterableType())->evaluate($input);
3030
if (!$iterableResult->hasPassed) {
31-
return $iterableResult->withIdFrom($this)->withNameFrom($this->validator);
31+
return $iterableResult->withIdFrom($this);
3232
}
3333

3434
$array = $this->toArray($input);
@@ -38,7 +38,7 @@ public function evaluate(mixed $input): Result
3838
);
3939
$result = $validator->evaluate($array);
4040
if (!$result->hasPassed) {
41-
return $result->withIdFrom($this)->withNameFrom($this->validator);
41+
return $result->withIdFrom($this);
4242
}
4343

4444
// @phpstan-ignore-next-line

src/Validators/Each.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ protected function evaluateNonEmptyArray(array $input): Result
3737
{
3838
$children = [];
3939
foreach ($input as $key => $value) {
40-
$children[] = $this->validator->evaluate($value)->withPath(new Path($key));
40+
$children[] = $this->validator->evaluate($value)->withPath(new Path($key))->withPrecedentName(false);
4141
}
4242

4343
$hasPassed = array_reduce(
@@ -46,6 +46,6 @@ protected function evaluateNonEmptyArray(array $input): Result
4646
true,
4747
);
4848

49-
return Result::of($hasPassed, $input, $this)->withChildren(...$children)->withNameFrom($this->validator);
49+
return Result::of($hasPassed, $input, $this)->withChildren(...$children);
5050
}
5151
}

src/Validators/Key.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public function evaluate(mixed $input): Result
4444
return $keyExistsResult->withNameFrom($this->validator);
4545
}
4646

47-
return $this->validator->evaluate($input[$this->key])->withPath($keyExistsResult->path ?? new Path($this->key));
47+
return $this->validator->evaluate($input[$this->key])
48+
->withPath($keyExistsResult->path ?? new Path($this->key));
4849
}
4950
}

tests/feature/Validators/EachTest.php

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -65,52 +65,52 @@
6565
test('With name, non-iterable', catchAll(
6666
fn() => v::each(v::named('Wrapped', v::intType()))->assert(null),
6767
fn(string $message, string $fullMessage, array $messages) => expect()
68-
->and($message)->toBe('Wrapped must be iterable')
69-
->and($fullMessage)->toBe('- Wrapped must be iterable')
70-
->and($messages)->toBe(['each' => 'Wrapped must be iterable']),
68+
->and($message)->toBe('`null` must be iterable')
69+
->and($fullMessage)->toBe('- `null` must be iterable')
70+
->and($messages)->toBe(['each' => '`null` must be iterable']),
7171
));
7272

7373
test('With name, empty', catchAll(
7474
fn() => v::each(v::named('Wrapped', v::intType()))->assert([]),
7575
fn(string $message, string $fullMessage, array $messages) => expect()
76-
->and($message)->toBe('The length of Wrapped must be greater than 0')
77-
->and($fullMessage)->toBe('- The length of Wrapped must be greater than 0')
78-
->and($messages)->toBe(['each' => 'The length of Wrapped must be greater than 0']),
76+
->and($message)->toBe('The length of `[]` must be greater than 0')
77+
->and($fullMessage)->toBe('- The length of `[]` must be greater than 0')
78+
->and($messages)->toBe(['each' => 'The length of `[]` must be greater than 0']),
7979
));
8080

8181
test('With name, default', catchAll(
82-
fn() => v::named('Wrapper', v::each(v::named('Wrapped', v::intType())))->assert(['a', 'b', 'c']),
82+
fn() => v::named('Outer', v::each(v::named('Inner', v::intType())))->assert(['a', 'b', 'c']),
8383
fn(string $message, string $fullMessage, array $messages) => expect()
84-
->and($message)->toBe('Wrapped must be an integer')
84+
->and($message)->toBe('`.0` (<- Inner) must be an integer')
8585
->and($fullMessage)->toBe(<<<'FULL_MESSAGE'
86-
- Each item in Wrapped must be valid
87-
- `.0` must be an integer
88-
- `.1` must be an integer
89-
- `.2` must be an integer
86+
- Each item in Outer must be valid
87+
- `.0` (<- Inner) must be an integer
88+
- `.1` (<- Inner) must be an integer
89+
- `.2` (<- Inner) must be an integer
9090
FULL_MESSAGE)
9191
->and($messages)->toBe([
92-
'__root__' => 'Each item in Wrapped must be valid',
93-
0 => '`.0` must be an integer',
94-
1 => '`.1` must be an integer',
95-
2 => '`.2` must be an integer',
92+
'__root__' => 'Each item in Outer must be valid',
93+
0 => '`.0` (<- Inner) must be an integer',
94+
1 => '`.1` (<- Inner) must be an integer',
95+
2 => '`.2` (<- Inner) must be an integer',
9696
]),
9797
));
9898

9999
test('With name, inverted', catchAll(
100-
fn() => v::named('Not', v::not(v::named('Wrapper', v::each(v::named('Wrapped', v::intType())))))->assert([1, 2, 3]),
100+
fn() => v::named('Not', v::not(v::named('Outer', v::each(v::named('Inner', v::intType())))))->assert([1, 2, 3]),
101101
fn(string $message, string $fullMessage, array $messages) => expect()
102-
->and($message)->toBe('Wrapped must not be an integer')
102+
->and($message)->toBe('`.0` (<- Inner) must not be an integer')
103103
->and($fullMessage)->toBe(<<<'FULL_MESSAGE'
104-
- Each item in Wrapped must be invalid
105-
- `.0` must not be an integer
106-
- `.1` must not be an integer
107-
- `.2` must not be an integer
104+
- Each item in Outer must be invalid
105+
- `.0` (<- Inner) must not be an integer
106+
- `.1` (<- Inner) must not be an integer
107+
- `.2` (<- Inner) must not be an integer
108108
FULL_MESSAGE)
109109
->and($messages)->toBe([
110-
'__root__' => 'Each item in Wrapped must be invalid',
111-
0 => '`.0` must not be an integer',
112-
1 => '`.1` must not be an integer',
113-
2 => '`.2` must not be an integer',
110+
'__root__' => 'Each item in Outer must be invalid',
111+
0 => '`.0` (<- Inner) must not be an integer',
112+
1 => '`.1` (<- Inner) must not be an integer',
113+
2 => '`.2` (<- Inner) must not be an integer',
114114
]),
115115
));
116116

0 commit comments

Comments
 (0)