Only pass the necessary parameters to the exceptions

Currently, we convert the properties of a rule into parameters and pass them to the exceptions. That complicates things for a few reasons: 1. The exception knows too much: there's a lot of information in an object, and the exception would only need a few parameters to work correctly. 2. Any variable change becomes a backward compatibility break: if we change the name of the variable type in a rule, even if it's a private one, we may need to change the template, which is a backward compatibility break. 3. The factory is bloated because of introspection tricks: it reads the properties from the class, even from the parent, and then passes it to the exception. Of course, that means we introduce another method to `Validatable`, but in most cases, extending `AbstractRule` is enough to create a new rule. Signed-off-by: Henrique Moody <[email protected]>
Respect · Jan 29, 2024 · 2b59e3d · 2b59e3d
1 parent dd896bb
commit 2b59e3d
Show file tree

Hide file tree

Showing 43 changed files with 298 additions and 106 deletions.
diff --git a/library/Factory.php b/library/Factory.php
@@ -128,7 +128,7 @@ public function exception(Validatable $validatable, mixed $input, array $extraPa
  $formatter = new Formatter($this->translator, $this->parameterStringifier);
  $reflection = new ReflectionObject($validatable);
  $ruleName = $reflection->getShortName();
- $params = ['input' => $input] + $extraParams + $this->extractPropertiesValues($validatable, $reflection);
+ $params = ['input' => $input] + $extraParams + $validatable->getParams();
  $id = lcfirst($ruleName);
  if ($validatable->getName() !== null) {
  $id = $params['name'] = $validatable->getName();
@@ -181,32 +181,4 @@ private function createReflectionClass(string $name, string $parentName): Reflec
 
  return $reflection;
  }
-
- /**
- * @param ReflectionObject|ReflectionClass<Validatable> $reflection
- * @return mixed[]
- */
- private function extractPropertiesValues(Validatable $validatable, ReflectionClass $reflection): array
- {
- $values = [];
- foreach ($reflection->getProperties() as $property) {
- if (!$property->isInitialized($validatable)) {
- continue;
- }
-
- $propertyValue = $property->getValue($validatable);
- if ($propertyValue === null) {
- continue;
- }
-
- $values[$property->getName()] = $propertyValue;
- }
-
- $parentReflection = $reflection->getParentClass();
- if ($parentReflection !== false) {
- return $values + $this->extractPropertiesValues($validatable, $parentReflection);
- }
-
- return $values;
- }
 }
diff --git a/library/Rules/AbstractAge.php b/library/Rules/AbstractAge.php
@@ -45,6 +45,14 @@ public function validate(mixed $input): bool
  return $this->isValidWithFormat($this->format, (string) $input);
  }
 
+ /**
+ * @return array<string, int>
+ */
+ public function getParams(): array
+ {
+ return ['age' => $this->age];
+ }
+
  private function isValidWithoutFormat(string $dateTime): bool
  {
  $timestamp = strtotime($dateTime);

diff --git a/library/Rules/AbstractComparison.php b/library/Rules/AbstractComparison.php
@@ -35,4 +35,12 @@ public function validate(mixed $input): bool
 
  return $this->compare($left, $right);
  }
+
+ /**
+ * @return array<string, mixed>
+ */
+ public function getParams(): array
+ {
+ return ['compareTo' => $this->compareTo];
+ }
 }
diff --git a/library/Rules/AbstractFilterRule.php b/library/Rules/AbstractFilterRule.php
@@ -48,6 +48,14 @@ public function getTemplate(mixed $input): string
  return $this->template ?? ($this->additionalChars ? self::TEMPLATE_EXTRA : self::TEMPLATE_STANDARD);
  }
 
+ /**
+ * @return array<string, mixed>
+ */
+ public function getParams(): array
+ {
+ return ['additionalChars' => $this->additionalChars];
+ }
+
  private function filter(string $input): string
  {
  return str_replace(str_split($this->additionalChars), '', $input);

diff --git a/library/Rules/AbstractRule.php b/library/Rules/AbstractRule.php
@@ -65,6 +65,14 @@ public function getTemplate(mixed $input): string
  return $this->template ?? self::TEMPLATE_STANDARD;
  }
 
+ /**
+ * @return array<string, mixed>
+ */
+ public function getParams(): array
+ {
+ return [];
+ }
+
  public function __invoke(mixed $input): bool
  {
  return $this->validate($input);

diff --git a/library/Rules/Base.php b/library/Rules/Base.php
@@ -34,4 +34,12 @@ public function validate(mixed $input): bool
 
  return (bool) preg_match('@^[' . $valid . ']+$@', (string) $input);
  }
+
+ /**
+ * @return array<string, int>
+ */
+ public function getParams(): array
+ {
+ return ['base' => $this->base];
+ }
 }
diff --git a/library/Rules/Call.php b/library/Rules/Call.php
@@ -72,6 +72,14 @@ public function validate(mixed $input): bool
  return true;
  }
 
+ /**
+ * @return array<string, callable>
+ */
+ public function getParams(): array
+ {
+ return ['callable' => $this->callable];
+ }
+
  private function setErrorHandler(mixed $input): void
  {
  set_error_handler(function () use ($input): void {

diff --git a/library/Rules/Charset.php b/library/Rules/Charset.php
@@ -40,4 +40,12 @@ public function validate(mixed $input): bool
  {
  return in_array(mb_detect_encoding($input, $this->charset, true), $this->charset, true);
  }
+
+ /**
+ * @return array<string, array<string>>
+ */
+ public function getParams(): array
+ {
+ return ['charset' => $this->charset];
+ }
 }
diff --git a/library/Rules/Contains.php b/library/Rules/Contains.php
@@ -36,6 +36,14 @@ public function validate(mixed $input): bool
  return $this->validateString((string) $input, (string) $this->containsValue);
  }
 
+ /**
+ * @return array<string, mixed>
+ */
+ public function getParams(): array
+ {
+ return ['containsValue' => $this->containsValue];
+ }
+
  private function validateString(string $haystack, string $needle): bool
  {
  if ($needle === '') {

diff --git a/library/Rules/CreditCard.php b/library/Rules/CreditCard.php
@@ -92,4 +92,12 @@ public function getTemplate(mixed $input): string
 
  return self::TEMPLATE_BRANDED;
  }
+
+ /**
+ * @return array<string, string>
+ */
+ public function getParams(): array
+ {
+ return ['brand' => $this->brand];
+ }
 }
diff --git a/library/Rules/Date.php b/library/Rules/Date.php
@@ -22,8 +22,6 @@ final class Date extends AbstractRule
 {
  use CanValidateDateTime;
 
- private readonly string $sample;
-
  /**
  * @throws ComponentException
  */
@@ -33,8 +31,6 @@ public function __construct(
  if (!preg_match('/^[djSFmMnYy\W]+$/', $format)) {
  throw new ComponentException(sprintf('"%s" is not a valid date format', $format));
  }
-
- $this->sample = date($format, strtotime('2005-12-30'));
  }
 
  public function validate(mixed $input): bool
@@ -45,4 +41,12 @@ public function validate(mixed $input): bool
 
  return $this->isDateTime($this->format, (string) $input);
  }
+
+ /**
+ * @return array<string, mixed>
+ */
+ public function getParams(): array
+ {
+ return ['sample' => date($this->format, strtotime('2005-12-30'))];
+ }
 }
diff --git a/library/Rules/DateTime.php b/library/Rules/DateTime.php
@@ -22,12 +22,9 @@ final class DateTime extends AbstractRule
 
  public const TEMPLATE_FORMAT = 'format';
 
- private readonly string $sample;
-
  public function __construct(
  private readonly ?string $format = null
  ) {
- $this->sample = date($format ?: 'c', strtotime('2005-12-30 01:02:03'));
  }
 
  public function validate(mixed $input): bool
@@ -51,4 +48,12 @@ public function getTemplate(mixed $input): string
  {
  return $this->template ?? ($this->format !== null ? self::TEMPLATE_FORMAT : self::TEMPLATE_STANDARD);
  }
+
+ /**
+ * @return array<string, mixed>
+ */
+ public function getParams(): array
+ {
+ return ['sample' => date($this->format ?: 'c', strtotime('2005-12-30 01:02:03'))];
+ }
 }
diff --git a/library/Rules/Decimal.php b/library/Rules/Decimal.php
@@ -31,6 +31,14 @@ public function validate(mixed $input): bool
  return $this->toFormattedString($input) === $this->toRawString($input);
  }
 
+ /**
+ * @return array<string, int>
+ */
+ public function getParams(): array
+ {
+ return ['decimals' => $this->decimals];
+ }
+
  private function toRawString(mixed $input): string
  {
  if (is_string($input)) {

diff --git a/library/Rules/EndsWith.php b/library/Rules/EndsWith.php
@@ -32,6 +32,14 @@ public function validate(mixed $input): bool
  return $this->validateEquals($input);
  }
 
+ /**
+ * @return array<string, mixed>
+ */
+ public function getParams(): array
+ {
+ return ['endValue' => $this->endValue];
+ }
+
  private function validateEquals(mixed $input): bool
  {
  if (is_array($input)) {

diff --git a/library/Rules/Equals.php b/library/Rules/Equals.php
@@ -9,15 +9,10 @@
 
 namespace Respect\Validation\Rules;
 
-final class Equals extends AbstractRule
+final class Equals extends AbstractComparison
 {
- public function __construct(
- private readonly mixed $compareTo
- ) {
- }
-
- public function validate(mixed $input): bool
+ protected function compare(mixed $left, mixed $right): bool
  {
- return $input == $this->compareTo;
+ return $left == $right;
  }
 }
diff --git a/library/Rules/Equivalent.php b/library/Rules/Equivalent.php
@@ -28,6 +28,14 @@ public function validate(mixed $input): bool
  return $input == $this->compareTo;
  }
 
+ /**
+ * @return array<string, mixed>
+ */
+ public function getParams(): array
+ {
+ return ['compareTo' => $this->compareTo];
+ }
+
  private function isStringEquivalent(string $input): bool
  {
  if (!is_scalar($this->compareTo)) {

diff --git a/library/Rules/Extension.php b/library/Rules/Extension.php
@@ -35,4 +35,12 @@ public function validate(mixed $input): bool
 
  return $this->extension === pathinfo($input, PATHINFO_EXTENSION);
  }
+
+ /**
+ * @return array<string, string>
+ */
+ public function getParams(): array
+ {
+ return ['extension' => $this->extension];
+ }
 }
diff --git a/library/Rules/Factor.php b/library/Rules/Factor.php
@@ -40,4 +40,12 @@ public function validate(mixed $input): bool
  // factor of the dividend.
  return is_integer($dividend / $input);
  }
+
+ /**
+ * @return array<string, int>
+ */
+ public function getParams(): array
+ {
+ return ['dividend' => $this->dividend];
+ }
 }
diff --git a/library/Rules/Identical.php b/library/Rules/Identical.php
@@ -9,15 +9,10 @@
 
 namespace Respect\Validation\Rules;
 
-final class Identical extends AbstractRule
+final class Identical extends AbstractComparison
 {
- public function __construct(
- private readonly mixed $compareTo
- ) {
- }
-
- public function validate(mixed $input): bool
+ protected function compare(mixed $left, mixed $right): bool
  {
- return $input === $this->compareTo;
+ return $left === $right;
  }
 }
diff --git a/library/Rules/In.php b/library/Rules/In.php
@@ -31,6 +31,14 @@ public function validate(mixed $input): bool
  return $this->validateEquals($input);
  }
 
+ /**
+ * @return array<string, mixed>
+ */
+ public function getParams(): array
+ {
+ return ['haystack' => $this->haystack];
+ }
+
  private function validateEquals(mixed $input): bool
  {
  if (is_array($this->haystack)) {

diff --git a/library/Rules/Instance.php b/library/Rules/Instance.php
@@ -20,4 +20,12 @@ public function validate(mixed $input): bool
  {
  return $input instanceof $this->instanceName;
  }
+
+ /**
+ * @return array<string, mixed>
+ */
+ public function getParams(): array
+ {
+ return ['instanceName' => $this->instanceName];
+ }
 }
diff --git a/library/Rules/Ip.php b/library/Rules/Ip.php
@@ -73,6 +73,14 @@ public function getTemplate(mixed $input): string
  return $this->template ?? ($this->range ? self::TEMPLATE_NETWORK_RANGE : self::TEMPLATE_STANDARD);
  }
 
+ /**
+ * @return array<string, mixed>
+ */
+ public function getParams(): array
+ {
+ return ['range' => $this->range];
+ }
+
  private function createRange(): ?string
  {
  if ($this->startAddress && $this->endAddress) {

diff --git a/library/Rules/KeySet.php b/library/Rules/KeySet.php
@@ -89,6 +89,17 @@ public function getTemplate(mixed $input): string
  return KeySet::TEMPLATE_STRUCTURE;
  }
 
+ /**
+ * @return array<string, mixed>
+ */
+ public function getParams(): array
+ {
+ return [
+ 'keys' => $this->keys,
+ 'extraKeys' => $this->extraKeys,
+ ];
+ }
+
  /**
  * @throws ComponentException
  */