Do not accept whitespace by default in "Digit" rule

The "Digit" rule is meant to validate digits. However, by default, it also considers any whitespace character (spaces, new lines, tabs, etc) as valid. Since the rule also accepts a list of characters to ignore during the validation it seemed logical to me to leave the responsibility of allowing whitespace characters on the hands of the one who uses the rule. The messages of the exception are not really consistent, this commit will also fix that. It's also clear that the "AbstractCtypeRule" is an unnecessary overhead since it is only a proxy for "AbstractFilterRule". That one can and should even be removed after this commit is applied especially because this commit will also remove the method "filterWhiteSpaceOption" which is the only substantial difference between "AbstractCtypeRule" and "AbstractFilterRule". This commit will also apply our guidelines to the "Digit" rule since we want to do that to all the rules we have. Signed-off-by: Henrique Moody <henriquemoody@gmail.com>
Respect · Sep 12, 2018 · 9283c8e · 9283c8e
1 parent 27bd5d2
commit 9283c8e
Show file tree

Hide file tree

Showing 11 changed files with 151 additions and 160 deletions.
diff --git a/docs/rules/Digit.md b/docs/rules/Digit.md
@@ -1,13 +1,22 @@
 # Digit
 
 - `Digit()`
+- `Digit(string $additionalChars)`
 
-This is similar to `Alnum()`, but it doesn't allow a-Z.
+Validates whether the input contains only digits.
+
+```php
+v::digit()->validate('020 612 1851'); // false
+v::digit(' ')->validate('020 612 1851'); // true
+v::digit()->validate('172.655.537-21'); // false
+v::digit('.-')->validate('172.655.537-21'); // true
+```
 
 ## Changelog
 
 Version | Description
 --------|-------------
+  2.0.0 | Removed support to whitespaces by default
   0.5.0 | Renamed from `Digits` to `Digit`
   0.3.9 | Created as `Digits`
 

diff --git a/library/Exceptions/DigitException.php b/library/Exceptions/DigitException.php
@@ -13,16 +13,23 @@
 
 namespace Respect\Validation\Exceptions;
 
-class DigitException extends FilteredValidationException
+/**
+ * @author Alexandre Gomes Gaigalas <alexandre@gaigalas.net>
+ * @author Henrique Moody <henriquemoody@gmail.com>
+ */
+final class DigitException extends FilteredValidationException
 {
+    /**
+     * {@inheritdoc}
+     */
     public static $defaultTemplates = [
         self::MODE_DEFAULT => [
             self::STANDARD => '{{name}} must contain only digits (0-9)',
-            self::EXTRA => '{{name}} must contain only digits (0-9) and "{{additionalChars}}"',
+            self::EXTRA => '{{name}} must contain only digits (0-9) and {{additionalChars}}',
         ],
         self::MODE_NEGATIVE => [
             self::STANDARD => '{{name}} must not contain digits (0-9)',
-            self::EXTRA => '{{name}} must not contain digits (0-9) or "{{additionalChars}}"',
+            self::EXTRA => '{{name}} must not contain digits (0-9) and {{additionalChars}}',
         ],
     ];
 }
diff --git a/library/Rules/AbstractCtypeRule.php b/library/Rules/AbstractCtypeRule.php
@@ -17,15 +17,6 @@ abstract class AbstractCtypeRule extends AbstractFilterRule
 {
     abstract protected function ctypeFunction($input);
 
-    protected function filterWhiteSpaceOption($input)
-    {
-        if (!empty($this->additionalChars)) {
-            $input = str_replace(str_split($this->additionalChars), '', $input);
-        }
-
-        return preg_replace('/\s/', '', $input);
-    }
-
     public function validateClean($input)
     {
         return $this->ctypeFunction($input);

diff --git a/library/Rules/Digit.php b/library/Rules/Digit.php
@@ -13,14 +13,21 @@
 
 namespace Respect\Validation\Rules;
 
-class Digit extends AbstractCtypeRule
-{
-    protected function filter($input)
-    {
-        return $this->filterWhiteSpaceOption($input);
-    }
+use function ctype_digit;
 
-    protected function ctypeFunction($input)
+/**
+ * Validates whether the input contains only digits.
+ *
+ * @author Alexandre Gomes Gaigalas <alexandre@gaigalas.net>
+ * @author Henrique Moody <henriquemoody@gmail.com>
+ * @author Nick Lombard <github@jigsoft.co.za>
+ */
+final class Digit extends AbstractFilterRule
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function validateClean($input)
     {
         return ctype_digit($input);
     }

diff --git a/tests/integration/rules/digit.phpt b/tests/integration/rules/digit.phpt
@@ -0,0 +1,65 @@
+--FILE--
+<?php
+require 'vendor/autoload.php';
+
+use Respect\Validation\Exceptions\DigitException;
+use Respect\Validation\Exceptions\NestedValidationException;
+use Respect\Validation\Validator as v;
+
+try {
+    v::digit()->check('abc');
+} catch (DigitException $exception) {
+    echo $exception->getMessage().PHP_EOL;
+}
+
+try {
+    v::digit('-')->check('a-b');
+} catch (DigitException $exception) {
+    echo $exception->getMessage().PHP_EOL;
+}
+
+try {
+    v::not(v::digit())->check('123');
+} catch (DigitException $exception) {
+    echo $exception->getMessage().PHP_EOL;
+}
+
+try {
+    v::not(v::digit('-'))->check('1-3');
+} catch (DigitException $exception) {
+    echo $exception->getMessage().PHP_EOL;
+}
+
+try {
+    v::digit()->assert('abc');
+} catch (NestedValidationException $exception) {
+    echo $exception->getFullMessage().PHP_EOL;
+}
+
+try {
+    v::digit('-')->assert('a-b');
+} catch (NestedValidationException $exception) {
+    echo $exception->getFullMessage().PHP_EOL;
+}
+
+try {
+    v::not(v::digit())->assert('123');
+} catch (NestedValidationException $exception) {
+    echo $exception->getFullMessage().PHP_EOL;
+}
+
+try {
+    v::not(v::digit('-'))->assert('1-3');
+} catch (NestedValidationException $exception) {
+    echo $exception->getFullMessage().PHP_EOL;
+}
+?>
+--EXPECT--
+"abc" must contain only digits (0-9)
+"a-b" must contain only digits (0-9) and "-"
+"123" must not contain digits (0-9)
+"1-3" must not contain digits (0-9) and "-"
+- "abc" must contain only digits (0-9)
+- "a-b" must contain only digits (0-9) and "-"
+- "123" must not contain digits (0-9)
+- "1-3" must not contain digits (0-9) and "-"
diff --git a/tests/integration/rules/digit_1.phpt b/tests/integration/rules/digit_1.phpt
diff --git a/tests/integration/rules/digit_2.phpt b/tests/integration/rules/digit_2.phpt
diff --git a/tests/integration/rules/digit_3.phpt b/tests/integration/rules/digit_3.phpt
diff --git a/tests/integration/rules/digit_4.phpt b/tests/integration/rules/digit_4.phpt
diff --git a/tests/integration/rules/digit_5.phpt b/tests/integration/rules/digit_5.phpt
diff --git a/tests/unit/Rules/DigitTest.php b/tests/unit/Rules/DigitTest.php
@@ -13,105 +13,87 @@
 
 namespace Respect\Validation\Rules;
 
-use PHPUnit\Framework\TestCase;
+use Respect\Validation\Test\RuleTestCase;
+use stdClass;
 
 /**
- * @group  rule
- * @covers \Respect\Validation\Exceptions\DigitException
+ * @group rule
+ *
  * @covers \Respect\Validation\Rules\Digit
+ *
+ * @author Alexandre Gomes Gaigalas <alexandre@gaigalas.net>
+ * @author Gabriel Caruso <carusogabriel34@gmail.com>
+ * @author Henrique Moody <henriquemoody@gmail.com>
+ * @author Nick Lombard <github@jigsoft.co.za>
+ * @author Pascal Borreli <pascal@borreli.com>
  */
-class DigitTest extends TestCase
+final class DigitTest extends RuleTestCase
 {
     /**
-     * @dataProvider providerForValidDigits
-     *
      * @test
-     */
-    public function validDataWithDigitsShouldReturnTrue($validDigits, $additional = ''): void
-    {
-        $validator = new Digit($additional);
-        self::assertTrue($validator->validate($validDigits));
-    }
-
-    /**
-     * @dataProvider providerForInvalidDigits
-     * @expectedException \Respect\Validation\Exceptions\DigitException
      *
-     * @test
-     */
-    public function invalidDigitsShouldFailAndThrowDigitException($invalidDigits, $additional = ''): void
-    {
-        $validator = new Digit($additional);
-        self::assertFalse($validator->validate($invalidDigits));
-        $validator->assert($invalidDigits);
-    }
-
-    /**
-     * @dataProvider providerForInvalidParams
      * @expectedException \Respect\Validation\Exceptions\ComponentException
      *
-     * @test
-     */
-    public function invalidConstructorParamsShouldThrowComponentExceptionUponInstantiation($additional): void
-    {
-        $validator = new Digit($additional);
-    }
-
-    /**
-     * @dataProvider providerAdditionalChars
+     * @dataProvider providerForInvalidParams
      *
-     * @test
+     * @param mixed $additional
      */
-    public function additionalCharsShouldBeRespected($additional, $query): void
-    {
-        $validator = new Digit($additional);
-        self::assertTrue($validator->validate($query));
-    }
-
-    public function providerAdditionalChars()
+    public function invalidConstructorParamsShouldThrowComponentExceptionUponInstantiation($additional): void
     {
-        return [
-            ['!@#$%^&*(){}', '!@#$%^&*(){} 123'],
-            ['[]?+=/\\-_|"\',<>.', "[]?+=/\\-_|\"',<>. \t \n 123"],
-        ];
+        new Digit($additional);
     }
 
-    public function providerForInvalidParams()
+    public function providerForInvalidParams(): array
     {
         return [
-            [new \stdClass()],
+            [new stdClass()],
             [[]],
             [0x2],
         ];
     }
 
-    public function providerForValidDigits()
+    /**
+     * @throws \Respect\Validation\Exceptions\ComponentException
+     *
+     * @return array
+     */
+    public function providerForValidInput(): array
     {
         return [
-            ["\n\t"],
-            [' '],
-            [165],
-            [1650],
-            ['01650'],
-            ['165'],
-            ['1650'],
-            ['16 50'],
-            ["\n5\t"],
-            ['16-50', '-'],
+            'positive integer' => [new Digit(), 165],
+            'positive string-integer' => [new Digit(), '01650'],
+            'positive string-integer with one exception' => [new Digit('-'), '16-50'],
+            'positive string-integer with multiple exceptions' => [new Digit('.-'), '16-5.0'],
+            'only exceptions' => [new Digit('!@#$%^&*(){}'), '!@#$%^&*(){}'],
+            'float' => [new Digit(), 1.0],
+            'boolean true' => [new Digit(), true],
+            'octal' => [new Digit(), 01],
+            'string-octal' => [new Digit(), '01'],
         ];
     }
 
-    public function providerForInvalidDigits()
+    /**
+     * {@inheritdoc}
+     *
+     * @throws \Respect\Validation\Exceptions\ComponentException
+     */
+    public function providerForInvalidInput(): array
     {
         return [
-            [''],
-            [null],
-            ['16-50'],
-            ['a'],
-            ['Foo'],
-            ['12.1'],
-            ['-12'],
-            [-12],
+            'empty' => [new Digit(), ''],
+            'with spaces' => [new Digit(), '16 50'],
+            'with tabs' => [new Digit(), "1\t1"],
+            'with newlines' => [new Digit(), "1\n1"],
+            'null' => [new Digit(), null],
+            'with non-numeric' => [new Digit(), '16-50'],
+            'alphabetic' => [new Digit(), 'a'],
+            'alphanumeric' => [new Digit(), 'a1'],
+            'negative integer' => [new Digit(), -12],
+            'negative string-integer' => [new Digit(), '-12'],
+            'float-string' => [new Digit(), '1.0'],
+            'negative float' => [new Digit(), -1.1],
+            'negative string-float' => [new Digit(), '-1.1'],
+            'boolean false' => [new Digit(), false],
         ];
     }
 }