From 1b844763a943d15c71a1a83adf6e5ef032037281 Mon Sep 17 00:00:00 2001
From: Henrique Moody <henriquemoody@gmail.com>
Date: Fri, 5 Apr 2019 22:44:42 +0200
Subject: [PATCH] Improve message when nested "Not" rules fail

When using nested "Not" and "AllOf" rules Validation does not fetch the
message of the rule that failed, and instead it fetches the default
message in "AllOfException."

That is because "Not" cannot reach the rules inside "AllOf," making it
impossible to fetch the correct message.

This commit will improve that a bit but making "Not" deal directly with
the rule inside "AllOf" when it has only one rule.

Unfortunately, it will not fix all the issues. For example, when the
negated rule is an "AllOf" with multiple rules and it fails "Not" will
fetch only the first message.

Signed-off-by: Henrique Moody <henriquemoody@gmail.com>
---
 library/Rules/Not.php                     | 27 ++++++++++++++++-
 tests/integration/not_with_recursion.phpt | 35 ++++++++++++++---------
 tests/unit/Rules/NotTest.php              | 12 ++++----
 3 files changed, 53 insertions(+), 21 deletions(-)

diff --git a/library/Rules/Not.php b/library/Rules/Not.php
index 9385f6fd..32004d56 100644
--- a/library/Rules/Not.php
+++ b/library/Rules/Not.php
@@ -16,6 +16,8 @@ namespace Respect\Validation\Rules;
 use Respect\Validation\Exceptions\ValidationException;
 use Respect\Validation\Validatable;
 use function array_shift;
+use function count;
+use function current;
 
 /**
  * @author Alexandre Gomes Gaigalas <alexandre@gaigalas.net>
@@ -31,7 +33,12 @@ final class Not extends AbstractRule
 
     public function __construct(Validatable $rule)
     {
-        $this->rule = $rule;
+        $this->rule = $this->extractNegatedRule($rule);
+    }
+
+    public function getNegatedRule(): Validatable
+    {
+        return $this->rule;
     }
 
     public function setName(string $name): Validatable
@@ -90,4 +97,22 @@ final class Not extends AbstractRule
 
         return $rule;
     }
+
+    private function extractNegatedRule(Validatable $rule): Validatable
+    {
+        if ($rule instanceof self && $rule->getNegatedRule() instanceof self) {
+            return $this->extractNegatedRule($rule->getNegatedRule()->getNegatedRule());
+        }
+
+        if (!$rule instanceof AllOf) {
+            return $rule;
+        }
+
+        $rules = $rule->getRules();
+        if (count($rules) === 1) {
+            return $this->extractNegatedRule(current($rules));
+        }
+
+        return $rule;
+    }
 }
diff --git a/tests/integration/not_with_recursion.phpt b/tests/integration/not_with_recursion.phpt
index d23fde9f..38d4d753 100644
--- a/tests/integration/not_with_recursion.phpt
+++ b/tests/integration/not_with_recursion.phpt
@@ -1,33 +1,40 @@
 --CREDITS--
 Henrique Moody <henriquemoody@gmail.com>
---TEST--
-not() with recursion should update mode of its children
 --FILE--
 <?php
 
-declare(strict_types=1);
+ declare(strict_types=1);
 
 require 'vendor/autoload.php';
 
 use Respect\Validation\Exceptions\NestedValidationException;
-use Respect\Validation\Validator;
+use Respect\Validation\Exceptions\ValidationException;
+use Respect\Validation\Validator as v;
 
-try {
-    $validator = Validator::not(
-        Validator::not(
-            Validator::not(
-                Validator::not(
-                    Validator::not(
-                        Validator::intVal()->positive()
-                    )
+$validator = v::not(
+    v::not(
+        v::not(
+            v::not(
+                v::not(
+                    v::intVal()->positive()
                 )
             )
         )
-    );
+    )
+);
+
+try {
     $validator->check(2);
+} catch (ValidationException $exception) {
+    echo $exception->getMessage().PHP_EOL;
+}
+
+try {
+    $validator->assert(2);
 } catch (NestedValidationException $exception) {
     echo $exception->getFullMessage().PHP_EOL;
 }
 ?>
 --EXPECT--
-- These rules must not pass for 2
+2 must not be positive
+- 2 must not be positive
diff --git a/tests/unit/Rules/NotTest.php b/tests/unit/Rules/NotTest.php
index 4ec4d8e5..792ea3de 100644
--- a/tests/unit/Rules/NotTest.php
+++ b/tests/unit/Rules/NotTest.php
@@ -69,7 +69,7 @@ final class NotTest extends TestCase
         $not->setName('Foo');
 
         self::assertEquals('Foo', $not->getName());
-        self::assertEquals('Foo', $rule->getName());
+        self::assertEquals('Foo', $not->getNegatedRule()->getName());
     }
 
     /**
@@ -108,11 +108,11 @@ final class NotTest extends TestCase
     public function providerForSetName(): array
     {
         return [
-            [new IntVal()],
-            [new AllOf(new NumericVal(), new IntVal())],
-            [new Not(new Not(new IntVal()))],
-            [Validator::intVal()->setName('Bar')],
-            [Validator::noneOf(Validator::numericVal(), Validator::intVal())],
+            'non-allOf' => [new IntVal()],
+            'allOf' => [new AllOf(new NumericVal(), new IntVal())],
+            'not' => [new Not(new Not(new IntVal()))],
+            'allOf with name' => [Validator::intVal()->setName('Bar')],
+            'noneOf' => [Validator::noneOf(Validator::numericVal(), Validator::intVal())],
         ];
     }
 }