From fc8230acef3765ef7bcda967da73b1b7039dd383 Mon Sep 17 00:00:00 2001 From: Alexandre Gomes Gaigalas Date: Sat, 18 Feb 2023 15:12:19 -0300 Subject: [PATCH] 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 --- docs/rules/KeySet.md | 3 +++ library/Exceptions/KeySetException.php | 11 +++++----- library/NonNegatable.php | 18 ++++++++++++++++ library/Rules/KeySet.php | 12 ++++++++++- library/Rules/Not.php | 13 +++++++++++ tests/unit/Rules/KeySetTest.php | 30 ++++++++++++++++++++++++++ 6 files changed, 81 insertions(+), 6 deletions(-) create mode 100644 library/NonNegatable.php diff --git a/docs/rules/KeySet.md b/docs/rules/KeySet.md index 07b7d971..ba2f7e0d 100644 --- a/docs/rules/KeySet.md +++ b/docs/rules/KeySet.md @@ -43,6 +43,8 @@ v::keySet( )->validate($dict); // true ``` +It is not possible to negate `keySet()` rules with `not()`. + The keys' order is not considered in the validation. ## Categorization @@ -55,6 +57,7 @@ The keys' order is not considered in the validation. Version | Description --------|------------- + 2.3.0 | KeySet is NonNegatable, fixed message with extra keys 1.0.0 | Created *** diff --git a/library/Exceptions/KeySetException.php b/library/Exceptions/KeySetException.php index 2f9e8fad..c0d5cd7e 100644 --- a/library/Exceptions/KeySetException.php +++ b/library/Exceptions/KeySetException.php @@ -17,6 +17,7 @@ use function count; final class KeySetException extends GroupedValidationException implements NonOmissibleException { public const STRUCTURE = 'structure'; + public const STRUCTURE_EXTRA = 'structure_extra'; /** * {@inheritDoc} @@ -26,11 +27,7 @@ final class KeySetException extends GroupedValidationException implements NonOmi self::NONE => 'All of the required rules must pass for {{name}}', self::SOME => 'These rules must pass for {{name}}', self::STRUCTURE => 'Must have keys {{keys}}', - ], - 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}}', + self::STRUCTURE_EXTRA => 'Must not have keys {{extraKeys}}', ], ]; @@ -39,6 +36,10 @@ final class KeySetException extends GroupedValidationException implements NonOmi */ protected function chooseTemplate(): string { + if (count($this->getParam('extraKeys'))) { + return self::STRUCTURE_EXTRA; + } + if (count($this->getChildren()) === 0) { return self::STRUCTURE; } diff --git a/library/NonNegatable.php b/library/NonNegatable.php new file mode 100644 index 00000000..7fa56265 --- /dev/null +++ b/library/NonNegatable.php @@ -0,0 +1,18 @@ + + * SPDX-License-Identifier: MIT + */ + +declare(strict_types=1); + +namespace Respect\Validation; + +/** Interface for validation rules */ +/** + * @author Alexandre Gomes Gaigalas + */ +interface NonNegatable +{ +} diff --git a/library/Rules/KeySet.php b/library/Rules/KeySet.php index ca9bf650..accc5ed9 100644 --- a/library/Rules/KeySet.php +++ b/library/Rules/KeySet.php @@ -10,6 +10,7 @@ declare(strict_types=1); namespace Respect\Validation\Rules; use Respect\Validation\Exceptions\ComponentException; +use Respect\Validation\NonNegatable; use Respect\Validation\Validatable; use function array_key_exists; @@ -24,13 +25,18 @@ use function is_array; * @author Emmerson Siqueira * @author Henrique Moody */ -final class KeySet extends AbstractWrapper +final class KeySet extends AbstractWrapper implements NonNegatable { /** * @var mixed[] */ private $keys; + /** + * @var mixed[] + */ + private $extraKeys = []; + /** * @var Key[] */ @@ -127,6 +133,10 @@ final class KeySet extends AbstractWrapper unset($input[$keyRule->getReference()]); } + foreach ($input as $extraKey => &$ignoreValue) { + $this->extraKeys[] = $extraKey; + } + return count($input) == 0; } } diff --git a/library/Rules/Not.php b/library/Rules/Not.php index 1ebdc7c3..919a902c 100644 --- a/library/Rules/Not.php +++ b/library/Rules/Not.php @@ -9,12 +9,16 @@ declare(strict_types=1); namespace Respect\Validation\Rules; +use Respect\Validation\Exceptions\ComponentException; use Respect\Validation\Exceptions\ValidationException; +use Respect\Validation\NonNegatable; use Respect\Validation\Validatable; use function array_shift; use function count; use function current; +use function get_class; +use function sprintf; /** * @author Alexandre Gomes Gaigalas @@ -97,6 +101,15 @@ final class Not extends AbstractRule 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) { return $this->extractNegatedRule($rule->getNegatedRule()->getNegatedRule()); } diff --git a/tests/unit/Rules/KeySetTest.php b/tests/unit/Rules/KeySetTest.php index 0fc4ef37..4e677c88 100644 --- a/tests/unit/Rules/KeySetTest.php +++ b/tests/unit/Rules/KeySetTest.php @@ -171,6 +171,36 @@ final class KeySetTest extends TestCase $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 *