Make KeySet impossible to wrap in not(), fix structure message

The use case for negating a keyset is very confusing, and can
lead to validators that don't do what they expect.

This commit introduces NonNegatable rules, which will throw
a Component exception if you try to wrap them in `Not`.

This change was necessary to ensure proper message reporting
when extra keys exist on the keyset.

This fixes #1349
This commit is contained in:
Alexandre Gomes Gaigalas 2023-02-18 15:12:19 -03:00
parent 493a665e99
commit fc8230acef
6 changed files with 81 additions and 6 deletions

View file

@ -43,6 +43,8 @@ v::keySet(
)->validate($dict); // true )->validate($dict); // true
``` ```
It is not possible to negate `keySet()` rules with `not()`.
The keys' order is not considered in the validation. The keys' order is not considered in the validation.
## Categorization ## Categorization
@ -55,6 +57,7 @@ The keys' order is not considered in the validation.
Version | Description Version | Description
--------|------------- --------|-------------
2.3.0 | KeySet is NonNegatable, fixed message with extra keys
1.0.0 | Created 1.0.0 | Created
*** ***

View file

@ -17,6 +17,7 @@ use function count;
final class KeySetException extends GroupedValidationException implements NonOmissibleException final class KeySetException extends GroupedValidationException implements NonOmissibleException
{ {
public const STRUCTURE = 'structure'; public const STRUCTURE = 'structure';
public const STRUCTURE_EXTRA = 'structure_extra';
/** /**
* {@inheritDoc} * {@inheritDoc}
@ -26,11 +27,7 @@ final class KeySetException extends GroupedValidationException implements NonOmi
self::NONE => 'All of the required rules must pass for {{name}}', self::NONE => 'All of the required rules must pass for {{name}}',
self::SOME => 'These rules must pass for {{name}}', self::SOME => 'These rules must pass for {{name}}',
self::STRUCTURE => 'Must have keys {{keys}}', self::STRUCTURE => 'Must have keys {{keys}}',
], self::STRUCTURE_EXTRA => 'Must not have keys {{extraKeys}}',
self::MODE_NEGATIVE => [
self::NONE => 'None of these rules must pass for {{name}}',
self::SOME => 'These rules must not pass for {{name}}',
self::STRUCTURE => 'Must not have keys {{keys}}',
], ],
]; ];
@ -39,6 +36,10 @@ final class KeySetException extends GroupedValidationException implements NonOmi
*/ */
protected function chooseTemplate(): string protected function chooseTemplate(): string
{ {
if (count($this->getParam('extraKeys'))) {
return self::STRUCTURE_EXTRA;
}
if (count($this->getChildren()) === 0) { if (count($this->getChildren()) === 0) {
return self::STRUCTURE; return self::STRUCTURE;
} }

18
library/NonNegatable.php Normal file
View file

@ -0,0 +1,18 @@
<?php
/*
* Copyright (c) Alexandre Gomes Gaigalas <alganet@gmail.com>
* SPDX-License-Identifier: MIT
*/
declare(strict_types=1);
namespace Respect\Validation;
/** Interface for validation rules */
/**
* @author Alexandre Gomes Gaigalas <alganet@gmail.com>
*/
interface NonNegatable
{
}

View file

@ -10,6 +10,7 @@ declare(strict_types=1);
namespace Respect\Validation\Rules; namespace Respect\Validation\Rules;
use Respect\Validation\Exceptions\ComponentException; use Respect\Validation\Exceptions\ComponentException;
use Respect\Validation\NonNegatable;
use Respect\Validation\Validatable; use Respect\Validation\Validatable;
use function array_key_exists; use function array_key_exists;
@ -24,13 +25,18 @@ use function is_array;
* @author Emmerson Siqueira <emmersonsiqueira@gmail.com> * @author Emmerson Siqueira <emmersonsiqueira@gmail.com>
* @author Henrique Moody <henriquemoody@gmail.com> * @author Henrique Moody <henriquemoody@gmail.com>
*/ */
final class KeySet extends AbstractWrapper final class KeySet extends AbstractWrapper implements NonNegatable
{ {
/** /**
* @var mixed[] * @var mixed[]
*/ */
private $keys; private $keys;
/**
* @var mixed[]
*/
private $extraKeys = [];
/** /**
* @var Key[] * @var Key[]
*/ */
@ -127,6 +133,10 @@ final class KeySet extends AbstractWrapper
unset($input[$keyRule->getReference()]); unset($input[$keyRule->getReference()]);
} }
foreach ($input as $extraKey => &$ignoreValue) {
$this->extraKeys[] = $extraKey;
}
return count($input) == 0; return count($input) == 0;
} }
} }

View file

@ -9,12 +9,16 @@ declare(strict_types=1);
namespace Respect\Validation\Rules; namespace Respect\Validation\Rules;
use Respect\Validation\Exceptions\ComponentException;
use Respect\Validation\Exceptions\ValidationException; use Respect\Validation\Exceptions\ValidationException;
use Respect\Validation\NonNegatable;
use Respect\Validation\Validatable; use Respect\Validation\Validatable;
use function array_shift; use function array_shift;
use function count; use function count;
use function current; use function current;
use function get_class;
use function sprintf;
/** /**
* @author Alexandre Gomes Gaigalas <alganet@gmail.com> * @author Alexandre Gomes Gaigalas <alganet@gmail.com>
@ -97,6 +101,15 @@ final class Not extends AbstractRule
private function extractNegatedRule(Validatable $rule): Validatable private function extractNegatedRule(Validatable $rule): Validatable
{ {
if ($rule instanceof NonNegatable) {
throw new ComponentException(
sprintf(
'"%s" can not be wrapped in Not()',
get_class($rule)
)
);
}
if ($rule instanceof self && $rule->getNegatedRule() instanceof self) { if ($rule instanceof self && $rule->getNegatedRule() instanceof self) {
return $this->extractNegatedRule($rule->getNegatedRule()->getNegatedRule()); return $this->extractNegatedRule($rule->getNegatedRule()->getNegatedRule());
} }

View file

@ -171,6 +171,36 @@ final class KeySetTest extends TestCase
$keySet->assert($input); $keySet->assert($input);
} }
/**
* @test
*/
public function shouldWarnOfExtraKeysWithMessage(): void
{
$input = ['foo' => 123, 'bar' => 456];
$key1 = new Key('foo', new AlwaysValid(), true);
$keySet = new KeySet($key1);
$this->expectException(KeySetException::class);
$this->expectExceptionMessage('Must not have keys `{ "bar" }`');
$keySet->assert($input);
}
/**
* @test
*/
public function cannotBeNegated(): void
{
$key1 = new Key('foo', new AlwaysValid(), true);
$this->expectException(ComponentException::class);
$this->expectExceptionMessage('"Respect\Validation\Rules\KeySet" can not be wrapped in Not()');
new Not(new KeySet($key1));
}
/** /**
* @test * @test
* *