Ensure formatters only format, not modify results

henriquemoody · henriquemoody · commit ec70cf7bc2bd · 2026-01-03T19:47:30.000+01:00
When I changed the library to not overwrite existing names [1], I wasn't happy with how `FirstResultStringFormatter` was changing the results, because the results should be completely ready by the time they arrive in the formatters. This commit changes that behaviour, ensuring the results are complete with all necessary information by the time they reach the formatters. Along with those changes, I refactored some stringifiers and simplified the `InterpolationRenderer`; we were not overwriting the "name" parameter anyway, as it was just an unnecessary overhead. [1] 8332d28
diff --git a/library/Message/Formatter/FirstResultStringFormatter.php b/library/Message/Formatter/FirstResultStringFormatter.php
@@ -11,30 +11,19 @@
 
 use Respect\Validation\Message\Renderer;
 use Respect\Validation\Message\StringFormatter;
-use Respect\Validation\Name;
 use Respect\Validation\Result;
 
 final readonly class FirstResultStringFormatter implements StringFormatter
 {
     /** @param array<string|int, mixed> $templates */
     public function format(Result $result, Renderer $renderer, array $templates): string
-    {
-        return $this->formatResult($result, $renderer, $templates, null);
-    }
-
-    /** @param array<string|int, mixed> $templates */
-    private function formatResult(Result $result, Renderer $renderer, array $templates, Name|null $parentName): string
     {
         if (!$result->hasCustomTemplate()) {
             foreach ($result->children as $child) {
-                return $this->formatResult($child, $renderer, $templates, $result->name ?? $parentName);
+                return $this->format($child, $renderer, $templates);
             }
         }
 
-        if ($parentName !== null) {
-            $result = $result->withName($parentName);
-        }
-
         return $renderer->render($result, $templates);
     }
 }
diff --git a/library/Message/Formatter/NestedListStringFormatter.php b/library/Message/Formatter/NestedListStringFormatter.php
@@ -11,6 +11,7 @@
 
 use Respect\Validation\Message\Renderer;
 use Respect\Validation\Message\StringFormatter;
+use Respect\Validation\Name;
 use Respect\Validation\Result;
 
 use function array_filter;
@@ -27,7 +28,7 @@
     /** @param array<string|int, mixed> $templates */
     public function format(Result $result, Renderer $renderer, array $templates): string
     {
-        return $this->formatRecursively($result, $renderer, $templates, 0);
+        return $this->formatRecursively($result, $renderer, $templates, 0, null);
     }
 
     /** @param array<string|int, mixed> $templates */
@@ -36,23 +37,31 @@ private function formatRecursively(
         Renderer $renderer,
         array $templates,
         int $depth,
+        Name|null $lastVisibleName,
         Result ...$siblings,
     ): string {
         $formatted = '';
-        $displayedName = null;
         if ($this->isVisible($result, ...$siblings)) {
             $indentation = str_repeat(' ', $depth * 2);
-            $displayedName = $result->name;
-            $formatted .= sprintf('%s- %s' . PHP_EOL, $indentation, $renderer->render($result, $templates));
+            $formatted .= sprintf(
+                '%s- %s' . PHP_EOL,
+                $indentation,
+                $renderer->render(
+                    $lastVisibleName === $result->name ? $result->withoutName() : $result,
+                    $templates,
+                ),
+            );
+            $lastVisibleName ??= $result->name;
             $depth++;
         }
 
         foreach ($result->children as $child) {
             $formatted .= $this->formatRecursively(
-                $displayedName === $child->name ? $child->withoutName() : $child,
+                $child,
                 $renderer,
                 $templates,
                 $depth,
+                $lastVisibleName,
                 ...array_filter($result->children, static fn(Result $sibling) => $sibling !== $child),
             );
             $formatted .= PHP_EOL;
diff --git a/library/Message/InterpolationRenderer.php b/library/Message/InterpolationRenderer.php
@@ -10,7 +10,7 @@
 namespace Respect\Validation\Message;
 
 use Respect\Validation\Message\Formatter\TemplateResolver;
-use Respect\Validation\Name;
+use Respect\Validation\Message\Placeholder\Subject;
 use Respect\Validation\Result;
 
 use function array_key_exists;
@@ -31,7 +31,7 @@ public function __construct(
     /** @param array<string|int, mixed> $templates */
     public function render(Result $result, array $templates): string
     {
-        $parameters = ['path' => $result->path, 'input' => $result->input, 'subject' => $this->getName($result)];
+        $parameters = ['path' => $result->path, 'input' => $result->input, 'subject' => Subject::fromResult($result)];
         $parameters += $result->parameters;
 
         $givenTemplate = $this->templateResolver->getGivenTemplate($result, $templates);
@@ -65,25 +65,4 @@ private function processPlaceholder(array $parameters, array $matches): string
 
         return $this->modifier->modify($parameters[$name], $pipe);
     }
-
-    private function getName(Result $result): mixed
-    {
-        if (array_key_exists('name', $result->parameters) && is_string($result->parameters['name'])) {
-            return new Name($result->parameters['name']);
-        }
-
-        if (array_key_exists('name', $result->parameters)) {
-            return $result->parameters['name'];
-        }
-
-        if ($result->name !== null) {
-            return $result->name;
-        }
-
-        if ($result->path?->value !== null) {
-            return $result->path;
-        }
-
-        return $result->input;
-    }
 }
diff --git a/library/Message/Placeholder/Subject.php b/library/Message/Placeholder/Subject.php
@@ -0,0 +1,30 @@
+<?php
+
+/*
+ * Copyright (c) Alexandre Gomes Gaigalas <alganet@gmail.com>
+ * SPDX-License-Identifier: MIT
+ */
+
+declare(strict_types=1);
+
+namespace Respect\Validation\Message\Placeholder;
+
+use Respect\Validation\Name;
+use Respect\Validation\Path;
+use Respect\Validation\Result;
+
+final readonly class Subject
+{
+    public function __construct(
+        public mixed $input,
+        public Path|null $path = null,
+        public Name|null $name = null,
+        public bool $hasPrecedentName = true,
+    ) {
+    }
+
+    public static function fromResult(Result $result): self
+    {
+        return new self($result->input, $result->path, $result->name, $result->hasPrecedentName);
+    }
+}
diff --git a/library/Message/Stringifier/NameStringifier.php b/library/Message/Stringifier/NameStringifier.php
@@ -12,29 +12,14 @@
 use Respect\Stringifier\Stringifier;
 use Respect\Validation\Name;
 
-use function sprintf;
-
 final readonly class NameStringifier implements Stringifier
 {
-    public function __construct(
-        private Stringifier $stringifier,
-    ) {
-    }
-
     public function stringify(mixed $raw, int $depth): string|null
     {
         if (!$raw instanceof Name) {
             return null;
         }
 
-        if ($raw->path === null) {
-            return $raw->value;
-        }
-
-        return sprintf(
-            '%s (<- %s)',
-            $this->stringifier->stringify($raw->path, $depth),
-            $raw->value,
-        );
+        return $raw->value;
     }
 }
diff --git a/library/Message/Stringifier/SubjectStringifier.php b/library/Message/Stringifier/SubjectStringifier.php
@@ -0,0 +1,48 @@
+<?php
+
+/*
+ * Copyright (c) Alexandre Gomes Gaigalas <alganet@gmail.com>
+ * SPDX-License-Identifier: MIT
+ */
+
+declare(strict_types=1);
+
+namespace Respect\Validation\Message\Stringifier;
+
+use Respect\Stringifier\Stringifier;
+use Respect\Validation\Message\Placeholder\Subject;
+
+use function sprintf;
+
+final readonly class SubjectStringifier implements Stringifier
+{
+    public function __construct(
+        private Stringifier $stringifier,
+    ) {
+    }
+
+    public function stringify(mixed $raw, int $depth): string|null
+    {
+        if (!$raw instanceof Subject) {
+            return null;
+        }
+
+        if ($raw->path === null && $raw->name === null) {
+            return $this->stringifier->stringify($raw->input, $depth);
+        }
+
+        if ($raw->name === null) {
+            return $this->stringifier->stringify($raw->path, $depth);
+        }
+
+        if ($raw->path === null || $raw->hasPrecedentName) {
+            return $this->stringifier->stringify($raw->name, $depth);
+        }
+
+        return sprintf(
+            '%s (<- %s)',
+            $this->stringifier->stringify($raw->path, $depth),
+            $this->stringifier->stringify($raw->name, $depth),
+        );
+    }
+}
diff --git a/library/Message/ValidationStringifier.php b/library/Message/ValidationStringifier.php
@@ -35,6 +35,7 @@
 use Respect\Validation\Message\Stringifier\NameStringifier;
 use Respect\Validation\Message\Stringifier\PathStringifier;
 use Respect\Validation\Message\Stringifier\QuotedStringifier;
+use Respect\Validation\Message\Stringifier\SubjectStringifier;
 
 final readonly class ValidationStringifier implements Stringifier
 {
@@ -95,7 +96,8 @@ private function createStringifier(Quoter $quoter): Stringifier
         $stringifier->prependStringifier(new PathStringifier($quoter));
         $stringifier->prependStringifier(new QuotedStringifier($quoter));
         $stringifier->prependStringifier(new ListedStringifier($stringifier));
-        $stringifier->prependStringifier(new NameStringifier($stringifier));
+        $stringifier->prependStringifier(new NameStringifier());
+        $stringifier->prependStringifier(new SubjectStringifier($stringifier));
 
         return $stringifier;
     }
diff --git a/library/Name.php b/library/Name.php
@@ -16,9 +16,4 @@ public function __construct(
         public Path|null $path = null,
     ) {
     }
-
-    public function withPath(Path $path): Name
-    {
-        return new self($this->value, $path);
-    }
 }
diff --git a/library/Result.php b/library/Result.php
@@ -30,6 +30,7 @@ public function __construct(
         public array $parameters = [],
         public string $template = Rule::TEMPLATE_STANDARD,
         public bool $hasInvertedMode = false,
+        public bool $hasPrecedentName = true,
         public Name|null $name = null,
         public Result|null $adjacent = null,
         public Path|null $path = null,
@@ -124,6 +125,7 @@ public function withPath(Path $path): self
         return clone($this, [
             'path' => $path,
             'adjacent' => $this->adjacent?->withPath($path),
+            'hasPrecedentName' => $this->name !== null,
             'children' => array_map(
                 static fn(Result $child) => $child->withPath($path),
                 $this->children,
@@ -149,43 +151,40 @@ public function withoutName(): self
 
     public function withChildren(Result ...$children): self
     {
-        if ($this->path === null) {
-            return clone($this, ['children' => $children]);
-        }
-
-        return clone($this, ['children' => array_map(fn(Result $child) => $child->withPath($this->path), $children)]);
+        return clone($this, ['children' => $children]);
     }
 
     public function withName(Name $name): self
     {
-        if ($this->path !== null && $this->name?->path !== $this->path) {
-            $name = $name->withPath($this->path);
+        if ($this->name !== null) {
+            return $this;
         }
 
         return clone($this, [
-            'name' => $this->name ?? $name,
+            'name' => $name,
             'adjacent' => $this->adjacent?->withName($name),
             'children' => array_map(
-                static fn(Result $child) => $child->path === null ? $child->withName($child->name ?? $name) : $child,
+                static fn(Result $child) => $child->withName($name),
                 $this->children,
             ),
         ]);
     }
 
     public function withNameFrom(Rule $rule): self
     {
-        if ($rule instanceof Nameable && $rule->getName() !== null) {
-            return clone($this, [
-                'name' => $this->name ?? $rule->getName(),
-                'adjacent' => $this->adjacent?->withNameFrom($rule),
-                'children' => array_map(
-                    static fn(Result $child) => $child->withNameFrom($rule),
-                    $this->children,
-                ),
-            ]);
+        if (!$rule instanceof Nameable || $rule->getName() === null) {
+            return $this;
         }
 
-        return $this;
+        return clone($this, [
+            'name' => $this->name ?? $rule->getName(),
+            'hasPrecedentName' => true,
+            'adjacent' => $this->adjacent?->withNameFrom($rule),
+            'children' => array_map(
+                static fn(Result $child) => $child->withNameFrom($rule),
+                $this->children,
+            ),
+        ]);
     }
 
     public function withInput(mixed $input): self
diff --git a/library/Rules/Key.php b/library/Rules/Key.php
@@ -38,6 +38,6 @@ public function evaluate(mixed $input): Result
             return $keyExistsResult->withNameFrom($this->rule);
         }
 
-        return $this->rule->evaluate($input[$this->key])->withPath(new Path($this->key));
+        return $this->rule->evaluate($input[$this->key])->withPath($keyExistsResult->path ?? new Path($this->key));
     }
 }
diff --git a/library/Rules/Property.php b/library/Rules/Property.php
@@ -36,7 +36,7 @@ public function evaluate(mixed $input): Result
 
         return $this->rule
             ->evaluate($this->getPropertyValue($input, $this->propertyName))
-            ->withPath(new Path($this->propertyName));
+            ->withPath($propertyExistsResult->path ?? new Path($this->propertyName));
     }
 
     private function getPropertyValue(object $object, string $propertyName): mixed
diff --git a/tests/library/Builders/ResultBuilder.php b/tests/library/Builders/ResultBuilder.php
@@ -24,6 +24,8 @@ final class ResultBuilder
 
     private bool $hasInvertedMode = false;
 
+    private bool $hasPrecedentName = true;
+
     private string $template = Rule::TEMPLATE_STANDARD;
 
     /** @var array<string, mixed> */
@@ -58,6 +60,7 @@ public function build(): Result
             $this->parameters,
             $this->template,
             $this->hasInvertedMode,
+            $this->hasPrecedentName,
             $this->name,
             $this->adjacent,
             $this->path,
diff --git a/tests/unit/Message/InterpolationRendererTest.php b/tests/unit/Message/InterpolationRendererTest.php
diff --git a/tests/unit/Message/Stringifier/SubjectStringifierTest.php b/tests/unit/Message/Stringifier/SubjectStringifierTest.php

Original file line number	Diff line number	Diff line change
`@@ -11,30 +11,19 @@`
`11`	`11`
`12`	`12`	`use Respect\Validation\Message\Renderer;`
`13`	`13`	`use Respect\Validation\Message\StringFormatter;`
`14`		`-use Respect\Validation\Name;`
`15`	`14`	`use Respect\Validation\Result;`
`16`	`15`
`17`	`16`	`final readonly class FirstResultStringFormatter implements StringFormatter`
`18`	`17`	`{`
`19`	`18`	`/** @param array<string\|int, mixed> $templates */`
`20`	`19`	`public function format(Result $result, Renderer $renderer, array $templates): string`
`21`		`- {`
`22`		`- return $this->formatResult($result, $renderer, $templates, null);`
`23`		`- }`
`24`		`-`
`25`		`- /** @param array<string\|int, mixed> $templates */`
`26`		`- private function formatResult(Result $result, Renderer $renderer, array $templates, Name\|null $parentName): string`
`27`	`20`	`{`
`28`	`21`	`if (!$result->hasCustomTemplate()) {`
`29`	`22`	`foreach ($result->children as $child) {`
`30`		`- return $this->formatResult($child, $renderer, $templates, $result->name ?? $parentName);`
	`23`	`+ return $this->format($child, $renderer, $templates);`
`31`	`24`	`}`
`32`	`25`	`}`
`33`	`26`
`34`		`- if ($parentName !== null) {`
`35`		`- $result = $result->withName($parentName);`
`36`		`- }`
`37`		`-`
`38`	`27`	`return $renderer->render($result, $templates);`
`39`	`28`	`}`
`40`	`29`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,29 +12,14 @@`
`12`	`12`	`use Respect\Stringifier\Stringifier;`
`13`	`13`	`use Respect\Validation\Name;`
`14`	`14`
`15`		`-use function sprintf;`
`16`		`-`
`17`	`15`	`final readonly class NameStringifier implements Stringifier`
`18`	`16`	`{`
`19`		`- public function __construct(`
`20`		`- private Stringifier $stringifier,`
`21`		`- ) {`
`22`		`- }`
`23`		`-`
`24`	`17`	`public function stringify(mixed $raw, int $depth): string\|null`
`25`	`18`	`{`
`26`	`19`	`if (!$raw instanceof Name) {`
`27`	`20`	`return null;`
`28`	`21`	`}`
`29`	`22`
`30`		`- if ($raw->path === null) {`
`31`		`- return $raw->value;`
`32`		`- }`
`33`		`-`
`34`		`- return sprintf(`
`35`		`- '%s (<- %s)',`
`36`		`- $this->stringifier->stringify($raw->path, $depth),`
`37`		`- $raw->value,`
`38`		`- );`
	`23`	`+ return $raw->value;`
`39`	`24`	`}`
`40`	`25`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,9 +16,4 @@ public function __construct(`
`16`	`16`	`public Path\|null $path = null,`
`17`	`17`	`) {`
`18`	`18`	`}`
`19`		`-`
`20`		`- public function withPath(Path $path): Name`
`21`		`- {`
`22`		`- return new self($this->value, $path);`
`23`		`- }`
`24`	`19`	`}`
Original file line number	Diff line number	Diff line change
`@@ -38,6 +38,6 @@ public function evaluate(mixed $input): Result`
`38`	`38`	`return $keyExistsResult->withNameFrom($this->rule);`
`39`	`39`	`}`
`40`	`40`
`41`		`- return $this->rule->evaluate($input[$this->key])->withPath(new Path($this->key));`
	`41`	`+ return $this->rule->evaluate($input[$this->key])->withPath($keyExistsResult->path ?? new Path($this->key));`
`42`	`42`	`}`
`43`	`43`	`}`