Merge pull request #174 from codisart/issue-167

Fix Between validator behavior

Author: weierophinney

src/Between.php

@@ -17,6 +17,13 @@ class Between extends AbstractValidator
     const NOT_BETWEEN        = 'notBetween';
     const NOT_BETWEEN_STRICT = 'notBetweenStrict';
 
+    /**
+     * Retain if min and max are numeric values. Allow to not compare string and numeric types
+     *
+     * @var boolean
+     */
+    private $numeric;
+
     /**
      * Validation failure message template definitions
      *
@@ -81,7 +88,17 @@ public function __construct($options = null)
         if (count($options) !== 2
             && (! array_key_exists('min', $options) || ! array_key_exists('max', $options))
         ) {
-            throw new Exception\InvalidArgumentException("Missing option. 'min' and 'max' have to be given");
+            throw new Exception\InvalidArgumentException("Missing option : 'min' and 'max' have to be given");
+        }
+
+        if (is_numeric($options['min']) && is_numeric($options['max'])) {
+            $this->numeric = true;
+        } elseif (is_string($options['min']) && is_string($options['max'])) {
+            $this->numeric = false;
+        } else {
+            throw new Exception\InvalidArgumentException(
+                "Invalid options : 'min' and 'max' should be of the same scalar type"
+            );
         }
 
         parent::__construct($options);
@@ -162,6 +179,13 @@ public function setInclusive($inclusive)
      */
     public function isValid($value)
     {
+        if ($this->numeric && ! is_numeric($value)) {
+            return false;
+        }
+        if (! $this->numeric && ! is_string($value)) {
+            return false;
+        }
+
         $this->setValue($value);
 
         if ($this->getInclusive()) {

test/BetweenTest.php

@@ -18,37 +18,76 @@
  */
 class BetweenTest extends TestCase
 {
+    public function providerBasic()
+    {
+        return [
+            [
+                'min' => 1,
+                'max' => 100,
+                'inclusive' => true,
+                'expected' => true,
+                'values' => [1, 10, 100],
+            ],
+            [
+                'min' => 1,
+                'max' => 100,
+                'inclusive' => true,
+                'expected' => false,
+                'values' => [0, 0.99, 100.01, 101],
+            ],
+            [
+                'min' => 1,
+                'max' => 100,
+                'inclusive' => false,
+                'expected' => false,
+                'values' => [0, 1, 100, 101],
+            ],
+            [
+                'min' => 'a',
+                'max' => 'z',
+                'inclusive' => true,
+                'expected' => true,
+                'values' => ['a', 'b', 'y', 'z'],
+            ],
+            [
+                'min' => 'a',
+                'max' => 'z',
+                'inclusive' => false,
+                'expected' => false,
+                'values' => ['!', 'a', 'z'],
+            ],
+            [
+                'min' => 0,
+                'max' => 99999999,
+                'inclusive' => true,
+                'expected' => false,
+                'values' => ['asdasd', 'q'],
+            ],
+            [
+                'min' => 'a',
+                'max' => 'zzzzz',
+                'inclusive' => true,
+                'expected' => false,
+                'values' => [0, 10, 548],
+            ],
+        ];
+    }
     /**
      * Ensures that the validator follows expected behavior
      *
+     * @dataProvider providerBasic
      * @return void
      */
-    public function testBasic()
+    public function testBasic($min, $max, $inclusive, $expected, $values)
     {
-        /**
-         * The elements of each array are, in order:
-         *      - minimum
-         *      - maximum
-         *      - inclusive
-         *      - expected validation result
-         *      - array of test input values
-         */
-        $valuesExpected = [
-            [1, 100, true, true, [1, 10, 100]],
-            [1, 100, true, false, [0, 0.99, 100.01, 101]],
-            [1, 100, false, false, [0, 1, 100, 101]],
-            ['a', 'z', true, true, ['a', 'b', 'y', 'z']],
-            ['a', 'z', false, false, ['!', 'a', 'z']]
-            ];
-        foreach ($valuesExpected as $element) {
-            $validator = new Between(['min' => $element[0], 'max' => $element[1], 'inclusive' => $element[2]]);
-            foreach ($element[4] as $input) {
-                $this->assertEquals(
-                    $element[3],
-                    $validator->isValid($input),
-                    'Failed values: ' . $input . ":" . implode("\n", $validator->getMessages())
-                );
-            }
+        $validator = new Between(['min' => $min, 'max' => $max, 'inclusive' => $inclusive]);
+
+        foreach ($values as $input) {
+            $this->assertEquals(
+                $expected,
+                $validator->isValid($input),
+                'Failed values: ' . $input . ":" . implode("\n", $validator->getMessages())
+            );
         }
     }
 
@@ -117,7 +156,7 @@ public function testEqualsMessageVariables()
     public function testMissingMinOrMax(array $args)
     {
         $this->expectException(InvalidArgumentException::class);
-        $this->expectExceptionMessage("Missing option. 'min' and 'max' have to be given");
+        $this->expectExceptionMessage("Missing option : 'min' and 'max' have to be given");
 
         new Between($args);
     }