Skip to content

Commit

Permalink
WIP: Improve messages in an array format
Browse files Browse the repository at this point in the history
- Name: That's what is in the placeholder, and could be used for path
  and id as well
- Id: Tha could be the name of the rule, just a means to identify it and
  be able to build templates for it on that level.
- Path: That's the real path of the rule, which will be useful when
  dealing with arrays or objects.
  • Loading branch information
henriquemoody committed Dec 20, 2024
1 parent 0e40917 commit 259be0c
Show file tree
Hide file tree
Showing 24 changed files with 235 additions and 108 deletions.
96 changes: 56 additions & 40 deletions library/Message/StandardFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public function __construct(
}

/**
* @param array<string, mixed> $templates
* @param array<string|int, mixed> $templates
*/
public function main(Result $result, array $templates, Translator $translator): string
{
Expand All @@ -50,7 +50,7 @@ public function main(Result $result, array $templates, Translator $translator):
}

/**
* @param array<string, mixed> $templates
* @param array<string|int, mixed> $templates
*/
public function full(
Result $result,
Expand Down Expand Up @@ -91,43 +91,46 @@ public function full(
}

/**
* @param array<string, mixed> $templates
* @param array<string|int, mixed> $templates
*
* @return array<string, mixed>
* @return array<string|int, mixed>
*/
public function array(Result $result, array $templates, Translator $translator): array
{
$selectedTemplates = $this->selectTemplates($result, $templates);
$deduplicatedChildren = $this->extractDeduplicatedChildren($result);
if (count($deduplicatedChildren) === 0 || $this->isFinalTemplate($result, $selectedTemplates)) {
return [
$result->id => $this->renderer->render($this->getTemplated($result, $selectedTemplates), $translator),
];
}

$messages = [];
$messages = [
'__root__' => $this->renderer->render($this->getTemplated($result, $selectedTemplates), $translator),
];

$children = [];

foreach ($deduplicatedChildren as $child) {
$messages[$child->id] = $this->array(
$childKey = $child->path ?? $child->id;

$children[$childKey] = $this->array(
$child,
$this->selectTemplates($child, $selectedTemplates),
$translator
);
if (count($messages[$child->id]) !== 1) {

if (count($children[$childKey]) !== 1) {
continue;
}

$messages[$child->id] = current($messages[$child->id]);
$children[$childKey] = current($children[$childKey]);
}

if (count($messages) > 1) {
$self = [
'__root__' => $this->renderer->render($this->getTemplated($result, $selectedTemplates), $translator),
];
if (count($children) === 0 || $this->isFinalTemplate($result, $selectedTemplates)) {
return [$result->path ?? $result->id => $messages['__root__']];
}

return $self + $messages;
if ($result->path !== null) {
return [$result->path => $messages + $children];
}

return $messages;
return $messages + $children;
}

private function isAlwaysVisible(Result $result, Result ...$siblings): bool
Expand Down Expand Up @@ -165,56 +168,69 @@ private function isAlwaysVisible(Result $result, Result ...$siblings): bool
);
}

/** @param array<string, mixed> $templates */
/** @param array<string|int, mixed> $templates */
private function getTemplated(Result $result, array $templates): Result
{
if ($result->hasCustomTemplate()) {
return $result;
}

if (!isset($templates[$result->id]) && isset($templates['__root__'])) {
return $result->withTemplate($templates['__root__']);
$keys = [$result->name, $result->path, $result->id, '__root__'];
foreach ($keys as $key) {
if (isset($templates[$key]) && is_string($templates[$key])) {
return $result->withTemplate($templates[$key]);
}
}

if (!isset($templates[$result->id])) {
if (!isset($templates[$result->id]) && !isset($templates[$result->path]) && !isset($templates[$result->name])) {
return $result;
}

$template = $templates[$result->id];
if (is_string($template)) {
return $result->withTemplate($template);
}

throw new ComponentException(
sprintf('Template for "%s" must be a string, %s given', $result->id, stringify($template))
sprintf(
'Template for "%s" must be a string, %s given',
$result->path ?? $result->name ?? $result->id,
stringify($templates)
)
);
}

/**
* @param array<string, mixed> $templates
* @param array<string|int, mixed> $templates
*/
private function isFinalTemplate(Result $result, array $templates): bool
{
if (isset($templates[$result->id]) && is_string($templates[$result->id])) {
return true;
$keys = [$result->name, $result->path, $result->id];
foreach ($keys as $key) {
if (isset($templates[$key]) && is_string($templates[$key])) {
return true;
}
}

if (count($templates) !== 1) {
return false;
}

return isset($templates['__root__']) || isset($templates[$result->id]);
foreach ($keys as $key) {
if (isset($templates[$key])) {
return true;
}
}

return isset($templates['__root__']);
}

/**
* @param array<string, mixed> $templates
* @param array<string|int, mixed> $templates
*
* @return array<string, mixed>
* @return array<string|int, mixed>
*/
private function selectTemplates(Result $message, array $templates): array
private function selectTemplates(Result $result, array $templates): array
{
if (isset($templates[$message->id]) && is_array($templates[$message->id])) {
return $templates[$message->id];
foreach ([$result->name, $result->path, $result->id] as $key) {
if (isset($templates[$key]) && is_array($templates[$key])) {
return $templates[$key];
}
}

return $templates;
Expand All @@ -227,7 +243,7 @@ private function extractDeduplicatedChildren(Result $result): array
$deduplicatedResults = [];
$duplicateCounters = [];
foreach ($result->children as $child) {
$id = $child->id;
$id = $child->path ?? $child->id;
if (isset($duplicateCounters[$id])) {
$id .= '.' . ++$duplicateCounters[$id];
} elseif (array_key_exists($id, $deduplicatedResults)) {
Expand All @@ -236,7 +252,7 @@ private function extractDeduplicatedChildren(Result $result): array
$duplicateCounters[$id] = 2;
$id .= '.2';
}
$deduplicatedResults[$id] = $child->isValid ? null : $child->withId($id);
$deduplicatedResults[$id] = $child->isValid ? null : $child->withId((string) $id);
}

return array_values(array_filter($deduplicatedResults));
Expand Down
16 changes: 6 additions & 10 deletions library/Result.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public function __construct(
?string $name = null,
?string $id = null,
public readonly ?Result $adjacent = null,
public readonly bool $unchangeableId = false,
public readonly string|int|null $path = null,
Result ...$children,
) {
$this->name = $rule->getName() ?? $name;
Expand Down Expand Up @@ -99,21 +99,17 @@ public function withTemplate(string $template): self

public function withId(string $id): self
{
if ($this->unchangeableId) {
return $this;
}

return $this->clone(id: $id);
}

public function withUnchangeableId(string $id): self
public function withPath(string|int $path): self
{
return $this->clone(id: $id, unchangeableId: true);
return $this->clone(path: $path);
}

public function withPrefix(string $prefix): self
{
if ($this->id === $this->name || $this->unchangeableId) {
if ($this->id === $this->name || $this->path !== null) {
return $this;
}

Expand Down Expand Up @@ -200,7 +196,7 @@ private function clone(
?string $name = null,
?string $id = null,
?Result $adjacent = null,
?bool $unchangeableId = null,
string|int|null $path = null,
?array $children = null
): self {
return new self(
Expand All @@ -213,7 +209,7 @@ private function clone(
$name ?? $this->name,
$id ?? $this->id,
$adjacent ?? $this->adjacent,
$unchangeableId ?? $this->unchangeableId,
$path ?? $this->path,
...($children ?? $this->children)
);
}
Expand Down
2 changes: 1 addition & 1 deletion library/Rules/Each.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ protected function evaluateNonEmptyArray(array $input): Result
{
$children = [];
foreach ($input as $key => $value) {
$children[] = $this->rule->evaluate($value)->withUnchangeableId((string) $key);
$children[] = $this->rule->evaluate($value)->withPath($key);
}
$isValid = array_reduce($children, static fn ($carry, $childResult) => $carry && $childResult->isValid, true);
if ($isValid) {
Expand Down
2 changes: 1 addition & 1 deletion library/Rules/Key.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public function evaluate(mixed $input): Result

return $this->rule
->evaluate($input[$this->key])
->withUnchangeableId((string) $this->key)
->withPath($this->key)
->withNameIfMissing($this->rule->getName() ?? (string) $this->key);
}
}
2 changes: 1 addition & 1 deletion library/Rules/KeyOptional.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public function evaluate(mixed $input): Result

return $this->rule
->evaluate($input[$this->key])
->withUnchangeableId((string) $this->key)
->withPath($this->key)
->withNameIfMissing($this->rule->getName() ?? (string) $this->key);
}
}
2 changes: 1 addition & 1 deletion library/Rules/Property.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function evaluate(mixed $input): Result

return $this->rule
->evaluate($this->extractPropertyValue($input, $this->propertyName))
->withUnchangeableId($this->propertyName)
->withPath($this->propertyName)
->withNameIfMissing($this->rule->getName() ?? $this->propertyName);
}
}
2 changes: 1 addition & 1 deletion library/Rules/PropertyOptional.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function evaluate(mixed $input): Result

return $this->rule
->evaluate($this->extractPropertyValue($input, $this->propertyName))
->withUnchangeableId($this->propertyName)
->withPath($this->propertyName)
->withNameIfMissing($this->rule->getName() ?? $this->propertyName);
}
}
6 changes: 3 additions & 3 deletions library/Validator.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ final class Validator implements Rule
/** @var array<Rule> */
private array $rules = [];

/** @var array<string, mixed> */
/** @var array<string|int, mixed> */
private array $templates = [];

private ?string $name = null;
Expand Down Expand Up @@ -65,7 +65,7 @@ public function isValid(mixed $input): bool
return $this->evaluate($input)->isValid;
}

/** @param array<string, mixed>|callable(ValidationException): Throwable|string|Throwable|null $template */
/** @param array<string|int, mixed>|callable(ValidationException): Throwable|string|Throwable|null $template */
public function assert(mixed $input, array|string|Throwable|callable|null $template = null): void
{
$result = $this->evaluate($input);
Expand Down Expand Up @@ -99,7 +99,7 @@ public function assert(mixed $input, array|string|Throwable|callable|null $templ
throw $template($exception);
}

/** @param array<string, mixed> $templates */
/** @param array<string|int, mixed> $templates */
public function setTemplates(array $templates): self
{
$this->templates = $templates;
Expand Down
1 change: 1 addition & 0 deletions tests/feature/Issues/Issue1289Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
- description must be a string value
FULL_MESSAGE,
[
'__root__' => 'Each item in `[["default": 2, "description": [], "children": ["nope"]]]` must be valid',
0 => [
'__root__' => 'These rules must pass for `["default": 2, "description": [], "children": ["nope"]]`',
'default' => [
Expand Down
22 changes: 19 additions & 3 deletions tests/feature/Issues/Issue1334Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,31 @@ function (): void {
- street must be a string
FULL_MESSAGE,
[
'__root__' => 'These rules must pass for `[["region": "Oregon", "country": "USA", "other": 123], ["street": "", "region": "Oregon", "country": "USA"], ["s ... ]`',
'each' => [
'__root__' => 'Each item in `[["region": "Oregon", "country": "USA", "other": 123], ["street": "", "region": "Oregon", "country": "USA"], ["s ... ]` must be valid',
0 => [
'__root__' => 'These rules must pass for `["region": "Oregon", "country": "USA", "other": 123]`',
'street' => 'street must be present',
'other' => 'other must be a string or must be null',
'other' => [
'__root__' => 'These rules must pass for other',
'nullOrStringType' => 'other must be a string or must be null',
],
],
1 => [
'__root__' => 'These rules must pass for `["street": "", "region": "Oregon", "country": "USA"]`',
'street' => [
'__root__' => 'These rules must pass for street',
'notEmpty' => 'street must not be empty',
],
],
2 => [
'__root__' => 'These rules must pass for `["street": 123, "region": "Oregon", "country": "USA"]`',
'street' => [
'__root__' => 'These rules must pass for street',
'stringType' => 'street must be a string',
],
],
1 => 'street must not be empty',
2 => 'street must be a string',
],
]
));
11 changes: 9 additions & 2 deletions tests/feature/Issues/Issue1348Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
- model must be in `["F150", "Bronco"]`
FULL_MESSAGE,
[
'__root__' => 'These rules must pass for `[["manufacturer": "Honda", "model": "Accord"], ["manufacturer": "Toyota", "model": "Rav4"], ["manufacturer": "Fo ... ]`',
'each' => [
'__root__' => 'Each item in `[["manufacturer": "Honda", "model": "Accord"], ["manufacturer": "Toyota", "model": "Rav4"], ["manufacturer": "Fo ... ]` must be valid',
2 => [
Expand All @@ -61,11 +62,17 @@
'manufacturer' => 'manufacturer must be equal to "Toyota"',
'model' => 'model must be in `["Rav4", "Camry"]`',
],
'allOf.3' => 'model must be in `["F150", "Bronco"]`',
'allOf.3' => [
'__root__' => 'These rules must pass for `["manufacturer": "Ford", "model": "not real"]`',
'model' => 'model must be in `["F150", "Bronco"]`',
],
],
3 => [
'__root__' => 'Only one of these rules must pass for `["manufacturer": "Honda", "model": "not valid"]`',
'allOf.1' => 'model must be in `["Accord", "Fit"]`',
'allOf.1' => [
'__root__' => 'These rules must pass for `["manufacturer": "Honda", "model": "not valid"]`',
'model' => 'model must be in `["Accord", "Fit"]`',
],
'allOf.2' => [
'__root__' => 'All the required rules must pass for `["manufacturer": "Honda", "model": "not valid"]`',
'manufacturer' => 'manufacturer must be equal to "Toyota"',
Expand Down
Loading

0 comments on commit 259be0c

Please sign in to comment.