Do not overwrite "IDs" when updating names

When you have a chain of rules in the Validation and overwrite the name with "setName()," it's impossible to get the messages from all rules in the chain as an array because they all have the same name. These changes will change that behavior by creating a more explicit distinction between "IDs" and "names." The "IDs" will remain unchangeable, while we can always overwrite the names. That means that the array messages will look more similar to the chain, and it will be possible to overwrite the messages from multiple rules in the same chain. Signed-off-by: Henrique Moody <[email protected]>
Respect · Mar 26, 2024 · 8573bc5 · 8573bc5
1 parent 9322cd6
commit 8573bc5
Show file tree

Hide file tree

Showing 25 changed files with 102 additions and 77 deletions.
diff --git a/library/Result.php b/library/Result.php
@@ -43,7 +43,7 @@ public function __construct(
  ) {
  $this->name = $rule->getName() ?? $name;
  $this->template = $rule->getTemplate() ?? $template;
- $this->id = $id ?? $this->name ?? lcfirst(substr((string) strrchr($rule::class, '\\'), 1));
+ $this->id = $id ?? lcfirst(substr((string) strrchr($rule::class, '\\'), 1));
  $this->children = $children;
  }
 

diff --git a/library/Rules/Key.php b/library/Rules/Key.php
@@ -38,10 +38,13 @@ public function evaluate(mixed $input): Result
  return $keyExistsResult;
  }
 
- $child = $this->rule->evaluate($input[$this->key]);
+ $child = $this->rule
+ ->evaluate($input[$this->key])
+ ->withId((string) $this->key);
 
  return (new Result($child->isValid, $input, $this))
  ->withChildren($child)
+ ->withId((string) $this->key)
  ->withNameIfMissing($this->rule->getName() ?? (string) $this->key);
  }
 }
diff --git a/library/Rules/KeyExists.php b/library/Rules/KeyExists.php
@@ -30,7 +30,7 @@ public function __construct(
 
  public function evaluate(mixed $input): Result
  {
- return new Result($this->hasKey($input), $input, $this, name: (string) $this->key);
+ return new Result($this->hasKey($input), $input, $this, name: (string) $this->key, id: (string) $this->key);
  }
 
  private function hasKey(mixed $input): bool

diff --git a/library/Rules/KeyOptional.php b/library/Rules/KeyOptional.php
@@ -33,10 +33,13 @@ public function evaluate(mixed $input): Result
  return $keyExistsResult->withInvertedMode();
  }
 
- $child = $this->rule->evaluate($input[$this->key]);
+ $child = $this->rule
+ ->evaluate($input[$this->key])
+ ->withId((string) $this->key);
 
  return (new Result($child->isValid, $input, $this))
  ->withChildren($child)
+ ->withId((string) $this->key)
  ->withNameIfMissing($this->rule->getName() ?? (string) $this->key);
  }
 }
diff --git a/library/Rules/Property.php b/library/Rules/Property.php
@@ -35,10 +35,13 @@ public function evaluate(mixed $input): Result
  return $propertyExistsResult;
  }
 
- $childResult = $this->rule->evaluate($this->extractPropertyValue($input, $this->propertyName));
+ $childResult = $this->rule
+ ->evaluate($this->extractPropertyValue($input, $this->propertyName))
+ ->withId($this->propertyName);
 
  return (new Result($childResult->isValid, $input, $this))
  ->withChildren($childResult)
+ ->withId($this->propertyName)
  ->withNameIfMissing($this->rule->getName() ?? $this->propertyName);
  }
 }
diff --git a/library/Rules/PropertyExists.php b/library/Rules/PropertyExists.php
@@ -30,11 +30,17 @@ public function __construct(
  public function evaluate(mixed $input): Result
  {
  if (!is_object($input)) {
- return Result::failed($input, $this)->withNameIfMissing($this->propertyName);
+ return Result::failed($input, $this)->withNameIfMissing($this->propertyName)->withId($this->propertyName);
  }
 
  $reflection = new ReflectionObject($input);
 
- return new Result($reflection->hasProperty($this->propertyName), $input, $this, name: $this->propertyName);
+ return new Result(
+ $reflection->hasProperty($this->propertyName),
+ $input,
+ $this,
+ name: $this->propertyName,
+ id: $this->propertyName
+ );
  }
 }
diff --git a/library/Rules/PropertyOptional.php b/library/Rules/PropertyOptional.php
@@ -35,10 +35,13 @@ public function evaluate(mixed $input): Result
  return $propertyExistsResult->withInvertedMode();
  }
 
- $childResult = $this->rule->evaluate($this->extractPropertyValue($input, $this->propertyName));
+ $childResult = $this->rule
+ ->evaluate($this->extractPropertyValue($input, $this->propertyName))
+ ->withId($this->propertyName);
 
  return (new Result($childResult->isValid, $input, $this))
  ->withChildren($childResult)
+ ->withId($this->propertyName)
  ->withNameIfMissing($this->rule->getName() ?? $this->propertyName);
  }
 }
diff --git a/tests/integration/issue-1244.phpt b/tests/integration/issue-1244.phpt
@@ -11,5 +11,5 @@ exceptionMessages(static fn () => v::key('firstname', v::notBlank()->setName('Fi
 ?>
 --EXPECTF--
 [
- 'First Name' => 'First Name must be present',
+ 'firstname' => 'First Name must be present',
 ]
diff --git a/tests/integration/issue-1333.phpt b/tests/integration/issue-1333.phpt
@@ -0,0 +1,17 @@
+--FILE--
+<?php
+
+declare(strict_types=1);
+
+use Respect\Validation\Validator as v;
+
+require 'vendor/autoload.php';
+
+exceptionMessages(static fn() => v::noWhitespace()->email()->setName('User Email')->assert('not email'));
+?>
+--EXPECT--
+[
+ '__root__' => 'All of the required rules must pass for User Email',
+ 'noWhitespace' => 'User Email must not contain whitespace',
+ 'email' => 'User Email must be valid email',
+]
diff --git a/tests/integration/rules/betweenExclusive.phpt b/tests/integration/rules/betweenExclusive.phpt
@@ -44,6 +44,6 @@ With name
 Range must be greater than 1 and less than 10
 - Range must be greater than 1 and less than 10
 [
- 'Range' => 'Range must be greater than 1 and less than 10',
+ 'betweenExclusive' => 'Range must be greater than 1 and less than 10',
 ]
 
diff --git a/tests/integration/rules/consecutive.phpt b/tests/integration/rules/consecutive.phpt
@@ -83,39 +83,39 @@ With wrapped name, default
 Wrapped must evaluate to `true`
 - Wrapped must evaluate to `true`
 [
- 'Wrapped' => 'Wrapped must evaluate to `true`',
+ 'trueVal' => 'Wrapped must evaluate to `true`',
 ]
 
 With wrapper name, default
 ⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺
 Wrapper must evaluate to `true`
 - Wrapper must evaluate to `true`
 [
- 'Wrapper' => 'Wrapper must evaluate to `true`',
+ 'trueVal' => 'Wrapper must evaluate to `true`',
 ]
 
 With the name set in the wrapped rule of an inverted failing rule
 ⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺
 Wrapped must not evaluate to `true`
 - Wrapped must not evaluate to `true`
 [
- 'Wrapped' => 'Wrapped must not evaluate to `true`',
+ 'trueVal' => 'Wrapped must not evaluate to `true`',
 ]
 
 With the name set in an inverted failing rule
 ⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺
 Not must not evaluate to `true`
 - Not must not evaluate to `true`
 [
- 'Not' => 'Not must not evaluate to `true`',
+ 'trueVal' => 'Not must not evaluate to `true`',
 ]
 
 With the name set in the "consecutive" that has an inverted failing rule
 ⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺
 Wrapper must not evaluate to `true`
 - Wrapper must not evaluate to `true`
 [
- 'Wrapper' => 'Wrapper must not evaluate to `true`',
+ 'trueVal' => 'Wrapper must not evaluate to `true`',
 ]
 
 With template
@@ -141,3 +141,4 @@ subdivisionCode must be a subdivision code of Brazil
 [
  'subdivisionCode' => 'subdivisionCode must be a subdivision code of Brazil',
 ]
+
diff --git a/tests/integration/rules/each.phpt b/tests/integration/rules/each.phpt
@@ -138,9 +138,9 @@ Wrapped must be of type integer
  - Wrapped must be of type integer
 [
  '__root__' => 'Each item in Wrapped must be valid',
- 'Wrapped.1' => 'Wrapped must be of type integer',
- 'Wrapped.2' => 'Wrapped must be of type integer',
- 'Wrapped.3' => 'Wrapped must be of type integer',
+ 'intType.1' => 'Wrapped must be of type integer',
+ 'intType.2' => 'Wrapped must be of type integer',
+ 'intType.3' => 'Wrapped must be of type integer',
 ]
 
 With name, negative
@@ -152,9 +152,9 @@ Wrapped must not be of type integer
  - Wrapped must not be of type integer
 [
  '__root__' => 'Each item in Wrapped must not validate',
- 'Wrapped.1' => 'Wrapped must not be of type integer',
- 'Wrapped.2' => 'Wrapped must not be of type integer',
- 'Wrapped.3' => 'Wrapped must not be of type integer',
+ 'intType.1' => 'Wrapped must not be of type integer',
+ 'intType.2' => 'Wrapped must not be of type integer',
+ 'intType.3' => 'Wrapped must not be of type integer',
 ]
 
 With wrapper name, default
@@ -166,9 +166,9 @@ Wrapper must be of type integer
  - Wrapper must be of type integer
 [
  '__root__' => 'Each item in Wrapper must be valid',
- 'Wrapper.1' => 'Wrapper must be of type integer',
- 'Wrapper.2' => 'Wrapper must be of type integer',
- 'Wrapper.3' => 'Wrapper must be of type integer',
+ 'intType.1' => 'Wrapper must be of type integer',
+ 'intType.2' => 'Wrapper must be of type integer',
+ 'intType.3' => 'Wrapper must be of type integer',
 ]
 
 With wrapper name, negative
@@ -180,9 +180,9 @@ Wrapper must not be of type integer
  - Wrapper must not be of type integer
 [
  '__root__' => 'Each item in Wrapper must not validate',
- 'Wrapper.1' => 'Wrapper must not be of type integer',
- 'Wrapper.2' => 'Wrapper must not be of type integer',
- 'Wrapper.3' => 'Wrapper must not be of type integer',
+ 'intType.1' => 'Wrapper must not be of type integer',
+ 'intType.2' => 'Wrapper must not be of type integer',
+ 'intType.3' => 'Wrapper must not be of type integer',
 ]
 
 With Not name, negative
@@ -194,9 +194,9 @@ Not must not be of type integer
  - Not must not be of type integer
 [
  '__root__' => 'Each item in Not must not validate',
- 'Not.1' => 'Not must not be of type integer',
- 'Not.2' => 'Not must not be of type integer',
- 'Not.3' => 'Not must not be of type integer',
+ 'intType.1' => 'Not must not be of type integer',
+ 'intType.2' => 'Not must not be of type integer',
+ 'intType.3' => 'Not must not be of type integer',
 ]
 
 With template, non-iterable
@@ -247,15 +247,14 @@ First item should have been an integer
 
 With array template and name, default
 ⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺
-First item should have been an integer
-- Here a sequence of items that did not pass the validation
- - First item should have been an integer
- - Second item should have been an integer
- - Third item should have been an integer
+Wrapped must be of type integer
+- Each item in Wrapped must be valid
+ - Wrapped must be of type integer
+ - Wrapped must be of type integer
+ - Wrapped must be of type integer
 [
- '__root__' => 'Here a sequence of items that did not pass the validation',
- 'Wrapped.1' => 'First item should have been an integer',
- 'Wrapped.2' => 'Second item should have been an integer',
- 'Wrapped.3' => 'Third item should have been an integer',
+ '__root__' => 'Each item in Wrapped must be valid',
+ 'intType.1' => 'Wrapped must be of type integer',
+ 'intType.2' => 'Wrapped must be of type integer',
+ 'intType.3' => 'Wrapped must be of type integer',
 ]
-
diff --git a/tests/integration/rules/hetu.phpt b/tests/integration/rules/hetu.phpt
@@ -44,6 +44,5 @@ With name
 Hetu must be a valid Finnish personal identity code
 - Hetu must be a valid Finnish personal identity code
 [
- 'Hetu' => 'Hetu must be a valid Finnish personal identity code',
+ 'hetu' => 'Hetu must be a valid Finnish personal identity code',
 ]
-
diff --git a/tests/integration/rules/iterableType.phpt b/tests/integration/rules/iterableType.phpt
@@ -44,6 +44,5 @@ With name
 Options must be of type iterable
 - Options must be of type iterable
 [
- 'Options' => 'Options must be of type iterable',
+ 'iterableType' => 'Options must be of type iterable',
 ]
-
diff --git a/tests/integration/rules/key.phpt b/tests/integration/rules/key.phpt
@@ -90,23 +90,23 @@ With wrapped name, missing key
 Wrapped must be present
 - Wrapped must be present
 [
- 'Wrapped' => 'Wrapped must be present',
+ 'foo' => 'Wrapped must be present',
 ]
 
 With wrapped name, default
 ⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺
 Wrapped must be of type integer
 - Wrapped must be of type integer
 [
- 'Wrapped' => 'Wrapped must be of type integer',
+ 'foo' => 'Wrapped must be of type integer',
 ]
 
 With wrapped name, negative
 ⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺
 Wrapped must not be of type integer
 - Wrapped must not be of type integer
 [
- 'Wrapped' => 'Wrapped must not be of type integer',
+ 'foo' => 'Wrapped must not be of type integer',
 ]
 
 With wrapper name, default
@@ -156,4 +156,3 @@ No off-key key
 [
  'foo' => 'No off-key key',
 ]
-
diff --git a/tests/integration/rules/keyExists.phpt b/tests/integration/rules/keyExists.phpt
@@ -36,7 +36,7 @@ Custom name
 Custom name must be present
 - Custom name must be present
 [
- 'Custom name' => 'Custom name must be present',
+ 'foo' => 'Custom name must be present',
 ]
 
 Custom template
@@ -46,4 +46,3 @@ Custom template for `foo`
 [
  'foo' => 'Custom template for `foo`',
 ]
-
diff --git a/tests/integration/rules/keyOptional.phpt b/tests/integration/rules/keyOptional.phpt
@@ -73,15 +73,15 @@ With wrapped name, default
 Wrapped must be of type integer
 - Wrapped must be of type integer
 [
- 'Wrapped' => 'Wrapped must be of type integer',
+ 'foo' => 'Wrapped must be of type integer',
 ]
 
 With wrapped name, negative
 ⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺
 Wrapped must not be of type integer
 - Wrapped must not be of type integer
 [
- 'Wrapped' => 'Wrapped must not be of type integer',
+ 'foo' => 'Wrapped must not be of type integer',
 ]
 
 With wrapper name, default
@@ -123,4 +123,3 @@ No off-key key
 [
  'foo' => 'No off-key key',
 ]
-