Merge pull request jquense#108 from jquense/errors

Clearer errors for common pitfalls

src/ValidationError.js

@@ -1,8 +1,9 @@
+import printValue from './util/printValue';
 
 let strReg = /\$\{\s*(\w+)\s*\}/g;
 
 let replace = str =>
-  params => str.replace(strReg, (_, key) => String(params[key]))
+  params => str.replace(strReg, (_, key) => printValue(params[key]))
 
 
 export default function ValidationError(errors, value, field, type) {
@@ -13,16 +14,16 @@ export default function ValidationError(errors, value, field, type) {
   this.errors = []
   this.inner  = []
 
-  if ( errors )
+  if (errors)
     [].concat(errors).forEach(err => {
       this.errors = this.errors.concat(err.errors || err)
 
-      if ( err.inner )
+      if (err.inner)
         this.inner = this.inner.concat(err.inner.length ? err.inner : err)
     })
 
   this.message = this.errors.length > 1
-    ? `${this.errors.length } errors occurred`
+    ? `${this.errors.length} errors occurred`
     : this.errors[0]
 
   if (Error.captureStackTrace)
@@ -43,8 +44,7 @@ ValidationError.formatError = function(message, params) {
 
   let fn = ({ path, label, ...params }) => {
     params.path = label || path || 'this'
-
-    return message(params)
+    return typeof message === 'function' ? message(params) : message
   }
 
   return arguments.length === 1 ? fn : fn(params)

src/array.js

@@ -61,6 +61,9 @@ inherits(ArraySchema, MixedSchema, {
     let endEarly  = this._option('abortEarly', options)
     let recursive = this._option('recursive', options)
 
+    let originalValue = options.originalValue != null ?
+      options.originalValue : _value
+
     return MixedSchema.prototype._validate
       .call(this, _value, options)
       .catch(propagateErrors(endEarly, errors))
@@ -70,6 +73,8 @@ inherits(ArraySchema, MixedSchema, {
           return value
         }
 
+        originalValue = originalValue || value
+
         let validations = value.map((item, idx) => {
           var path  = makePath`${options.path}[${idx}]`
 
@@ -78,7 +83,8 @@ inherits(ArraySchema, MixedSchema, {
             ...options,
             path,
             strict: true,
-            parent: value
+            parent: value,
+            originalValue: originalValue[idx]
           };
 
           if (subType.validate)

src/locale.js

@@ -1,13 +1,25 @@
+import printValue from './util/printValue';
 import { getLocale } from './customLocale'
 
 const customLocale = getLocale()
 
 export let mixed = {
   default:   '${path} is invalid',
-  notType:   '${path} must be a `${type}` type, got: "${value}" instead',
   required:  '${path} is a required field',
   oneOf:     '${path} must be one the following values: ${values}',
   notOneOf:  '${path} must not be one the following values: ${values}',
+  notType:   ({ path, type, value, originalValue }) => {
+    let isCast = originalValue != null && originalValue !== value
+    let msg = `${path} must be a \`${type}\` type, ` +
+      `but the final value was: \`${printValue(value, true)}\`` + (isCast ?
+      ` (cast from the value \`${printValue(originalValue, true)}\`).` : '.')
+
+    if (value === null) {
+      msg += `\n If "null" is intended as an empty value be sure to mark the schema as \`.nullable()\``
+    }
+
+    return msg;
+  },
   ...customLocale.mixed,
 }
 

src/mixed.js

@@ -7,6 +7,7 @@ import merge from './util/merge';
 import isAbsent from './util/isAbsent';
 import cloneDeep from './util/clone';
 import createValidation from './util/createValidation';
+import printValue from './util/printValue';
 import BadSet from './util/set';
 import Ref from './Reference';
 
@@ -140,14 +141,14 @@ SchemaType.prototype = {
       options.assert !== false &&
       resolvedSchema.isType(result) !== true
     ) {
-      let formattedValue = JSON.stringify(value);
-      let formattedResult = JSON.stringify(result);
+      let formattedValue = printValue(value);
+      let formattedResult = printValue(result);
       throw new TypeError(
         `The value of ${options.path || 'field'} could not be cast to a value ` +
         `that satisfies the schema type: "${resolvedSchema._type}". \n\n` +
-        `attempted value: ${JSON.stringify(value)} \n` +
+        `attempted value: ${formattedValue} \n` +
         ((formattedResult !== formattedValue)
-          ? `result of cast: ${JSON.stringify(result)}` : '')
+          ? `result of cast: ${formattedResult}` : '')
       );
     }
 
@@ -173,6 +174,8 @@ SchemaType.prototype = {
 
   _validate(_value, options = {}) {
     let value  = _value;
+    let originalValue = options.originalValue != null ?
+      options.originalValue : _value
 
     let isStrict = this._option('strict', options)
     let endEarly = this._option('abortEarly', options)
@@ -181,11 +184,10 @@ SchemaType.prototype = {
     let label = this._label
 
     if (!isStrict) {
-
       value = this._cast(value, { assert: false, ...options })
     }
     // value is cast, we can check if it meets type requirements
-    let validationParams = { value, path, schema: this, options, label }
+    let validationParams = { value, path, schema: this, options, label, originalValue }
     let initialTests = []
 
     if (this._typeError)
@@ -429,11 +431,3 @@ Object.keys(aliases).forEach(method => {
   )
 })
 
-function nodeify(promise, cb){
-  if (typeof cb !== 'function') return promise
-
-  promise.then(
-    val => cb(null, val),
-    err => cb(err)
-  )
-}

src/object.js

@@ -121,13 +121,15 @@ inherits(ObjectSchema, MixedSchema, {
   },
 
   _validate(_value, opts = {}) {
-    var errors = []
-      , endEarly, recursive;
+    let endEarly, recursive;
+    let errors = []
+    let originalValue = opts.originalValue != null ?
+      opts.originalValue : _value
 
     endEarly = this._option('abortEarly', opts)
     recursive = this._option('recursive', opts)
 
-    opts = {...opts, __validating: true };
+    opts = { ...opts, __validating: true, originalValue };
 
     return MixedSchema.prototype._validate
       .call(this, _value, opts)
@@ -138,14 +140,22 @@ inherits(ObjectSchema, MixedSchema, {
           return value
         }
 
+        originalValue = originalValue || value
+
         let validations = this._nodes.map(key => {
-          var path  = makePath`${opts.path}.${key}`
-            , field = this.fields[key]
-            , innerOptions = { ...opts, path, parent: value };
+          let path  = makePath`${opts.path}.${key}`
+          let field = this.fields[key]
+
+          let innerOptions = {
+            ...opts,
+            path,
+            parent: value,
+            originalValue: originalValue[key],
+          };
 
           if (field) {
             // inner fields are always strict:
-            // 1. this isn't strict so we just cast the value leaving nested values already cast
+            // 1. this isn't strict so the casting will also have cast inner values
             // 2. this is strict in which case the nested values weren't cast either
             innerOptions.strict = true;
 

src/util/createValidation.js

@@ -8,30 +8,38 @@ function resolveParams(oldParams, newParams, resolve) {
   return mapValues({ ...oldParams, ...newParams }, resolve)
 }
 
-function createErrorFactory({ value, label, resolve, ...opts}) {
+function createErrorFactory({ value, label, resolve, originalValue, ...opts}) {
   return function createError({ path = opts.path, message = opts.message, type = opts.name, params } = {}) {
-    params = { path, value, label, ...resolveParams(opts.params, params, resolve) };
+    params = {
+      path,
+      value,
+      originalValue,
+      label,
+      ...resolveParams(opts.params, params, resolve)
+    };
 
-    return Object.assign(new ValidationError(
-        typeof message ==='string' ? formatError(message, params) : message
+    return Object.assign(
+      new ValidationError(
+        formatError(message, params)
       , value
       , path
-      , type)
+      , type
+    )
     , { params })
   }
 }
 
 export default function createValidation(options) {
   let { name, message, test, params } = options
 
-  function validate({ value, path, label, options, ...rest }) {
+  function validate({ value, path, label, options, originalValue, ...rest }) {
     let parent = options.parent;
     var resolve = (value) => Ref.isRef(value)
       ? value.getValue(parent, options.context)
       : value
 
     var createError = createErrorFactory({
-        message, path, value, params
+        message, path, value, originalValue, params
       , label, resolve, name
     })
 

src/util/printValue.js

@@ -0,0 +1,67 @@
+import isFunction from 'lodash/isFunction';
+import isMap from 'lodash/isMap';
+import isSet from 'lodash/isSet';
+import isWeakMap from 'lodash/isWeakMap';
+import isWeakSet from 'lodash/isWeakSet';
+import isSymbol from 'lodash/isSymbol';
+
+const toString = Object.prototype.toString;
+const toISOString = Date.prototype.toISOString;
+const errorToString = Error.prototype.toString;
+const regExpToString = RegExp.prototype.toString;
+const symbolToString = Symbol.prototype.toString;
+
+const SYMBOL_REGEXP = /^Symbol\((.*)\)(.*)$/;
+const NEWLINE_REGEXP = /\n/gi;
+
+const getSymbols = Object.getOwnPropertySymbols || (obj => []);
+
+
+function printNumber(val) {
+  if (val != +val) return 'NaN';
+  const isNegativeZero = val === 0 && 1 / val < 0;
+  return isNegativeZero ? '-0' : '' + val;
+}
+
+function printFunction(val) {
+  return '[Function ' + (val.name || 'anonymous') + ']';
+}
+
+function printSymbol(val: Symbol): string {
+  return symbolToString.call(val).replace(SYMBOL_REGEXP, 'Symbol($1)');
+}
+
+function printError(val: Error): string {
+  return '[' + errorToString.call(val) + ']';
+}
+
+function printSimpleValue(val, quoteStrings = false) {
+  if (val === true || val === false) return '' + val;
+  if (val === undefined) return 'undefined';
+  if (val === null) return 'null';
+
+  const typeOf = typeof val;
+
+  if (typeOf === 'number') return printNumber(val);
+  if (typeOf === 'string') return quoteStrings ? `"${val}"` : val;
+  if (isFunction(val)) return printFunction(val);
+  if (isSymbol(val)) return printSymbol(val);
+
+  const tag = toString.call(val);
+  if (tag === '[object Date]') return isNaN(val.getTime()) ? String(val) : toISOString.call(val);
+  if (tag === '[object Error]' || val instanceof Error) return printError(val);
+  if (tag === '[object RegExp]') return regExpToString.call(val);
+
+  return null;
+}
+
+export default function printValue(value, quoteStrings) {
+  let result = printSimpleValue(value, quoteStrings);
+  if (result !== null) return result;
+
+  return JSON.stringify(value, function (key, value) {
+    let result = printSimpleValue(this[key], quoteStrings);
+    if (result !== null) return result
+    return value
+  }, 2)
+}

test/ValidationError.js

@@ -40,6 +40,13 @@ describe('ValidationError', function() {
       str.should.contain('null')
     })
 
+    it(`should include "NaN" in the message if null is provided as a param`, function() {
+      const str = ValidationError.formatError('${path} value is ${min}', {
+        min: NaN
+      })
+      str.should.contain('NaN')
+    })
+
     it(`should include 0 in the message if 0 is provided as a param`, function() {
       const str = ValidationError.formatError('${path} value is ${min}', {
         min: 0

test/mixed.js

@@ -1,7 +1,7 @@
 import mixed from '../src/mixed';
 import object from '../src/object';
 import string from '../src/string';
-import ValidationError from '../src/ValidationError';
+import number from '../src/number';
 import reach from '../src/util/reach';
 
 let noop = () => {}
@@ -32,6 +32,20 @@ describe( 'Mixed Types ', function(){
     inst.cast(undefined).should.equal('hello')
   })
 
+  it('should warn about null types', async () => {
+    let error = await string().strict()
+      .validate(null).should.be.rejected()
+
+    expect(error.message).to.match(/If "null" is intended/)
+  })
+
+  it('should print the original value', async () => {
+    let error = await number()
+      .validate('john').should.be.rejected()
+
+    expect(error.message).to.match(/the final value was: `NaN`.+cast from the value `"john"`/)
+  })
+
   it('should check types', async () => {
     let inst = string().strict().typeError('must be a ${type}!')