Update some templates and improve tests

Some templates were a bit confusing, and I would like to favour adding the `{{name}}` at the beginning of the templates as it helps when reading nested messages. I also deleted the regression tests for issue Respect#1348, because it's a non-issue, actually. The best approach to that problem is indeed using `When` insteaf of `OneOf`.
henriquemoody · Dec 27, 2024 · a0d6355 · a0d6355
1 parent a8ae57f
commit a0d6355
Show file tree

Hide file tree

Showing 44 changed files with 435 additions and 240 deletions.
diff --git a/docs/03-handling-exceptions.md b/docs/03-handling-exceptions.md
@@ -19,7 +19,7 @@ try {
 The code above generates the following output:
 
 ```no-highlight
-- All the required rules must pass for "The Respect Panda"
+- "The Respect Panda" must pass all the rules
   - "The Respect Panda" must contain only letters (a-z) and digits (0-9)
   - "The Respect Panda" must contain only lowercase letters
 ```
@@ -44,7 +44,7 @@ The code above generates the following output:
 ```no-highlight
 Array
 (
-    [__root__] => All the required rules must pass for "The Respect Panda"
+    [__root__] => "The Respect Panda" must pass all the rules
     [alnum] => "The Respect Panda" must contain only letters (a-z) and digits (0-9)
     [lowercase] => "The Respect Panda" must contain only lowercase letters
 )

diff --git a/docs/rules/AllOf.md b/docs/rules/AllOf.md
@@ -12,17 +12,21 @@ v::allOf(v::intVal(), v::positive())->isValid(15); // true
 
 ### `AllOf::TEMPLATE_SOME`
 
-| Mode       | Template                               |
-|------------|----------------------------------------|
-| `default`  | These rules must pass for {{name}}     |
-| `inverted` | These rules must not pass for {{name}} |
+Used when some rules must be failed.
 
-### `AllOf::TEMPLATE_NONE`
+| Mode       | Template                     |
+|------------|------------------------------|
+| `default`  | {{name}} must pass the rules |
+| `inverted` | {{name}} must pass the rules |
 
-| Mode       | Template                                      |
-|------------|-----------------------------------------------|
-| `default`  | All the required rules must pass for {{name}} |
-| `inverted` | None of these rules must pass for {{name}}    |
+### `AllOf::TEMPLATE_ALL`
+
+Used when all rules have failed.
+
+| Mode       | Template                         |
+|------------|----------------------------------|
+| `default`  | {{name}} must pass all the rules |
+| `inverted` | {{name}} must pass all the rules |
 
 ## Template placeholders
 

diff --git a/docs/rules/AnyOf.md b/docs/rules/AnyOf.md
@@ -17,10 +17,10 @@ so `AnyOf()` returns true.
 
 ### `AnyOf::TEMPLATE_STANDARD`
 
-| Mode       | Template                                               |
-|------------|--------------------------------------------------------|
-| `default`  | At least one of these rules must pass for {{name}}     |
-| `inverted` | At least one of these rules must not pass for {{name}} |
+| Mode       | Template                                     |
+|------------|----------------------------------------------|
+| `default`  | {{name}} must pass at least one of the rules |
+| `inverted` | {{name}} must pass at least one of the rules |
 
 ## Template placeholders
 

diff --git a/docs/rules/Attributes.md b/docs/rules/Attributes.md
@@ -49,10 +49,10 @@ v::attributes()->assert(new Person('John Doe', '[email protected]', '2020-06-23
 
 v::attributes()->assert(new Person('', 'not an email', 'not a date', 'not a phone number'));
 // Full message:
-// - All the required rules must pass for `Person { +$name="" +$email="not an email" +$birthdate="not a date" +$phone="not a phone number" }`
+// - `Person { +$name="" +$email="not an email" +$birthdate="not a date" +$phone="not a phone number" }` must pass all the rules
 //   - name must not be empty
 //   - email must be a valid email address
-//   - All the required rules must pass for birthdate
+//   - birthdate must pass all the rules
 //     - birthdate must be a valid date in the format "2005-12-30"
 //     - For comparison with now, birthdate must be a valid datetime
 //   - phone must be a valid telephone number or must be null

diff --git a/docs/rules/NoneOf.md b/docs/rules/NoneOf.md
@@ -15,12 +15,23 @@ In the sample above, 'foo' isn't a integer nor a float, so noneOf returns true.
 
 ## Templates
 
-### `NoneOf::TEMPLATE_STANDARD`
+### `NoneOf::TEMPLATE_SOME`
 
-| Mode       | Template                                   |
-|------------|--------------------------------------------|
-| `default`  | None of these rules must pass for {{name}} |
-| `inverted` | All of these rules must pass for {{name}}  |
+Used when some rules have passed.
+
+| Mode       | Template                     |
+|------------|------------------------------|
+| `default`  | {{name}} must pass the rules |
+| `inverted` | {{name}} must pass the rules |
+
+### `NoneOf::TEMPLATE_ALL`
+
+Used when all rules have passed.
+
+| Mode       | Template                         |
+|------------|----------------------------------|
+| `default`  | {{name}} must pass all the rules |
+| `inverted` | {{name}} must pass all the rules |
 
 ## Template placeholders
 

diff --git a/docs/rules/OneOf.md b/docs/rules/OneOf.md
@@ -16,12 +16,23 @@ character, one or the other, but not neither nor both.
 
 ## Templates
 
-### `OneOf::TEMPLATE_STANDARD`
+### `OneOf::TEMPLATE_NONE`
 
-| Mode       | Template                                           |
-|------------|----------------------------------------------------|
-| `default`  | Only one of these rules must pass for {{name}}     |
-| `inverted` | Only one of these rules must not pass for {{name}} |
+Used when none of the rules have passed.
+
+| Mode       | Template                            |
+|------------|-------------------------------------|
+| `default`  | {{name}} must pass one of the rules |
+| `inverted` | {{name}} must pass one of the rules |
+
+### `OneOf::TEMPLATE_MORE_THAN_ONE`
+
+Used when more than one rule has passed.
+
+| Mode       | Template                                 |
+|------------|------------------------------------------|
+| `default`  | {{name}} must pass only one of the rules |
+| `inverted` | {{name}} must pass only one of the rules |
 
 ## Template placeholders
 

diff --git a/library/Rules/AllOf.php b/library/Rules/AllOf.php
@@ -22,18 +22,18 @@
 
 #[Attribute(Attribute::TARGET_PROPERTY | Attribute::IS_REPEATABLE)]
 #[Template(
-    'These rules must pass for {{name}}',
-    'These rules must not pass for {{name}}',
+    '{{name}} must pass the rules',
+    '{{name}} must pass the rules',
     self::TEMPLATE_SOME,
 )]
 #[Template(
-    'All the required rules must pass for {{name}}',
-    'None of these rules must pass for {{name}}',
-    self::TEMPLATE_NONE,
+    '{{name}} must pass all the rules',
+    '{{name}} must pass all the rules',
+    self::TEMPLATE_ALL,
 )]
 final class AllOf extends Composite
 {
-    public const TEMPLATE_NONE = '__none__';
+    public const TEMPLATE_ALL = '__all__';
     public const TEMPLATE_SOME = '__some__';
 
     public function evaluate(mixed $input): Result
@@ -43,7 +43,7 @@ public function evaluate(mixed $input): Result
         $failed = array_filter($children, static fn (Result $result): bool => !$result->isValid);
         $template = self::TEMPLATE_SOME;
         if (count($children) === count($failed)) {
-            $template = self::TEMPLATE_NONE;
+            $template = self::TEMPLATE_ALL;
         }
 
         return (new Result($valid, $input, $this, [], $template))->withChildren(...$children);

diff --git a/library/Rules/AnyOf.php b/library/Rules/AnyOf.php
@@ -20,8 +20,8 @@
 
 #[Attribute(Attribute::TARGET_PROPERTY | Attribute::IS_REPEATABLE)]
 #[Template(
-    'At least one of these rules must pass for {{name}}',
-    'At least one of these rules must not pass for {{name}}',
+    '{{name}} must pass at least one of the rules',
+    '{{name}} must pass at least one of the rules',
 )]
 final class AnyOf extends Composite
 {

diff --git a/library/Rules/NoneOf.php b/library/Rules/NoneOf.php
@@ -15,24 +15,37 @@
 use Respect\Validation\Rule;
 use Respect\Validation\Rules\Core\Composite;
 
+use function array_filter;
 use function array_map;
 use function array_reduce;
+use function count;
 
 #[Attribute(Attribute::TARGET_PROPERTY | Attribute::IS_REPEATABLE)]
 #[Template(
-    'None of these rules must pass for {{name}}',
-    'All of these rules must pass for {{name}}',
+    '{{name}} must pass the rules',
+    '{{name}} must pass the rules',
+    self::TEMPLATE_SOME,
+)]
+#[Template(
+    '{{name}} must pass all the rules',
+    '{{name}} must pass all the rules',
+    self::TEMPLATE_ALL,
 )]
 final class NoneOf extends Composite
 {
+    public const TEMPLATE_ALL = '__all__';
+    public const TEMPLATE_SOME = '__some__';
+
     public function evaluate(mixed $input): Result
     {
-        $children = array_map(
-            static fn (Rule $rule) => $rule->evaluate($input)->withInvertedMode(),
-            $this->rules
-        );
+        $children = array_map(static fn (Rule $rule) => $rule->evaluate($input)->withInvertedMode(), $this->rules);
         $valid = array_reduce($children, static fn (bool $carry, Result $result) => $carry && $result->isValid, true);
+        $failed = array_filter($children, static fn (Result $result): bool => !$result->isValid);
+        $template = self::TEMPLATE_SOME;
+        if (count($children) === count($failed)) {
+            $template = self::TEMPLATE_ALL;
+        }
 
-        return (new Result($valid, $input, $this))->withChildren(...$children);
+        return (new Result($valid, $input, $this, [], $template))->withChildren(...$children);
     }
 }
diff --git a/library/Rules/OneOf.php b/library/Rules/OneOf.php
@@ -15,21 +15,45 @@
 use Respect\Validation\Rule;
 use Respect\Validation\Rules\Core\Composite;
 
+use function array_filter;
 use function array_map;
 use function array_reduce;
+use function count;
+use function usort;
 
 #[Attribute(Attribute::TARGET_PROPERTY | Attribute::IS_REPEATABLE)]
 #[Template(
-    'Only one of these rules must pass for {{name}}',
-    'Only one of these rules must not pass for {{name}}',
+    '{{name}} must pass one of the rules',
+    '{{name}} must pass one of the rules',
+    self::TEMPLATE_NONE,
+)]
+#[Template(
+    '{{name}} must pass only one of the rules',
+    '{{name}} must pass only one of the rules',
+    self::TEMPLATE_MORE_THAN_ONE,
 )]
 final class OneOf extends Composite
 {
+    public const TEMPLATE_NONE = '__none__';
+    public const TEMPLATE_MORE_THAN_ONE = '__more_than_one__';
+
     public function evaluate(mixed $input): Result
     {
         $children = array_map(static fn (Rule $rule) => $rule->evaluate($input), $this->rules);
         $valid = array_reduce($children, static fn (bool $carry, Result $result) => $carry xor $result->isValid, false);
+        $validChildren = array_filter($children, static fn (Result $result): bool => $result->isValid);
+        $template = self::TEMPLATE_NONE;
+        if (count($validChildren) > 1) {
+            // Put the failed children at the top, so it makes sense in the main error message
+            usort($children, static fn (Result $a, Result $b): int => $a->isValid <=> $b->isValid);
+
+            $template = self::TEMPLATE_MORE_THAN_ONE;
+            $children = array_map(
+                static fn (Result $child) => $child->isValid ? $child->withInvertedValidation() : $child,
+                $children
+            );
+        }
 
-        return (new Result($valid, $input, $this))->withChildren(...$children);
+        return (new Result($valid, $input, $this, [], $template))->withChildren(...$children);
     }
 }
diff --git a/tests/feature/AssertWithKeysTest.php b/tests/feature/AssertWithKeysTest.php
@@ -37,13 +37,13 @@
             ],
         ]),
     <<<'FULL_MESSAGE'
-    - All the required rules must pass for the given data
-      - All the required rules must pass for mysql
+    - the given data must pass all the rules
+      - mysql must pass all the rules
         - host must be a string
         - user must be present
         - password must be present
         - schema must be a string
-      - All the required rules must pass for postgresql
+      - postgresql must pass all the rules
         - host must be present
         - user must be a string
         - password must be a string

diff --git a/tests/feature/AssertWithPropertiesTest.php b/tests/feature/AssertWithPropertiesTest.php
@@ -45,10 +45,10 @@ function (): void {
             ->assert($object);
     },
     <<<'FULL_MESSAGE'
-    - All the required rules must pass for the given data
-      - These rules must pass for mysql
+    - the given data must pass all the rules
+      - mysql must pass the rules
         - host must be a string
-      - These rules must pass for postgresql
+      - postgresql must pass the rules
         - user must be a string
     FULL_MESSAGE,
 ));
diff --git a/tests/feature/GetFullMessageShouldIncludeAllValidationMessagesInAChainTest.php b/tests/feature/GetFullMessageShouldIncludeAllValidationMessagesInAChainTest.php
@@ -12,7 +12,7 @@
 test('Scenario #1', expectFullMessage(
     fn() => Validator::stringType()->lengthBetween(2, 15)->assert(0),
     <<<'FULL_MESSAGE'
-    - All the required rules must pass for 0
+    - 0 must pass all the rules
       - 0 must be a string
       - 0 must be a countable value or a string
     FULL_MESSAGE,

diff --git a/tests/feature/GetMessagesShouldIncludeAllValidationMessagesInAChainTest.php b/tests/feature/GetMessagesShouldIncludeAllValidationMessagesInAChainTest.php
@@ -20,7 +20,7 @@ function (): void {
             ->assert(['username' => 'u', 'birthdate' => 'Not a date', 'password' => '']);
     },
     [
-        '__root__' => 'All the required rules must pass for `["username": "u", "birthdate": "Not a date", "password": ""]`',
+        '__root__' => '`["username": "u", "birthdate": "Not a date", "password": ""]` must pass all the rules',
         'username' => 'The length of username must be between 2 and 32',
         'birthdate' => 'birthdate must be a valid date/time',
         'password' => 'password must not be empty',

diff --git a/tests/feature/GetMessagesTest.php b/tests/feature/GetMessagesTest.php
@@ -20,16 +20,16 @@
             ->key('schema', v::stringType()))
         ->assert(['mysql' => ['host' => 42, 'schema' => 42], 'postgresql' => ['user' => 42, 'password' => 42]]),
     [
-        '__root__' => 'All the required rules must pass for `["mysql": ["host": 42, "schema": 42], "postgresql": ["user": 42, "password": 42]]`',
+        '__root__' => '`["mysql": ["host": 42, "schema": 42], "postgresql": ["user": 42, "password": 42]]` must pass all the rules',
         'mysql' => [
-            '__root__' => 'All the required rules must pass for mysql',
+            '__root__' => 'mysql must pass all the rules',
             'host' => 'host must be a string',
             'user' => 'user must be present',
             'password' => 'password must be present',
             'schema' => 'schema must be a string',
         ],
         'postgresql' => [
-            '__root__' => 'All the required rules must pass for postgresql',
+            '__root__' => 'postgresql must pass all the rules',
             'host' => 'host must be present',
             'user' => 'user must be a string',
             'password' => 'password must be a string',

diff --git a/tests/feature/GetMessagesWithReplacementsTest.php b/tests/feature/GetMessagesWithReplacementsTest.php
@@ -49,16 +49,16 @@ function (): void {
             );
     },
     [
-        '__root__' => 'All the required rules must pass for `["mysql": ["host": 42, "schema": 42], "postgresql": ["user": 42, "password": 42]]`',
+        '__root__' => '`["mysql": ["host": 42, "schema": 42], "postgresql": ["user": 42, "password": 42]]` must pass all the rules',
         'mysql' => [
-            '__root__' => 'All the required rules must pass for mysql',
+            '__root__' => 'mysql must pass all the rules',
             'host' => '`host` should be a MySQL host',
             'user' => 'Value should be a MySQL username',
             'password' => 'password must be present',
             'schema' => 'schema must be a string',
         ],
         'postgresql' => [
-            '__root__' => 'All the required rules must pass for postgresql',
+            '__root__' => 'postgresql must pass all the rules',
             'host' => 'host must be present',
             'user' => 'user must be a string',
             'password' => 'password must be a string',

diff --git a/tests/feature/Issues/Issue1289Test.php b/tests/feature/Issues/Issue1289Test.php
@@ -47,17 +47,17 @@
         ]),
     'default must be a string',
     <<<'FULL_MESSAGE'
-    - These rules must pass for `["default": 2, "description": [], "children": ["nope"]]`
-      - Only one of these rules must pass for default
+    - `["default": 2, "description": [], "children": ["nope"]]` must pass the rules
+      - default must pass one of the rules
         - default must be a string
         - default must be a boolean
       - description must be a string value
     FULL_MESSAGE,
     [
         0 => [
-            '__root__' => 'These rules must pass for `["default": 2, "description": [], "children": ["nope"]]`',
+            '__root__' => '`["default": 2, "description": [], "children": ["nope"]]` must pass the rules',
             'default' => [
-                '__root__' => 'Only one of these rules must pass for default',
+                '__root__' => 'default must pass one of the rules',
                 'stringType' => 'default must be a string',
                 'boolType' => 'default must be a boolean',
             ],

diff --git a/tests/feature/Issues/Issue1333Test.php b/tests/feature/Issues/Issue1333Test.php
@@ -11,12 +11,12 @@
     fn() => v::noWhitespace()->email()->setName('User Email')->assert('not email'),
     'User Email must not contain whitespaces',
     <<<'FULL_MESSAGE'
-    - All the required rules must pass for User Email
+    - User Email must pass all the rules
       - User Email must not contain whitespaces
       - User Email must be a valid email address
     FULL_MESSAGE,
     [
-        '__root__' => 'All the required rules must pass for User Email',
+        '__root__' => 'User Email must pass all the rules',
         'noWhitespace' => 'User Email must not contain whitespaces',
         'email' => 'User Email must be a valid email address',
     ],