[fixed] type and whitelist/blacklist checks threw inconsistent errors

closes #19

Author: jquense

.eslintrc

@@ -23,7 +23,7 @@
     "no-unused-vars": [2, {"vars": "all", "args": "after-used"}],
     "no-use-before-define": 0,
     "key-spacing": 0,
-    "consitent-return": 0,
+    "consistent-return": 0,
     "no-shadow": 0,
     "no-sequences": 0
   }

src/mixed.js

@@ -10,10 +10,14 @@ var Promise = require('promise/lib/es6-extensions')
   , createValidation = require('./util/createValidation')
   , BadSet = require('./util/set');
 
-let formatError = ValidationError.formatError
-
 let notEmpty = value => !isAbsent(value);
 
+function runValidations(validations, endEarly, value, path) {
+  return endEarly
+    ? Promise.all(validations)
+    : _.collectErrors(validations, value, path)
+}
+
 module.exports = SchemaType
 
 function SchemaType(options = {}){
@@ -27,7 +31,10 @@ function SchemaType(options = {}){
   this._blacklist   = new BadSet()
   this.tests        = []
   this.transforms   = []
-  this._typeError   = formatError(locale.notType)
+
+  this.withMutation(() => {
+    this.typeError(locale.notType)
+  })
 
   if (_.has(options, 'default'))
     this._defaultDefault = options.default
@@ -114,9 +121,7 @@ SchemaType.prototype = {
 
   //-- tests
   _validate(_value, options = {}, state = {}) {
-    let valids   = this._whitelist
-      , invalids = this._blacklist
-      , context  = options.context
+    let context  = options.context
       , parent   = state.parent
       , value    = _value
       , schema, endEarly, isStrict;
@@ -127,41 +132,28 @@ SchemaType.prototype = {
 
     let path = state.path
 
-    let errors = [];
-    let reject = () => Promise.reject(new ValidationError(errors, value));
-
     if (!state.isCast && !isStrict)
       value = schema._cast(value, options)
 
     // value is cast, we can check if it meets type requirements
-    if (value !== undefined && !schema.isType(value)) {
-      errors.push(schema._typeError({ value, path, type: schema._type }))
-      if ( endEarly ) return reject()
-    }
-
-    // next check Whitelist for matching values
-    if (valids.length && !valids.has(value)) {
-      errors.push(schema._whitelistError(valids.values(), path))
-      if (endEarly) return reject()
-    }
-
-    // next check Blacklist for matching values
-    if (invalids.has(value)) {
-      errors.push(schema._blacklistError(invalids.values(), path))
-      if (endEarly) return reject()
-    }
-
-    // It makes no sense to validate further at this point if their are errors
-    if (errors.length)
-      return reject()
-
-    let result = schema.tests.map(fn => fn({ value, path, state, schema, options }))
-
-    result = endEarly
-      ? Promise.all(result)
-      : _.collectErrors(result, value, path)
-
-    return result.then(() => value);
+    let validationParams = { value, path, state, schema, options }
+    let initialTests = []
+
+    if (schema._typeError)
+      initialTests.push(schema._typeError(validationParams));
+    if (schema._whitelistError)
+      initialTests.push(schema._whitelistError(validationParams));
+    if (schema._blacklistError)
+      initialTests.push(schema._blacklistError(validationParams));
+
+    return runValidations(initialTests, endEarly, value, path)
+      .then(() => runValidations(
+          schema.tests.map(fn => fn(validationParams))
+        , endEarly
+        , value
+        , path
+      ))
+      .then(() => value)
   },
 
   validate(value, options, cb) {
@@ -212,12 +204,6 @@ SchemaType.prototype = {
     )
   },
 
-  typeError(msg){
-    var next = this.clone()
-    next._typeError = formatError(msg)
-    return next
-  },
-
   nullable(value) {
     var next = this.clone()
     next._nullable = value === false ? false : true
@@ -296,34 +282,67 @@ SchemaType.prototype = {
     return next
   },
 
-  oneOf(enums, msg) {
+  typeError(message) {
     var next = this.clone()
 
-    if( next.tests.length )
-      throw new TypeError('Cannot specify values when there are validation rules specified')
+    next._typeError = createValidation({
+      message,
+      name: 'typeError',
+      test(value) {
+        if (value !== undefined && !this.schema.isType(value))
+          return this.createError({
+            params: { type: this.schema._type }
+          })
+        return true
+      }
+    })
+    return next
+  },
 
-    next._whitelistError = (valids, path) =>
-      formatError(msg || locale.oneOf, { values: valids.join(', '), path })
+  oneOf(enums, message = locale.oneOf) {
+    var next = this.clone();
 
-    enums.forEach( val => {
+    if (next.tests.length)
+      throw new TypeError('Cannot specify values when there are validation rules specified')
+
+    enums.forEach(val => {
       next._blacklist.delete(val)
       next._whitelist.add(val)
     })
 
+    next._whitelistError = createValidation({
+      message,
+      name: 'oneOf',
+      test(value) {
+        let valids = this.schema._whitelist
+        if (valids.length && !valids.has(value))
+          return this.createError({ params: { values: valids.values().join(', ') }})
+        return true
+      }
+    })
+
     return next
   },
 
-  notOneOf(enums, msg) {
-    var next = this.clone()
-
-    next._blacklistError = (invalids, path) =>
-      formatError(msg || locale.notOneOf, { values: invalids.join(', '), path })
+  notOneOf(enums, message = locale.notOneOf) {
+    var next = this.clone();
 
     enums.forEach( val => {
       next._whitelist.delete(val)
       next._blacklist.add(val)
     })
 
+    next._blacklistError = createValidation({
+      message,
+      name: 'notOneOf',
+      test(value) {
+        let invalids = this.schema._blacklist
+        if (invalids.length && invalids.has(value))
+          return this.createError({ params: { values: invalids.values().join(', ') }})
+        return true
+      }
+    })
+
     return next
   },
 

src/util/createValidation.js

@@ -1,24 +1,19 @@
 'use strict';
 var Promise = require('promise/lib/es6-extensions')
-  , Condition   = require('./condition')
-  , ValidationError = require('./validation-error')
-  , getter = require('property-expr').getter
-  , locale = require('../locale.js').mixed
-  , _ = require('./_');
+  , ValidationError = require('./validation-error');
 
 let formatError = ValidationError.formatError
 
-
-function createErrorFactory(orginalMessage, orginalPath, value, params, originalType) {
-  return function createError({ path = orginalPath, message = orginalMessage, type = originalType } = {}) {
+function createErrorFactory(orginalMessage, orginalPath, value, orginalParams, originalType) {
+  return function createError({ path = orginalPath, message = orginalMessage, type = originalType, params } = {}) {
     return new ValidationError(
-      formatError(message, { path, value, ...params}), value, path, type)
+      formatError(message, { path, value, ...orginalParams, ...params }), value, path, type)
   }
 }
 
 module.exports = function createValidation(options) {
   let { name, message, test, params, useCallback } = options
-  
+
   function validate({ value, path, state: { parent }, ...rest }) {
     var createError = createErrorFactory(message, path, value, params, name)
     var ctx = { path, parent, createError, type: name, ...rest }
@@ -43,3 +38,5 @@ module.exports = function createValidation(options) {
 
   return validate
 }
+
+module.exports.createErrorFactory = createErrorFactory

src/util/validation-error.js

@@ -38,6 +38,7 @@ ValidationError.isError = function(err){
 }
 
 ValidationError.formatError = function(message, params) {
+  
   if ( typeof message === 'string')
     message = replace(message)
 

test/mixed.js

@@ -40,6 +40,23 @@ describe( 'Mixed Types ', function(){
     return inst.cast().should.equal('hello')
   })
 
+  it('should check types', function(){
+    var inst = string().strict().typeError('must be a ${type}!')
+
+    return Promise.all([
+      inst.validate(5).should.be.rejected.then(function(err) {
+        err.type.should.equal('typeError')
+        err.message.should.equal('must be a string!')
+        err.inner.length.should.equal(0)
+      }),
+      inst.validate(5, { abortEarly: false }).should.be.rejected.then(function(err) {
+        chai.expect(err.type).to.not.exist
+        err.message.should.equal('must be a string!')
+        err.inner.length.should.equal(1)
+      })
+    ])
+  })
+
   it('should limit values', function(){
     var inst = mixed().oneOf(['hello', 5])
 
@@ -65,11 +82,22 @@ describe( 'Mixed Types ', function(){
       inst.validate(5).should.be.rejected.then(function(err){
         err.errors[0].should.equal('this must not be one the following values: hello, 5')
       }),
-
       inst.oneOf([5]).isValid(5).should.eventually.equal(true)
     ])
   })
 
+  it('should run subset of validations first', function(){
+    var called = false;
+    var inst = string()
+      .strict()
+      .test('test', 'boom', () => called = true)
+
+    return inst.validate(25).should.be.rejected
+      .then(() => {
+        called.should.equal(false)
+      })
+  })
+
   it('should respect strict', function(){
     var inst = string().equals(['hello', '5'])
 
@@ -181,7 +209,7 @@ describe( 'Mixed Types ', function(){
         message: 'invalid',
         exclusive: true,
         name: 'max',
-        test: function(){
+        test() {
           this.path.should.equal('test')
           this.parent.should.eql({ other: 5, test : 'hi' })
           this.options.context.should.eql({ user: 'jason' })
@@ -197,7 +225,7 @@ describe( 'Mixed Types ', function(){
     var inst = mixed().test({
         message: 'invalid ${path}',
         name: 'max',
-        test: function(){
+        test() {
           return this.createError({ path: 'my.path' })
         }
       })