From d9380588e76337ef06c23daeb935f1aa2d0e7085 Mon Sep 17 00:00:00 2001 From: Henrique Moody Date: Fri, 2 Jan 2026 12:50:33 +0100 Subject: [PATCH] Remove the `setTemplate()` method from `Validator` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `setTemplate()` method can be confusing, as it can be tricky for someone to determine which chain is being templated. Using the `Templated` rule makes this much more explicit and adds a little bit of verbosity. For users, this will be a significant change, but there are easy ways to update this code in their projects, so I’m not overly concerned about it. Another benefit of this change is that it makes the `Validator` much simpler, as it no longer needs to track the template. This change also makes the `Reducer` simpler, for similar reasons to the `Validator`. However, because the `Validator` is simpler, we can remove the code that the `Reducer` had to meet the specific needs of the `Validator`. --- library/Rules/Core/Reducer.php | 22 ---- library/Validator.php | 49 +++----- tests/feature/AssertWithTemplatesTest.php | 2 +- tests/feature/Issues/Issue619Test.php | 2 +- tests/feature/Issues/Issue805Test.php | 2 +- tests/feature/Rules/BetweenExclusiveTest.php | 2 +- tests/feature/Rules/CircuitTest.php | 3 +- tests/feature/Rules/DateTimeDiffTest.php | 6 +- tests/feature/Rules/EachTest.php | 10 +- ...ouldUseTemplateAsFullMessageHeaderTest.php | 2 +- ...torsShouldUseTemplateAsMainMessageTest.php | 2 +- ...atorShouldUseTemplateAsMainMessageTest.php | 2 +- tests/unit/Rules/Core/ReducerTest.php | 16 --- tests/unit/ValidatorTest.php | 106 +++++++----------- 14 files changed, 71 insertions(+), 155 deletions(-) diff --git a/library/Rules/Core/Reducer.php b/library/Rules/Core/Reducer.php index 3419d860..e6c58b7c 100644 --- a/library/Rules/Core/Reducer.php +++ b/library/Rules/Core/Reducer.php @@ -9,11 +9,8 @@ declare(strict_types=1); namespace Respect\Validation\Rules\Core; -use Respect\Validation\Name; use Respect\Validation\Rule; use Respect\Validation\Rules\AllOf; -use Respect\Validation\Rules\Named; -use Respect\Validation\Rules\Templated; final class Reducer extends Wrapper { @@ -21,23 +18,4 @@ final class Reducer extends Wrapper { parent::__construct($rules === [] ? $rule1 : new AllOf($rule1, ...$rules)); } - - public function withTemplate(string $template): self - { - return new self(new Templated($this->rule, $template)); - } - - public function withName(Name $name): self - { - return new self(new Named($this->rule, $name)); - } - - public function getName(): Name|null - { - if ($this->rule instanceof Nameable) { - return $this->rule->getName(); - } - - return null; - } } diff --git a/library/Validator.php b/library/Validator.php index cf462261..fac358f2 100644 --- a/library/Validator.php +++ b/library/Validator.php @@ -9,15 +9,18 @@ declare(strict_types=1); namespace Respect\Validation; +use Respect\Validation\Exceptions\ComponentException; use Respect\Validation\Exceptions\ValidationException; use Respect\Validation\Message\ArrayFormatter; use Respect\Validation\Message\Renderer; use Respect\Validation\Message\StringFormatter; use Respect\Validation\Mixins\Builder; +use Respect\Validation\Rules\AllOf; use Respect\Validation\Rules\Core\Nameable; -use Respect\Validation\Rules\Core\Reducer; use Throwable; +use function count; +use function current; use function is_array; use function is_callable; use function is_string; @@ -31,8 +34,6 @@ final class Validator implements Rule, Nameable /** @var array */ private array $templates = []; - private string|null $template = null; - /** @param array $ignoredBacktracePaths */ public function __construct( private readonly RuleFactory $ruleFactory, @@ -55,7 +56,13 @@ final class Validator implements Rule, Nameable public function evaluate(mixed $input): Result { - return $this->getRule()->evaluate($input); + $rule = match (count($this->rules)) { + 0 => throw new ComponentException('No rules have been added to this validator.'), + 1 => current($this->rules), + default => new AllOf(...$this->rules), + }; + + return $rule->evaluate($input); } /** @param array|string|null $template */ @@ -112,16 +119,6 @@ final class Validator implements Rule, Nameable return $this; } - public function getRule(): Reducer - { - $reducer = new Reducer(...$this->rules); - if ($this->template !== null) { - $reducer = $reducer->withTemplate($this->template); - } - - return $reducer; - } - /** @return array */ public function getRules(): array { @@ -130,32 +127,18 @@ final class Validator implements Rule, Nameable public function getName(): Name|null { - return $this->getRule()->getName(); - } + if (count($this->rules) === 1 && current($this->rules) instanceof Nameable) { + return current($this->rules)->getName(); + } - public function getTemplate(): string|null - { - return $this->template; - } - - public function setTemplate(string $template): static - { - $this->template = $template; - - return $this; + return null; } /** @param array|string|null $template */ private function toResultQuery(Result $result, array|string|null $template): ResultQuery { - if (is_string($template)) { - $result = $result->withTemplate($template); - } elseif ($this->getTemplate() != null) { - $result = $result->withTemplate($this->getTemplate()); - } - return new ResultQuery( - $this->resultFilter->filter($result), + $this->resultFilter->filter(is_string($template) ? $result->withTemplate($template) : $result), $this->renderer, $this->mainMessageFormatter, $this->fullMessageFormatter, diff --git a/tests/feature/AssertWithTemplatesTest.php b/tests/feature/AssertWithTemplatesTest.php index df083f0d..e4709bb0 100644 --- a/tests/feature/AssertWithTemplatesTest.php +++ b/tests/feature/AssertWithTemplatesTest.php @@ -8,7 +8,7 @@ declare(strict_types=1); test('Template as a string in the chain', catchAll( - fn() => v::alwaysInvalid()->setTemplate('My string template in the chain')->assert(1), + fn() => v::templated(v::alwaysInvalid(), 'My string template in the chain')->assert(1), fn(string $message, string $fullMessage, array $messages) => expect() ->and($message)->toBe('My string template in the chain') ->and($fullMessage)->toBe('- My string template in the chain') diff --git a/tests/feature/Issues/Issue619Test.php b/tests/feature/Issues/Issue619Test.php index 9dd0742f..866530ed 100644 --- a/tests/feature/Issues/Issue619Test.php +++ b/tests/feature/Issues/Issue619Test.php @@ -8,7 +8,7 @@ declare(strict_types=1); test('https://github.com/Respect/Validation/issues/619', catchAll( - fn() => v::instance(stdClass::class)->setTemplate('invalid object')->assert('test'), + fn() => v::templated(v::instance(stdClass::class), 'invalid object')->assert('test'), fn(string $message, string $fullMessage, array $messages) => expect() ->and($message)->toBe('invalid object') ->and($fullMessage)->toBe('- invalid object') diff --git a/tests/feature/Issues/Issue805Test.php b/tests/feature/Issues/Issue805Test.php index 48fb5e16..1d73f0c3 100644 --- a/tests/feature/Issues/Issue805Test.php +++ b/tests/feature/Issues/Issue805Test.php @@ -8,7 +8,7 @@ declare(strict_types=1); test('https://github.com/Respect/Validation/issues/805', catchAll( - fn() => v::key('email', v::email()->setTemplate('WRONG EMAIL!!!!!!'))->assert(['email' => 'qwe']), + fn() => v::key('email', v::templated(v::email(), 'WRONG EMAIL!!!!!!'))->assert(['email' => 'qwe']), fn(string $message, string $fullMessage, array $messages) => expect() ->and($message)->toBe('WRONG EMAIL!!!!!!') ->and($fullMessage)->toBe('- WRONG EMAIL!!!!!!') diff --git a/tests/feature/Rules/BetweenExclusiveTest.php b/tests/feature/Rules/BetweenExclusiveTest.php index 0979b4e1..8c5d4c57 100644 --- a/tests/feature/Rules/BetweenExclusiveTest.php +++ b/tests/feature/Rules/BetweenExclusiveTest.php @@ -24,7 +24,7 @@ test('Inverted', catchAll( )); test('With template', catchAll( - fn() => v::betweenExclusive(1, 10)->setTemplate('Bewildered bees buzzed between blooming begonias')->assert(12), + fn() => v::templated(v::betweenExclusive(1, 10), 'Bewildered bees buzzed between blooming begonias')->assert(12), fn(string $message, string $fullMessage, array $messages) => expect() ->and($message)->toBe('Bewildered bees buzzed between blooming begonias') ->and($fullMessage)->toBe('- Bewildered bees buzzed between blooming begonias') diff --git a/tests/feature/Rules/CircuitTest.php b/tests/feature/Rules/CircuitTest.php index 7d89f624..61b47a5e 100644 --- a/tests/feature/Rules/CircuitTest.php +++ b/tests/feature/Rules/CircuitTest.php @@ -72,8 +72,7 @@ test('With the name set in the "circuit" that has an inverted failing rule', cat )); test('With template', catchAll( - fn() => v::circuit(v::alwaysValid(), v::trueVal()) - ->setTemplate('Circuit cool cats cunningly continuous cookies') + fn() => v::templated(v::circuit(v::alwaysValid(), v::trueVal()), 'Circuit cool cats cunningly continuous cookies') ->assert(false), fn(string $message, string $fullMessage, array $messages) => expect() ->and($message)->toBe('Circuit cool cats cunningly continuous cookies') diff --git a/tests/feature/Rules/DateTimeDiffTest.php b/tests/feature/Rules/DateTimeDiffTest.php index 5be75bae..d281a81a 100644 --- a/tests/feature/Rules/DateTimeDiffTest.php +++ b/tests/feature/Rules/DateTimeDiffTest.php @@ -85,7 +85,7 @@ test('With custom $now', catchAll( )); test('With custom template', catchAll( - fn() => v::dateTimeDiff('years', v::equals(2)->setTemplate('Custom template'))->assert('1 year ago'), + fn() => v::dateTimeDiff('years', v::templated(v::equals(2), 'Custom template'))->assert('1 year ago'), fn(string $message, string $fullMessage, array $messages) => expect() ->and($message)->toBe('Custom template') ->and($fullMessage)->toBe('- Custom template') @@ -109,7 +109,7 @@ test('Wrapping "not"', catchAll( )); test('Wrapped with custom template', catchAll( - fn() => v::dateTimeDiff('years', v::equals(2)->setTemplate('Wrapped with custom template'))->assert('1 year ago'), + fn() => v::dateTimeDiff('years', v::templated(v::equals(2), 'Wrapped with custom template'))->assert('1 year ago'), fn(string $message, string $fullMessage, array $messages) => expect() ->and($message)->toBe('Wrapped with custom template') ->and($fullMessage)->toBe('- Wrapped with custom template') @@ -117,7 +117,7 @@ test('Wrapped with custom template', catchAll( )); test('Wrapper with custom template', catchAll( - fn() => v::dateTimeDiff('years', v::equals(2))->setTemplate('Wrapper with custom template')->assert('1 year ago'), + fn() => v::templated(v::dateTimeDiff('years', v::equals(2)), 'Wrapper with custom template')->assert('1 year ago'), fn(string $message, string $fullMessage, array $messages) => expect() ->and($message)->toBe('Wrapper with custom template') ->and($fullMessage)->toBe('- Wrapper with custom template') diff --git a/tests/feature/Rules/EachTest.php b/tests/feature/Rules/EachTest.php index 25245346..d74a7303 100644 --- a/tests/feature/Rules/EachTest.php +++ b/tests/feature/Rules/EachTest.php @@ -166,7 +166,7 @@ test('With Not name, inverted', catchAll( )); test('With template, non-iterable', catchAll( - fn() => v::each(v::intType())->setTemplate('You should have passed an iterable')->assert(null), + fn() => v::templated(v::each(v::intType()), 'You should have passed an iterable')->assert(null), fn(string $message, string $fullMessage, array $messages) => expect() ->and($message)->toBe('You should have passed an iterable') ->and($fullMessage)->toBe('- You should have passed an iterable') @@ -174,7 +174,7 @@ test('With template, non-iterable', catchAll( )); test('With template, empty', catchAll( - fn() => v::each(v::intType())->setTemplate('You should have passed an non-empty') + fn() => v::templated(v::each(v::intType()), 'You should have passed an non-empty') ->assert([]), fn(string $message, string $fullMessage, array $messages) => expect() ->and($message)->toBe('You should have passed an non-empty') @@ -183,8 +183,7 @@ test('With template, empty', catchAll( )); test('With template, default', catchAll( - fn() => v::each(v::intType()) - ->setTemplate('All items should have been integers') + fn() => v::templated(v::each(v::intType()), 'All items should have been integers') ->assert(['a', 'b', 'c']), fn(string $message, string $fullMessage, array $messages) => expect() ->and($message)->toBe('All items should have been integers') @@ -203,8 +202,7 @@ test('With template, default', catchAll( )); test('with template, inverted', catchAll( - fn() => v::not(v::each(v::intType())) - ->setTemplate('All items should not have been integers') + fn() => v::templated(v::not(v::each(v::intType())), 'All items should not have been integers') ->assert([1, 2, 3]), fn(string $message, string $fullMessage, array $messages) => expect() ->and($message)->toBe('All items should not have been integers') diff --git a/tests/feature/SetTemplateWithMultipleValidatorsShouldUseTemplateAsFullMessageHeaderTest.php b/tests/feature/SetTemplateWithMultipleValidatorsShouldUseTemplateAsFullMessageHeaderTest.php index 7d46671f..6f6ad030 100644 --- a/tests/feature/SetTemplateWithMultipleValidatorsShouldUseTemplateAsFullMessageHeaderTest.php +++ b/tests/feature/SetTemplateWithMultipleValidatorsShouldUseTemplateAsFullMessageHeaderTest.php @@ -8,7 +8,7 @@ declare(strict_types=1); test('Scenario #1', catchFullMessage( - fn() => v::callback('is_string')->between(1, 2)->setTemplate('{{subject}} is not tasty')->assert('something'), + fn() => v::templated(v::callback('is_string')->between(1, 2), '{{subject}} is not tasty')->assert('something'), fn(string $fullMessage) => expect($fullMessage)->toBe(<<<'FULL_MESSAGE' - "something" is not tasty - "something" must be between 1 and 2 diff --git a/tests/feature/SetTemplateWithMultipleValidatorsShouldUseTemplateAsMainMessageTest.php b/tests/feature/SetTemplateWithMultipleValidatorsShouldUseTemplateAsMainMessageTest.php index fc2a8994..639abef3 100644 --- a/tests/feature/SetTemplateWithMultipleValidatorsShouldUseTemplateAsMainMessageTest.php +++ b/tests/feature/SetTemplateWithMultipleValidatorsShouldUseTemplateAsMainMessageTest.php @@ -10,6 +10,6 @@ declare(strict_types=1); use Respect\Validation\Validator; test('Scenario #1', catchMessage(function (): void { - Validator::callback('is_int')->between(1, 2)->setTemplate('{{subject}} is not tasty')->assert('something'); + v::templated(Validator::callback('is_int')->between(1, 2), '{{subject}} is not tasty')->assert('something'); }, fn(string $message) => expect($message)->toBe('"something" is not tasty'))); diff --git a/tests/feature/SetTemplateWithSingleValidatorShouldUseTemplateAsMainMessageTest.php b/tests/feature/SetTemplateWithSingleValidatorShouldUseTemplateAsMainMessageTest.php index 2a8531b5..e578e7ab 100644 --- a/tests/feature/SetTemplateWithSingleValidatorShouldUseTemplateAsMainMessageTest.php +++ b/tests/feature/SetTemplateWithSingleValidatorShouldUseTemplateAsMainMessageTest.php @@ -10,6 +10,6 @@ declare(strict_types=1); use Respect\Validation\Validator; test('Scenario', catchMessage( - fn() => Validator::callback('is_int')->setTemplate('{{subject}} is not tasty')->assert('something'), + fn() => v::templated(Validator::callback('is_int'), '{{subject}} is not tasty')->assert('something'), fn(string $message) => expect($message)->toBe('"something" is not tasty'), )); diff --git a/tests/unit/Rules/Core/ReducerTest.php b/tests/unit/Rules/Core/ReducerTest.php index a6bea7da..1cacc7b6 100644 --- a/tests/unit/Rules/Core/ReducerTest.php +++ b/tests/unit/Rules/Core/ReducerTest.php @@ -41,20 +41,4 @@ final class ReducerTest extends TestCase self::assertEquals(new AllOf($rule1, $rule2, $rule3), $result->rule); } - - #[Test] - public function shouldCreateWithTemplate(): void - { - $rule = Stub::any(1); - - $template = 'This is my template'; - - $reducer = new Reducer($rule); - $withTemplated = $reducer->withTemplate($template); - - $result = $withTemplated->evaluate(null); - - self::assertSame($rule, $result->rule); - self::assertSame($template, $result->template); - } } diff --git a/tests/unit/ValidatorTest.php b/tests/unit/ValidatorTest.php index 0936b936..7f2748b0 100644 --- a/tests/unit/ValidatorTest.php +++ b/tests/unit/ValidatorTest.php @@ -9,7 +9,6 @@ declare(strict_types=1); namespace Respect\Validation; -use Exception; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DoesNotPerformAssertions; use PHPUnit\Framework\Attributes\Test; @@ -17,7 +16,6 @@ use Respect\Validation\Exceptions\ComponentException; use Respect\Validation\Exceptions\ValidationException; use Respect\Validation\Test\Rules\Stub; use Respect\Validation\Test\TestCase; -use Throwable; use function uniqid; @@ -79,44 +77,6 @@ final class ValidatorTest extends TestCase $validator->assert('whatever'); } - #[Test] - public function itShouldAssertUsingThePreDefinedTemplateInTheChain(): void - { - $template = 'This is my pre-defined template'; - - $this->expectExceptionMessage($template); - - $validator = Validator::create(Stub::fail(1)); - $validator->setTemplate($template); - $validator->assert('whatever'); - } - - #[Test] - public function itShouldAssertUsingTheGivingCallableEvenWhenRuleAlreadyHasTemplate(): void - { - $predefinedTemplate = 'Current template'; - - $template = static fn(Throwable $exception) => new Exception('My exception: ' . $exception->getMessage()); - - $this->expectExceptionMessage('My exception: ' . $predefinedTemplate); - - $validator = Validator::create(Stub::fail(1)); - $validator->setTemplate($predefinedTemplate); - $validator->assert('whatever', $template); - } - - #[Test] - public function itShouldAssertUsingTheGivingExceptionEvenWhenRuleAlreadyHasTemplate(): void - { - $template = new Exception('This is a test'); - - $this->expectExceptionObject($template); - - $validator = Validator::create(Stub::fail(1)); - $validator->setTemplate('This wont be used'); - $validator->assert('whatever', $template); - } - #[Test] public function itShouldAssertUsingTheGivingStringTemplate(): void { @@ -196,19 +156,6 @@ final class ValidatorTest extends TestCase self::assertSame($template, $resultQuery->toMessage()); } - #[Test] - public function itShouldValidateUsingPreDefinedTemplateFromSetTemplate(): void - { - $template = uniqid(); - - $validator = Validator::create(Stub::fail(1)); - $validator->setTemplate($template); - - $resultQuery = $validator->validate('whatever'); - - self::assertSame($template, $resultQuery->toMessage()); - } - #[Test] public function itShouldValidateUsingPreDefinedTemplatesFromSetTemplates(): void { @@ -222,19 +169,6 @@ final class ValidatorTest extends TestCase self::assertSame($template, $resultQuery->toMessage()); } - #[Test] - public function itShouldValidateOverridingPreDefinedTemplateWithStringTemplate(): void - { - $overrideTemplate = uniqid(); - - $validator = Validator::create(Stub::fail(1)); - $validator->setTemplate('This should be overridden'); - - $resultQuery = $validator->validate('whatever', $overrideTemplate); - - self::assertSame($overrideTemplate, $resultQuery->toMessage()); - } - #[Test] public function itShouldValidateOverridingPreDefinedTemplatesWithArrayTemplates(): void { @@ -247,4 +181,44 @@ final class ValidatorTest extends TestCase self::assertSame($overrideTemplate, $resultQuery->toMessage()); } + + #[Test] + public function itShouldEvaluateAndThrowExceptionWhenNoRulesAreAdded(): void + { + $this->expectException(ComponentException::class); + $this->expectExceptionMessage('No rules have been added to this validator.'); + + $validator = Validator::create(); + $validator->evaluate('whatever'); + } + + #[Test] + public function itShouldEvaluateAndReturnResultWhenOneRuleIsAdded(): void + { + $validator = Validator::create(Stub::pass(1)); + + $result = $validator->evaluate('whatever'); + + self::assertTrue($result->hasPassed); + } + + #[Test] + public function itShouldEvaluateAndReturnResultWhenMultipleRulesAreAdded(): void + { + $validator = Validator::create(Stub::pass(1), Stub::fail(2)); + + $result = $validator->evaluate('whatever'); + + self::assertFalse($result->hasPassed); + } + + #[Test] + public function itShouldEvaluateAndReturnResultWhenSingleFailingRuleIsAdded(): void + { + $validator = Validator::create(Stub::fail(1)); + + $result = $validator->evaluate('whatever'); + + self::assertFalse($result->hasPassed); + } }