Proper check for value of min and max

changelog.md

@@ -0,0 +1,4 @@
+1.0.4 / 2015-03-20
+==================
+
+* Bug fix ([#20](https://github.com/TabDigital/strummer/issues/20)): `integer` and `number` matchers fail to catch invalid data when `{min: 0}` or `{max: 0}` is passed as opt.

lib/matchers/integer.js

@@ -1,4 +1,7 @@
-var s = require('../s');
+var s        = require('../s');
+var utils    = require('../utils');
+var hasValue = utils.hasValue;
+var isNumber = utils.isNumber;
 
 var parseIntFromString = function (value) {
   if(/^(\-|\+)?([0-9]+)$/.test(value))
@@ -7,28 +10,41 @@ var parseIntFromString = function (value) {
 }
 
 module.exports = function (opts) {
+  var hasMinValue = false;
+  var hasMaxValue = false;
 
-  if (!opts) opts = {};
+  if(hasValue(opts)) {
+    hasMinValue = hasValue(opts.min);
+    hasMaxValue = hasValue(opts.max);
 
-  return s(function(value) {
+    if(hasMinValue && !isNumber(opts.min)) {
+      throw new Error('Invalid minimum option: ' + opts.min);
+    }
+
+    if(hasMaxValue && !isNumber(opts.max)) {
+      throw new Error('Invalid maximum option: ' + opts.max);
+    }
+  } else {
+    opts = {};
+  }
 
+  return s(function(value) {
     if (opts.parse) {
       value = parseIntFromString(value);
     }
 
     if (typeof value !== 'number' || value % 1 !== 0) {
       return 'should be an integer';
     }
-    if (opts) {
-      if (opts.min && opts.max && (value < opts.min || value > opts.max)) {
-        return 'should be an integer between ' + opts.min + ' and ' + opts.max;
-      }
-      if (opts.min && value < opts.min) {
-        return 'should be an integer >= ' + opts.min;
-      }
-      if (opts.max && value > opts.max) {
-        return 'should be an integer <= ' + opts.max;
-      }
+
+    if (hasMinValue && hasMaxValue && (value < opts.min || value > opts.max)) {
+      return 'should be an integer between ' + opts.min + ' and ' + opts.max;
+    }
+    if (hasMinValue && value < opts.min) {
+      return 'should be an integer >= ' + opts.min;
+    }
+    if (hasMaxValue && value > opts.max) {
+      return 'should be an integer <= ' + opts.max;
     }
   });
 

lib/matchers/number.js

@@ -1,4 +1,7 @@
-var s = require('../s');
+var s        = require('../s');
+var utils    = require('../utils');
+var hasValue = utils.hasValue;
+var isNumber = utils.isNumber;
 
 var parseFloatFromString = function (value) {
   if(/^(\-|\+)?([0-9]+(\.[0-9]+)?)$/.test(value))
@@ -7,11 +10,25 @@ var parseFloatFromString = function (value) {
 }
 
 module.exports = function (opts) {
+  var hasMinValue = false;
+  var hasMaxValue = false;
 
-  if (!opts) opts = {};
+  if(hasValue(opts)) {
+    hasMinValue = hasValue(opts.min);
+    hasMaxValue = hasValue(opts.max);
 
-  return s(function(value) {
+    if(hasMinValue && !isNumber(opts.min)) {
+      throw new Error('Invalid minimum option: ' + opts.min);
+    }
+
+    if(hasMaxValue && !isNumber(opts.max)) {
+      throw new Error('Invalid maximum option: ' + opts.max);
+    }
+  } else {
+    opts = {};
+  }
 
+  return s(function(value) {
     if (opts.parse) {
       value = parseFloatFromString(value);
     }
@@ -20,16 +37,14 @@ module.exports = function (opts) {
       return 'should be a number';
     }
 
-    if (opts) {
-      if (opts.min && opts.max && (value < opts.min || value > opts.max)) {
-        return 'should be a number between ' + opts.min + ' and ' + opts.max;
-      }
-      if (opts.min && value < opts.min) {
-        return 'should be a number >= ' + opts.min;
-      }
-      if (opts.max && value > opts.max) {
-        return 'should be a number <= ' + opts.max;
-      }
+    if (hasMinValue && hasMaxValue && (value < opts.min || value > opts.max)) {
+      return 'should be a number between ' + opts.min + ' and ' + opts.max;
+    }
+    if (hasMinValue && value < opts.min) {
+      return 'should be a number >= ' + opts.min;
+    }
+    if (hasMaxValue && value > opts.max) {
+      return 'should be a number <= ' + opts.max;
     }
   });
 

lib/utils.js

@@ -0,0 +1,12 @@
+var hasValue = function(val){
+  return (typeof val !== 'undefined') && (val !== null);
+}
+
+var isNumber = function(val){
+  return typeof val === 'number';
+}
+
+module.exports = {
+  hasValue: hasValue,
+  isNumber: isNumber
+}

test/matchers/integer.spec.js

@@ -19,6 +19,23 @@ describe('integer matcher', function() {
     integer({min: 3})('', 0).should.have.error(/should be an integer >= 3/);
     integer({max: 3})('', 5).should.have.error(/should be an integer <= 3/);
     integer({min: 3, max: 5})('', 7).should.have.error(/should be an integer between 3 and 5/);
+    integer({min: 0})('', -10).should.have.error(/should be an integer >= 0/);
+    integer({max: 0})('', 3).should.have.error(/should be an integer <= 0/);
+  });
+
+  it('fails for invalid min or max values', function(){
+    var shouldFail = function(val) {
+      (function(){
+        integer({min: val});
+      }).should.throw('Invalid minimum option: ' + val);
+
+      (function(){
+        integer({max: val});
+      }).should.throw('Invalid maximum option: ' + val);
+    }
+
+    var invalidValues = ['a', '', {}, []];
+    invalidValues.forEach(shouldFail);
   });
 
   it('can parse integer from string', function() {

test/matchers/number.spec.js

@@ -16,10 +16,27 @@ describe('number matcher', function() {
 
   it('supports min and max', function() {
     number({min: 3})('', 0).should.have.error(/should be a number >= 3/);
+    number({min: 0})('', -10).should.have.error(/should be a number >= 0/);
+    number({max: 0})('', 12).should.have.error(/should be a number <= 0/);
     number({max: 3})('', 5).should.have.error(/should be a number <= 3/);
     number({min: 3, max: 5})('', 7).should.have.error(/should be a number between 3 and 5/);
   });
 
+  it('fails for invalid min or max values', function(){
+    var shouldFail = function(val) {
+      (function(){
+        number({min: val});
+      }).should.throw('Invalid minimum option: ' + val);
+
+      (function(){
+        number({max: val});
+      }).should.throw('Invalid maximum option: ' + val);
+    }
+
+    var invalidValues = ['a', '', {}, []];
+    invalidValues.forEach(shouldFail);
+  });
+
   it('can parse string into number', function() {
     number({parse: true})('', 0).should.not.have.error();
     number({parse: true})('', 3).should.not.have.error();

test/util.spec.js

@@ -0,0 +1,28 @@
+var utils    = require('../lib/utils');
+var hasValue = utils.hasValue;
+
+describe("hasValue() util function", function(){
+  it('Should return false when value is <null>', function(){
+    hasValue(null).should.be.false;
+  });
+
+  it('Should return false when value is <undefined>', function(){
+    hasValue(undefined).should.be.false;
+  });
+
+  it('Should return true when value is a number', function(){
+    hasValue(12).should.be.true;
+  });
+
+  it('Should return true when value is a string', function(){
+    hasValue('a').should.be.true;
+  });
+
+  it('Should return true when value is <{}>', function(){
+    hasValue({}).should.be.true;
+  });
+
+  it('Should return true when value is <"">', function(){
+    hasValue('').should.be.true;
+  });
+});