Apply contribution guidelines to "Length" rule

Because of the type hinting some validation could be removed from the
"length" constructor.

While applying the contribution guidelines we could also see some
duplicated logic in the "extractLength" method and that the rule was
validating

Co-authored-by: Henrique Moody <henriquemoody@gmail.com>
Signed-off-by: Henrique Moody <henriquemoody@gmail.com>

docs/rules/Length.md

@@ -5,7 +5,9 @@
 - `Length(null, int $max)`
 - `Length(int $min, int $max, bool $inclusive)`
 
-Validates lengths. Most simple example:
+Validates the length of the given input.
+
+Most simple example:
 
 ```php
 v::stringType()->length(1, 5)->validate('abc'); // true

library/Exceptions/LengthException.php

@@ -13,13 +13,21 @@ declare(strict_types=1);
 
 namespace Respect\Validation\Exceptions;
 
-class LengthException extends ValidationException
+/**
+ * @author Alexandre Gomes Gaigalas <alexandre@gaigalas.net>
+ * @author Danilo Correa <dcorrea@autodoc.com.br>
+ * @author Henrique Moody <henriquemoody@gmail.com>
+ */
+final class LengthException extends ValidationException
 {
     public const BOTH = 'both';
     public const LOWER = 'lower';
     public const GREATER = 'greater';
     public const EXACT = 'exact';
 
+    /**
+     * {@inheritdoc}
+     */
     public static $defaultTemplates = [
         self::MODE_DEFAULT => [
             self::BOTH => '{{name}} must have a length between {{minValue}} and {{maxValue}}',
@@ -35,6 +43,9 @@ class LengthException extends ValidationException
         ],
     ];
 
+    /**
+     * {@inheritdoc}
+     */
     protected function chooseTemplate(): string
     {
         if (!$this->getParam('minValue')) {

library/Rules/Length.php

@@ -13,70 +13,101 @@ declare(strict_types=1);
 
 namespace Respect\Validation\Rules;
 
+use Countable as CountableInterface;
 use Respect\Validation\Exceptions\ComponentException;
+use function count;
+use function is_array;
+use function is_int;
+use function is_object;
+use function is_string;
+use function mb_detect_encoding;
+use function mb_strlen;
 
-class Length extends AbstractRule
+/**
+ * Validates the length of the given input.
+ *
+ * @author Alexandre Gomes Gaigalas <alexandre@gaigalas.net>
+ * @author Blake Hair <blake.hair@gmail.com>
+ * @author Danilo Correa <danilosilva87@gmail.com>
+ * @author Henrique Moody <henriquemoody@gmail.com>
+ * @author Hugo Hamon <hugo.hamon@sensiolabs.com>
+ * @author João Torquato <joao.otl@gmail.com>
+ * @author Marcelo Araujo <msaraujo@php.net>
+ */
+final class Length extends AbstractRule
 {
-    public $minValue;
-    public $maxValue;
-    public $inclusive;
+    /**
+     * @var int
+     */
+    private $minValue;
 
-    public function __construct($min = null, $max = null, $inclusive = true)
+    /**
+     * @var int
+     */
+    private $maxValue;
+
+    /**
+     * @var bool
+     */
+    private $inclusive;
+
+    /**
+     * Creates the rule with a minimum and maximum value.
+     *
+     * @param int|null $min
+     * @param int|null $max
+     * @param bool $inclusive TRUE by default
+     *
+     * @throws ComponentException
+     */
+    public function __construct(int $min = null, int $max = null, $inclusive = true)
     {
         $this->minValue = $min;
         $this->maxValue = $max;
         $this->inclusive = $inclusive;
-        $paramValidator = new AnyOf(new NumericVal(), new NullType());
-        if (!$paramValidator->validate($min)) {
-            throw new ComponentException(
-                sprintf('%s is not a valid numeric length', $min)
-            );
-        }
 
-        if (!$paramValidator->validate($max)) {
-            throw new ComponentException(
-                sprintf('%s is not a valid numeric length', $max)
-            );
-        }
-
-        if (!is_null($min) && !is_null($max) && $min > $max) {
-            throw new ComponentException(
-                sprintf('%s cannot be less than %s for validation', $min, $max)
-            );
+        if (null !== $max && $min > $max) {
+            throw new ComponentException(sprintf('%d cannot be less than %d for validation', $min, $max));
         }
     }
 
+    /**
+     * {@inheritdoc}
+     */
     public function validate($input): bool
     {
         $length = $this->extractLength($input);
+        if (null === $length) {
+            return false;
+        }
 
         return $this->validateMin($length) && $this->validateMax($length);
     }
 
-    protected function extractLength($input)
+    private function extractLength($input): ?int
     {
         if (is_string($input)) {
             return mb_strlen($input, mb_detect_encoding($input));
         }
 
-        if (is_array($input) || $input instanceof \Countable) {
+        if (is_array($input) || $input instanceof CountableInterface) {
             return count($input);
         }
 
         if (is_object($input)) {
-            return count(get_object_vars($input));
+            return $this->extractLength(get_object_vars($input));
         }
 
         if (is_int($input)) {
-            return mb_strlen((string) $input);
+            return $this->extractLength((string) $input);
         }
 
-        return false;
+        return null;
     }
 
-    protected function validateMin($length)
+    private function validateMin(int $length): bool
     {
-        if (is_null($this->minValue)) {
+        if (null === $this->minValue) {
             return true;
         }
 
@@ -87,9 +118,9 @@ class Length extends AbstractRule
         return $length > $this->minValue;
     }
 
-    protected function validateMax($length)
+    private function validateMax(int $length): bool
     {
-        if (is_null($this->maxValue)) {
+        if (null === $this->maxValue) {
             return true;
         }
 

tests/integration/rules/length.phpt

@@ -0,0 +1,94 @@
+--FILE--
+<?php
+
+require_once 'vendor/autoload.php';
+
+use Respect\Validation\Exceptions\LengthException;
+use Respect\Validation\Exceptions\NestedValidationException;
+use Respect\Validation\Validator as v;
+
+try {
+    v::length(0, 5)->check('phpsp.org.br');
+} catch (LengthException $exception) {
+    echo $exception->getMessage().PHP_EOL;
+}
+
+try {
+    v::length(20)->check('phpsp.org.br');
+} catch (LengthException $exception) {
+    echo $exception->getMessage().PHP_EOL;
+}
+
+try {
+    v::length(5, 10)->check('phpsp.org.br');
+} catch (LengthException $exception) {
+    echo $exception->getMessage().PHP_EOL;
+}
+
+try {
+    v::not(v::length(0, 20))->check('phpsp.org.br');
+} catch (LengthException $exception) {
+    echo $exception->getMessage().PHP_EOL;
+}
+
+try {
+    v::not(v::length(10))->check('phpsp.org.br');
+} catch (LengthException $exception) {
+    echo $exception->getMessage().PHP_EOL;
+}
+
+try {
+    v::not(v::length(5, 20))->check('phpsp.org.br');
+} catch (LengthException $exception) {
+    echo $exception->getMessage().PHP_EOL;
+}
+
+try {
+    v::length(0, 5)->assert('phpsp.org.br');
+} catch (NestedValidationException $exception) {
+    echo $exception->getFullMessage().PHP_EOL;
+}
+
+try {
+    v::length(20)->assert('phpsp.org.br');
+} catch (NestedValidationException $exception) {
+    echo $exception->getFullMessage().PHP_EOL;
+}
+
+try {
+    v::length(5, 10)->assert('phpsp.org.br');
+} catch (NestedValidationException $exception) {
+    echo $exception->getFullMessage().PHP_EOL;
+}
+
+try {
+    v::not(v::length(0, 20))->assert('phpsp.org.br');
+} catch (NestedValidationException $exception) {
+    echo $exception->getFullMessage().PHP_EOL;
+}
+
+try {
+    v::not(v::length(10))->assert('phpsp.org.br');
+} catch (NestedValidationException $exception) {
+    echo $exception->getFullMessage().PHP_EOL;
+}
+
+try {
+    v::not(v::length(5, 20))->assert('phpsp.org.br');
+} catch (NestedValidationException $exception) {
+    echo $exception->getFullMessage().PHP_EOL;
+}
+?>
+--EXPECT--
+"phpsp.org.br" must have a length lower than 5
+"phpsp.org.br" must have a length greater than 20
+"phpsp.org.br" must have a length between 5 and 10
+"phpsp.org.br" must not have a length lower than 20
+"phpsp.org.br" must not have a length greater than 10
+"phpsp.org.br" must not have a length between 5 and 20
+- "phpsp.org.br" must have a length lower than 5
+- "phpsp.org.br" must have a length greater than 20
+- "phpsp.org.br" must have a length between 5 and 10
+- "phpsp.org.br" must not have a length lower than 20
+- "phpsp.org.br" must not have a length greater than 10
+- "phpsp.org.br" must not have a length between 5 and 20

tests/integration/rules/length_1.phpt

@@ -1,15 +0,0 @@
---FILE--
-<?php
-
-require_once 'vendor/autoload.php';
-
-use Respect\Validation\Validator as v;
-
-$validator = v::length(0, 10);
-
-$validator->validate('phpsp');
-v::not($validator)->validate('phpsp');
-$validator->assert('nickolas');
-$validator->check('nawarian');
-?>
---EXPECTF--

tests/integration/rules/length_2.phpt

@@ -1,31 +0,0 @@
---FILE--
-<?php
-
-require_once 'vendor/autoload.php';
-
-use Respect\Validation\Exceptions\LengthException;
-use Respect\Validation\Validator as v;
-
-try {
-    v::length(0, 5)->check('nawarian');
-} catch (LengthException $e) {
-    echo $e->getMessage().PHP_EOL;
-}
-
-try {
-    v::length(13, null)->check('phpsp.org.br');
-} catch (LengthException $e) {
-    echo $e->getMessage().PHP_EOL;
-}
-
-try {
-    v::not(v::length(5, 20))->check('phpsp.org.br');
-} catch (LengthException $e) {
-    echo $e->getMessage().PHP_EOL;
-}
-
-?>
---EXPECTF--
-"nawarian" must have a length lower than 5
-"phpsp.org.br" must have a length greater than 13
-"phpsp.org.br" must not have a length between 5 and 20

tests/integration/rules/length_3.phpt

@@ -1,16 +0,0 @@
---FILE--
-<?php
-
-require_once 'vendor/autoload.php';
-
-use Respect\Validation\Exceptions\AllOfException;
-use Respect\Validation\Validator as v;
-
-try {
-    v::length(3, 5)->assert('phpsp.org.br');
-} catch (AllOfException $e) {
-    echo $e->getFullMessage();
-}
-?>
---EXPECTF--
-- "phpsp.org.br" must have a length between 3 and 5

tests/integration/rules/length_4.phpt

@@ -1,16 +0,0 @@
---FILE--
-<?php
-
-require_once 'vendor/autoload.php';
-
-use Respect\Validation\Exceptions\LengthException;
-use Respect\Validation\Validator as v;
-
-try {
-    v::length(5, 5)->check('123456');
-} catch (LengthException $e) {
-    echo $e->getMessage();
-}
-?>
---EXPECTF--
-"123456" must have a length of 5

tests/unit/Rules/LengthTest.php

@@ -13,120 +13,76 @@ declare(strict_types=1);
 
 namespace Respect\Validation\Rules;
 
-use PHPUnit\Framework\TestCase;
+use Respect\Validation\Exceptions\ComponentException;
+use Respect\Validation\Test\RuleTestCase;
+use Respect\Validation\Test\Stubs\CountableStub;
+use function range;
+use function tmpfile;
 
 /**
- * @group  rule
- * @covers \Respect\Validation\Exceptions\LengthException
+ * @group rule
+ *
  * @covers \Respect\Validation\Rules\Length
+ *
+ * @author Alexandre Gomes Gaigalas <alexandre@gaigalas.net>
+ * @author Augusto Pascutti <contato@augustopascutti.com>
+ * @author Danilo Correa <danilosilva87@gmail.com>
+ * @author Gabriel Caruso <carusogabriel34@gmail.com>
+ * @author Henrique Moody <henriquemoody@gmail.com>
  */
-class LengthTest extends TestCase
+final class LengthTest extends RuleTestCase
 {
     /**
-     * @dataProvider providerForValidLengthInclusive
-     *
-     * @test
+     * {@inheritdoc}
      */
-    public function lengthInsideBoundsForInclusiveCasesReturnTrue($string, $min, $max): void
+    public function providerForValidInput(): array
     {
-        $validator = new Length($min, $max, true);
-        self::assertTrue($validator->validate($string));
+        return [
+            [new Length(1, 15), 'alganet'],
+            [new Length(4, 6), 'ççççç'],
+            [new Length(1, 30), range(1, 20)],
+            [new Length(0, 2), (object) ['foo' => 'bar', 'bar' => 'baz']],
+            [new Length(1, null), 'alganet'], //null is a valid max length, means "no maximum",
+            [new Length(null, 15), 'alganet'], //null is a valid min length, means "no minimum"
+            [new Length(1, 15, false), 'alganet'],
+            [new Length(4, 6, false), 'ççççç'],
+            [new Length(1, 30, false), range(1, 20)],
+            [new Length(1, 3, false), (object) ['foo' => 'bar', 'bar' => 'baz']],
+            [new Length(1, null, false), 'alganet'], //null is a valid max length, means "no maximum",
+            [new Length(null, 15, false), 'alganet'], //null is a valid min length, means "no minimum"
+            [new Length(1, 15), new CountableStub(5)],
+            [new Length(1, 15), 12345],
+        ];
     }
 
     /**
-     * @dataProvider providerForValidLengthNonInclusive
-     *
-     * @test
+     * {@inheritdoc}
      */
-    public function lengthInsideBoundsForNonInclusiveCasesShouldReturnTrue($string, $min, $max): void
+    public function providerForInvalidInput(): array
     {
-        $validator = new Length($min, $max, false);
-        self::assertTrue($validator->validate($string));
+        return [
+            [new Length(1, 15), ''],
+            [new Length(1, 6), 'alganet'],
+            [new Length(1, 19), range(1, 20)],
+            [new Length(8, null), 'alganet'], //null is a valid max length, means "no maximum",
+            [new Length(null, 6), 'alganet'], //null is a valid min length, means "no minimum"
+            [new Length(1, 7, false), 'alganet'],
+            [new Length(3, 5, false), (object) ['foo' => 'bar', 'bar' => 'baz']],
+            [new Length(1, 30, false), range(1, 50)],
+            [new Length(0), tmpfile()],
+            [new Length(1, 4), new CountableStub(5)],
+            [new Length(1, 4), 12345],
+        ];
     }
 
     /**
-     * @dataProvider providerForInvalidLengthInclusive
-     *
      * @test
      */
-    public function lengthOutsideBoundsForInclusiveCasesReturnFalse($string, $min, $max): void
+    public function isShouldNotNotAcceptInvalidLengths(): void
     {
-        $validator = new Length($min, $max, true);
-        self::assertfalse($validator->validate($string));
-    }
+        $this->expectException(ComponentException::class);
+        $this->expectExceptionMessage('10 cannot be less than 1 for validation');
 
-    /**
-     * @dataProvider providerForInvalidLengthNonInclusive
-     *
-     * @test
-     */
-    public function lengthOutsideBoundsForNonInclusiveCasesReturnFalse($string, $min, $max): void
-    {
-        $validator = new Length($min, $max, false);
-        self::assertfalse($validator->validate($string));
-    }
-
-    /**
-     * @dataProvider providerForComponentException
-     * @expectedException \Respect\Validation\Exceptions\ComponentException
-     *
-     * @test
-     */
-    public function componentExceptionsForInvalidParameters($min, $max): void
-    {
-        $buggyValidator = new Length($min, $max);
-    }
-
-    public function providerForValidLengthInclusive()
-    {
-        return [
-            ['alganet', 1, 15],
-            ['ççççç', 4, 6],
-            [range(1, 20), 1, 30],
-            [(object) ['foo' => 'bar', 'bar' => 'baz'], 0, 2],
-            ['alganet', 1, null], //null is a valid max length, means "no maximum",
-            ['alganet', null, 15], //null is a valid min length, means "no minimum"
-        ];
-    }
-
-    public function providerForValidLengthNonInclusive()
-    {
-        return [
-            ['alganet', 1, 15],
-            ['ççççç', 4, 6],
-            [range(1, 20), 1, 30],
-            [(object) ['foo' => 'bar', 'bar' => 'baz'], 1, 3],
-            ['alganet', 1, null], //null is a valid max length, means "no maximum",
-            ['alganet', null, 15], //null is a valid min length, means "no minimum"
-        ];
-    }
-
-    public function providerForInvalidLengthInclusive()
-    {
-        return [
-            ['', 1, 15],
-            ['alganet', 1, 6],
-            [range(1, 20), 1, 19],
-            ['alganet', 8, null], //null is a valid max length, means "no maximum",
-            ['alganet', null, 6], //null is a valid min length, means "no minimum"
-        ];
-    }
-
-    public function providerForInvalidLengthNonInclusive()
-    {
-        return [
-            ['alganet', 1, 7],
-            [(object) ['foo' => 'bar', 'bar' => 'baz'], 3, 5],
-            [range(1, 50), 1, 30],
-        ];
-    }
-
-    public function providerForComponentException()
-    {
-        return [
-            ['a', 15],
-            [1, 'abc d'],
-            [10, 1],
-        ];
+        new Length(10, 1);
     }
 }