Skip to content

Commit ab992b4

Browse files
committed
Fix deep path chain and __root__ template cascading bugs
Result::withPath() previously set `$this->path->parent = $path` directly on the leaf node, overwriting intermediate path segments. With 3+ levels of nested keys (e.g. server.database.host), the middle segment was lost, producing `.server.host` instead of `.server.database.host`. The fix walks to the root of the existing path chain before appending. TemplateResolver::getGivenTemplate() treated `__root__` as a universal fallback in the lookup chain, causing it to match ALL results in the tree instead of only the composite it was intended for. The fix introduces an `$isRoot` flag threaded from each formatter through the Renderer into TemplateResolver, combined with a `fullyConsumed` check from path drilling, so `__root__` only applies when the result is at the correct template scope. Optimizations made during the fixes: - NestedListStringFormatter::isVisible(): replaced O(n!) mutual sibling recursion with O(n) linear scan — the transitive visibility check reduces to "does any sibling have a custom template or != 1 children" - InterpolationRenderer: inlined getValidatorTemplate() into the ?? expression so it short-circuits when a given template exists Add 12 feature tests covering deep path rendering, multi-level __root__ scoping, deep template resolution, partial template coverage, and mixed depth siblings.
1 parent f087f55 commit ab992b4

13 files changed

+747
-56
lines changed

src/Message/Formatter/FirstResultStringFormatter.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@
1818
final readonly class FirstResultStringFormatter implements StringFormatter
1919
{
2020
/** @param array<string|int, mixed> $templates */
21-
public function format(Result $result, Renderer $renderer, array $templates): string
21+
public function format(Result $result, Renderer $renderer, array $templates, bool $isRoot = true): string
2222
{
2323
if (!$result->hasCustomTemplate()) {
2424
foreach ($result->children as $child) {
25-
return $this->format($child, $renderer, $templates);
25+
return $this->format($child, $renderer, $templates, false);
2626
}
2727
}
2828

29-
return $renderer->render($result, $templates);
29+
return $renderer->render($result, $templates, $isRoot);
3030
}
3131
}

src/Message/Formatter/NestedArrayFormatter.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@
2828
*
2929
* @return array<string|int, mixed>
3030
*/
31-
public function format(Result $result, Renderer $renderer, array $templates): array
31+
public function format(Result $result, Renderer $renderer, array $templates, bool $isRoot = true): array
3232
{
3333
if ($result->children === []) {
34-
return [$result->path->value ?? $result->id->value => $renderer->render($result, $templates)];
34+
return [$result->path->value ?? $result->id->value => $renderer->render($result, $templates, $isRoot)];
3535
}
3636

3737
$hasStringKey = false;
@@ -57,7 +57,7 @@ public function format(Result $result, Renderer $renderer, array $templates): ar
5757
}
5858

5959
if (count($messages) > 1) {
60-
return ['__root__' => $renderer->render($result, $templates)] + $messages;
60+
return ['__root__' => $renderer->render($result, $templates, $isRoot)] + $messages;
6161
}
6262

6363
return $messages;
@@ -85,12 +85,13 @@ private function formatChild(
8585
$hasMultipleChildren && $child->name === $parent->name ? $child->withoutName() : $child,
8686
$renderer,
8787
$templates,
88+
false,
8889
);
8990

9091
$childMessage = count($formatted) === 1 ? current($formatted) : $formatted;
9192

9293
if (is_array($childMessage) && count($childMessage) > 1 && !isset($childMessage['__root__'])) {
93-
$childMessage = ['__root__' => $renderer->render($child, $templates)] + $childMessage;
94+
$childMessage = ['__root__' => $renderer->render($child, $templates, false)] + $childMessage;
9495
}
9596

9697
if (!$hasStringKey) {
@@ -147,7 +148,7 @@ private function flattenToIndexedList(
147148
Renderer $renderer,
148149
array $templates,
149150
): array {
150-
$parentMessage = $messages['__root__'] ?? $renderer->render($parent, $templates);
151+
$parentMessage = $messages['__root__'] ?? $renderer->render($parent, $templates, false);
151152

152153
return ['__root__' => $parentMessage] + array_values($messages) + [count($messages) => $childMessage];
153154
}

src/Message/Formatter/NestedListStringFormatter.php

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
/** @param array<string|int, mixed> $templates */
3030
public function format(Result $result, Renderer $renderer, array $templates): string
3131
{
32-
return $this->formatRecursively($result, $renderer, $templates, 0, null);
32+
return $this->formatRecursively($result, $renderer, $templates, 0, null, true);
3333
}
3434

3535
/** @param array<string|int, mixed> $templates */
@@ -39,6 +39,7 @@ private function formatRecursively(
3939
array $templates,
4040
int $depth,
4141
Name|null $lastVisibleName,
42+
bool $isRoot,
4243
Result ...$siblings,
4344
): string {
4445
$formatted = '';
@@ -50,6 +51,7 @@ private function formatRecursively(
5051
$renderer->render(
5152
$lastVisibleName === $result->name ? $result->withoutName() : $result,
5253
$templates,
54+
$isRoot,
5355
),
5456
);
5557
$lastVisibleName ??= $result->name;
@@ -63,6 +65,7 @@ private function formatRecursively(
6365
$templates,
6466
$depth,
6567
$lastVisibleName,
68+
false,
6669
...array_filter($result->children, static fn(Result $sibling) => $sibling !== $child),
6770
);
6871
$formatted .= PHP_EOL;
@@ -77,21 +80,12 @@ private function isVisible(Result $result, Result ...$siblings): bool
7780
return true;
7881
}
7982

80-
// Parents of an only child are not visible by default
8183
if (count($result->children) !== 1) {
8284
return true;
8385
}
8486

85-
// Only children are always visible
86-
if (count($siblings) === 0) {
87-
return false;
88-
}
89-
90-
// The visibility of a result then will depend on whether any of its siblings is visible
91-
foreach ($siblings as $key => $currentSibling) {
92-
$otherSiblings = $siblings;
93-
unset($otherSiblings[$key]);
94-
if ($this->isVisible($currentSibling, ...$otherSiblings)) {
87+
foreach ($siblings as $sibling) {
88+
if ($sibling->hasCustomTemplate() || count($sibling->children) !== 1) {
9589
return true;
9690
}
9791
}

src/Message/Formatter/TemplateResolver.php

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
use Respect\Validation\Path;
1616
use Respect\Validation\Result;
1717

18-
use function array_reduce;
19-
use function array_reverse;
2018
use function is_array;
2119
use function is_string;
2220

@@ -28,26 +26,33 @@ public function __construct(
2826
}
2927

3028
/** @param array<string|int, mixed> $templates */
31-
public function getGivenTemplate(Result $result, array $templates): string|null
29+
public function getGivenTemplate(Result $result, array $templates, bool $isRoot = true): string|null
3230
{
3331
if ($result->hasCustomTemplate()) {
3432
return $result->template;
3533
}
3634

35+
$filtered = $templates;
36+
$isAtCorrectScope = $isRoot;
37+
3738
if ($result->path !== null) {
38-
$templates = $this->filterByPath($result->path, $templates);
39+
[$filtered, $isAtCorrectScope] = $this->filterByPath($result->path, $templates);
3940
}
4041

41-
foreach ([$result->path?->value, $result->name?->value, $result->id->value, '__root__'] as $key) {
42-
if ($key === null || !isset($templates[$key])) {
42+
foreach ([$result->path?->value, $result->name?->value, $result->id->value] as $key) {
43+
if ($key === null || !isset($filtered[$key])) {
4344
continue;
4445
}
4546

46-
if (is_string($templates[$key])) {
47-
return $templates[$key];
47+
if (is_string($filtered[$key])) {
48+
return $filtered[$key];
4849
}
4950
}
5051

52+
if ($isAtCorrectScope && isset($filtered['__root__']) && is_string($filtered['__root__'])) {
53+
return $filtered['__root__'];
54+
}
55+
5156
return null;
5257
}
5358

@@ -69,31 +74,22 @@ public function getValidatorTemplate(Result $result): string
6974
}
7075

7176
/**
72-
* @param array<string|int> $nodes
77+
* @param array<string|int, mixed> $templates
7378
*
74-
* @return non-empty-array<string|int>
79+
* @return array{array<string|int, mixed>, bool}
7580
*/
76-
private function getNodes(Path $path, array $nodes = []): array
81+
private function filterByPath(Path $path, array $templates): array
7782
{
78-
$nodes[] = $path->value;
7983
if ($path->parent !== null) {
80-
return $this->getNodes($path->parent, $nodes);
84+
[$templates, $fullyConsumed] = $this->filterByPath($path->parent, $templates);
85+
} else {
86+
$fullyConsumed = true;
8187
}
8288

83-
return $nodes;
84-
}
89+
if (isset($templates[$path->value]) && is_array($templates[$path->value])) {
90+
return [$templates[$path->value], $fullyConsumed];
91+
}
8592

86-
/**
87-
* @param array<string|int, mixed> $templates
88-
*
89-
* @return array<string|int, mixed>
90-
*/
91-
private function filterByPath(Path $path, array $templates): array
92-
{
93-
return array_reduce(
94-
array_reverse($this->getNodes($path)),
95-
static fn(array $carry, $node) => isset($carry[$node]) && is_array($carry[$node]) ? $carry[$node] : $carry,
96-
$templates,
97-
);
93+
return [$templates, false];
9894
}
9995
}

src/Message/InterpolationRenderer.php

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,20 @@ public function __construct(
2626
}
2727

2828
/** @param array<string|int, mixed> $templates */
29-
public function render(Result $result, array $templates): string
29+
public function render(Result $result, array $templates, bool $isRoot = true): string
3030
{
3131
$parameters = ['path' => $result->path, 'input' => $result->input, 'subject' => $result];
3232
$parameters += $result->parameters;
3333

34-
$givenTemplate = $this->templateResolver->getGivenTemplate($result, $templates);
35-
$ruleTemplate = $this->templateResolver->getValidatorTemplate($result);
34+
$givenTemplate = $this->templateResolver->getGivenTemplate($result, $templates, $isRoot);
3635

3736
$rendered = $this->formatter->formatUsing(
38-
$this->translator->trans($givenTemplate ?? $ruleTemplate),
37+
$this->translator->trans($givenTemplate ?? $this->templateResolver->getValidatorTemplate($result)),
3938
$parameters,
4039
);
4140

4241
if (!$result->hasCustomTemplate() && $givenTemplate === null && $result->adjacent !== null) {
43-
$rendered .= ' ' . $this->render($result->adjacent, $templates);
42+
$rendered .= ' ' . $this->render($result->adjacent, $templates, false);
4443
}
4544

4645
return $rendered;

src/Message/Renderer.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,5 @@
1616
interface Renderer
1717
{
1818
/** @param array<string|int, mixed> $templates */
19-
public function render(Result $result, array $templates): string;
19+
public function render(Result $result, array $templates, bool $isRoot = true): string;
2020
}

src/Result.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,12 @@ public function withPrecedentName(bool $hasPrecedentName): self
125125
public function withPath(Path $path): self
126126
{
127127
if ($this->path !== null) {
128-
$this->path->parent = $path;
128+
$current = $this->path;
129+
while ($current->parent !== null) {
130+
$current = $current->parent;
131+
}
132+
133+
$current->parent = $path;
129134

130135
return $this;
131136
}

0 commit comments

Comments
 (0)