From a646761ba6029b7db4a80df0efe1afb90df0e50d Mon Sep 17 00:00:00 2001
From: Nicolas Morel <nicolas@morel.io>
Date: Sat, 25 Jun 2016 16:24:49 +0200
Subject: [PATCH] Allow all return values in custom validators

Fixes #899.
---
 API.md              |  6 +++---
 lib/alternatives.js |  4 ++--
 lib/any.js          |  6 +++---
 lib/array.js        |  8 ++++----
 lib/binary.js       |  6 +++---
 lib/date.js         |  2 +-
 lib/errors.js       |  6 +++---
 lib/index.js        | 17 ++++++++---------
 lib/number.js       | 12 ++++++------
 lib/object.js       | 37 +++++++++++++++++++------------------
 lib/string.js       | 28 ++++++++++++++--------------
 test/helper.js      |  1 +
 test/index.js       |  4 ++--
 13 files changed, 69 insertions(+), 68 deletions(-)

diff --git a/API.md b/API.md
index 457ce4f9c..09f8c4789 100755
--- a/API.md
+++ b/API.md
@@ -311,7 +311,7 @@ const customJoi = Joi.extend({
             return Math.round(value); // Change the value
         }
 
-        return null; // Keep the value as it was
+        return value; // Keep the value as it was
     },
     rules: [
         {
@@ -327,7 +327,7 @@ const customJoi = Joi.extend({
                     return this.createError('number.round', { v: value }, state, options);
                 }
 
-                return null; // Everything is OK
+                return value; // Everything is OK
             }
         },
         {
@@ -342,7 +342,7 @@ const customJoi = Joi.extend({
                     return this.createError('number.dividable', { v: value, q: params.q }, state, options);
                 }
 
-                return null; // Everything is OK
+                return value; // Everything is OK
             }
         }
     ]
diff --git a/lib/alternatives.js b/lib/alternatives.js
index d871cc01f..ea1ec4269 100755
--- a/lib/alternatives.js
+++ b/lib/alternatives.js
@@ -90,9 +90,9 @@ internals.Alternatives = class extends Any {
         const obj = this.clone();
         let is = Cast.schema(options.is);
 
-        if (options.is === null || !options.is.isJoi) {
+        if (options.is === null || !(Ref.isRef(options.is) || options.is instanceof Any)) {
 
-            // Only apply required if this wasn't already a schema, we'll suppose people know what they're doing
+            // Only apply required if this wasn't already a schema or a ref, we'll suppose people know what they're doing
             is = is.required();
         }
 
diff --git a/lib/any.js b/lib/any.js
index c086e9f10..192742cbb 100755
--- a/lib/any.js
+++ b/lib/any.js
@@ -115,7 +115,7 @@ module.exports = internals.Any = class {
 
     concat(schema) {
 
-        Hoek.assert(schema && schema.isJoi, 'Invalid schema object');
+        Hoek.assert(schema instanceof internals.Any, 'Invalid schema object');
         Hoek.assert(this._type === 'any' || schema._type === 'any' || schema._type === this._type, 'Cannot merge type', this._type, 'with another type:', schema._type);
 
         let obj = this.clone();
@@ -566,13 +566,13 @@ module.exports = internals.Any = class {
         for (let i = 0; i < this._tests.length; ++i) {
             const test = this._tests[i];
             const ret = test.func.call(this, value, state, options);
-            if (ret && ret.isJoi) {
+            if (ret instanceof Errors.Err) {
                 errors.push(ret);
                 if (options.abortEarly) {
                     return finish();
                 }
             }
-            else if (ret !== null) {
+            else {
                 value = ret;
             }
         }
diff --git a/lib/array.js b/lib/array.js
index 10bf41ddd..65c75f0c0 100755
--- a/lib/array.js
+++ b/lib/array.js
@@ -395,7 +395,7 @@ internals.Array = class extends Any {
         return this._test('min', limit, function (value, state, options) {
 
             if (value.length >= limit) {
-                return null;
+                return value;
             }
 
             return this.createError('array.min', { limit, value }, state, options);
@@ -409,7 +409,7 @@ internals.Array = class extends Any {
         return this._test('max', limit, function (value, state, options) {
 
             if (value.length <= limit) {
-                return null;
+                return value;
             }
 
             return this.createError('array.max', { limit, value }, state, options);
@@ -423,7 +423,7 @@ internals.Array = class extends Any {
         return this._test('length', limit, function (value, state, options) {
 
             if (value.length === limit) {
-                return null;
+                return value;
             }
 
             return this.createError('array.length', { limit, value }, state, options);
@@ -475,7 +475,7 @@ internals.Array = class extends Any {
                 }
             }
 
-            return null;
+            return value;
         });
     }
 
diff --git a/lib/binary.js b/lib/binary.js
index e4e3fed72..6d9851f55 100755
--- a/lib/binary.js
+++ b/lib/binary.js
@@ -55,7 +55,7 @@ internals.Binary = class extends Any {
         return this._test('min', limit, function (value, state, options) {
 
             if (value.length >= limit) {
-                return null;
+                return value;
             }
 
             return this.createError('binary.min', { limit, value }, state, options);
@@ -69,7 +69,7 @@ internals.Binary = class extends Any {
         return this._test('max', limit, function (value, state, options) {
 
             if (value.length <= limit) {
-                return null;
+                return value;
             }
 
             return this.createError('binary.max', { limit, value }, state, options);
@@ -83,7 +83,7 @@ internals.Binary = class extends Any {
         return this._test('length', limit, function (value, state, options) {
 
             if (value.length === limit) {
-                return null;
+                return value;
             }
 
             return this.createError('binary.length', { limit, value }, state, options);
diff --git a/lib/date.js b/lib/date.js
index 4e0fd57ff..e4910df00 100755
--- a/lib/date.js
+++ b/lib/date.js
@@ -176,7 +176,7 @@ internals.compare = function (type, compare) {
             }
 
             if (compare(value.getTime(), compareTo)) {
-                return null;
+                return value;
             }
 
             return this.createError('date.' + type, { limit: new Date(compareTo) }, state, options);
diff --git a/lib/errors.js b/lib/errors.js
index 01f3cc472..28f48a5d0 100755
--- a/lib/errors.js
+++ b/lib/errors.js
@@ -22,7 +22,7 @@ internals.stringify = function (value, wrapArrays) {
         return value;
     }
 
-    if (value instanceof internals.Err || type === 'function') {
+    if (value instanceof exports.Err || type === 'function') {
         return value.toString();
     }
 
@@ -43,7 +43,7 @@ internals.stringify = function (value, wrapArrays) {
     return JSON.stringify(value);
 };
 
-internals.Err = class {
+exports.Err = class {
 
     constructor(type, context, state, options, flags) {
 
@@ -97,7 +97,7 @@ internals.Err = class {
 
 exports.create = function (type, context, state, options, flags) {
 
-    return new internals.Err(type, context, state, options, flags);
+    return new exports.Err(type, context, state, options, flags);
 };
 
 
diff --git a/lib/index.js b/lib/index.js
index 9de6ecf4a..c170bcb7b 100755
--- a/lib/index.js
+++ b/lib/index.js
@@ -5,6 +5,7 @@
 const Hoek = require('hoek');
 const Any = require('./any');
 const Cast = require('./cast');
+const Errors = require('./errors');
 const Lazy = require('./lazy');
 const Ref = require('./ref');
 
@@ -151,7 +152,7 @@ internals.root = function () {
 
     root.reach = function (schema, path) {
 
-        Hoek.assert(schema && schema.isJoi, 'you must provide a joi schema');
+        Hoek.assert(schema && schema instanceof Any, 'you must provide a joi schema');
         Hoek.assert(typeof path === 'string', 'path must be a string');
 
         if (path === '') {
@@ -218,12 +219,11 @@ internals.root = function () {
                     let ret;
                     if (extension.coerce) {
                         ret = extension.coerce.call(this, value, state, options);
-                        if (ret && ret.isJoi) {
+                        if (ret instanceof Errors.Err) {
                             return { value, errors: ret };
                         }
-                        else if (ret !== null) {
-                            value = ret;
-                        }
+
+                        value = ret;
                     }
 
                     if (ctor.prototype._base) {
@@ -238,12 +238,11 @@ internals.root = function () {
 
                     if (extension.pre) {
                         ret = extension.pre.call(this, value, state, options);
-                        if (ret && ret.isJoi) {
+                        if (ret instanceof Errors.Err) {
                             return { value, errors: ret };
                         }
-                        else if (ret !== null) {
-                            return { value: ret };
-                        }
+
+                        return { value: ret };
                     }
 
                     return { value };
diff --git a/lib/number.js b/lib/number.js
index 6e5b2d515..366faf1e8 100755
--- a/lib/number.js
+++ b/lib/number.js
@@ -69,7 +69,7 @@ internals.Number = class extends Any {
             }
 
             if (value % divisor === 0) {
-                return null;
+                return value;
             }
 
             return this.createError('number.multiple', { multiple: base, value }, state, options);
@@ -80,7 +80,7 @@ internals.Number = class extends Any {
 
         return this._test('integer', undefined, function (value, state, options) {
 
-            return Hoek.isInteger(value) ? null : this.createError('number.integer', { value }, state, options);
+            return Hoek.isInteger(value) ? value : this.createError('number.integer', { value }, state, options);
         });
     }
 
@@ -89,7 +89,7 @@ internals.Number = class extends Any {
         return this._test('negative', undefined, function (value, state, options) {
 
             if (value < 0) {
-                return null;
+                return value;
             }
 
             return this.createError('number.negative', { value }, state, options);
@@ -101,7 +101,7 @@ internals.Number = class extends Any {
         return this._test('positive', undefined, function (value, state, options) {
 
             if (value > 0) {
-                return null;
+                return value;
             }
 
             return this.createError('number.positive', { value }, state, options);
@@ -118,7 +118,7 @@ internals.Number = class extends Any {
             const places = value.toString().match(internals.precisionRx);
             const decimals = Math.max((places[1] ? places[1].length : 0) - (places[2] ? parseInt(places[2], 10) : 0), 0);
             if (decimals <= limit) {
-                return null;
+                return value;
             }
 
             return this.createError('number.precision', { limit, value }, state, options);
@@ -155,7 +155,7 @@ internals.compare = function (type, compare) {
             }
 
             if (compare(value, compareTo)) {
-                return null;
+                return value;
             }
 
             return this.createError('number.' + type, { limit: compareTo, value }, state, options);
diff --git a/lib/object.js b/lib/object.js
index f0986c574..93cc4939c 100755
--- a/lib/object.js
+++ b/lib/object.js
@@ -5,6 +5,7 @@
 const Hoek = require('hoek');
 const Topo = require('topo');
 const Any = require('./any');
+const Errors = require('./errors');
 const Cast = require('./cast');
 const Ref = require('./ref');
 
@@ -244,7 +245,7 @@ internals.Object = class extends Any {
         for (let i = 0; i < this._inner.dependencies.length; ++i) {
             const dep = this._inner.dependencies[i];
             const err = internals[dep.type].call(this, dep.key !== null && value[dep.key], dep.peers, target, { key: dep.key, path: (state.path || '') + (dep.key ? '.' + dep.key : '') }, options);
-            if (err) {
+            if (err instanceof Errors.Err) {
                 errors.push(err);
                 if (options.abortEarly) {
                     return finish();
@@ -265,7 +266,7 @@ internals.Object = class extends Any {
     keys(schema) {
 
         Hoek.assert(schema === null || schema === undefined || typeof schema === 'object', 'Object schema must be a valid object');
-        Hoek.assert(!schema || !schema.isJoi, 'Object schema cannot be a joi schema');
+        Hoek.assert(!schema || !(schema instanceof Any), 'Object schema cannot be a joi schema');
 
         const obj = this.clone();
 
@@ -330,7 +331,7 @@ internals.Object = class extends Any {
         return this._test('length', limit, function (value, state, options) {
 
             if (Object.keys(value).length === limit) {
-                return null;
+                return value;
             }
 
             return this.createError('object.length', { limit }, state, options);
@@ -344,7 +345,7 @@ internals.Object = class extends Any {
         return this._test('arity', n, function (value, state, options) {
 
             if (value.length === n) {
-                return null;
+                return value;
             }
 
             return this.createError('function.arity', { n }, state, options);
@@ -358,7 +359,7 @@ internals.Object = class extends Any {
         return this._test('minArity', n, function (value, state, options) {
 
             if (value.length >= n) {
-                return null;
+                return value;
             }
 
             return this.createError('function.minArity', { n }, state, options);
@@ -372,7 +373,7 @@ internals.Object = class extends Any {
         return this._test('maxArity', n, function (value, state, options) {
 
             if (value.length <= n) {
-                return null;
+                return value;
             }
 
             return this.createError('function.maxArity', { n }, state, options);
@@ -386,7 +387,7 @@ internals.Object = class extends Any {
         return this._test('min', limit, function (value, state, options) {
 
             if (Object.keys(value).length >= limit) {
-                return null;
+                return value;
             }
 
             return this.createError('object.min', { limit }, state, options);
@@ -400,7 +401,7 @@ internals.Object = class extends Any {
         return this._test('max', limit, function (value, state, options) {
 
             if (Object.keys(value).length <= limit) {
-                return null;
+                return value;
             }
 
             return this.createError('object.max', { limit }, state, options);
@@ -436,7 +437,7 @@ internals.Object = class extends Any {
         return this._test('schema', null, function (value, state, options) {
 
             if (value instanceof Any) {
-                return null;
+                return value;
             }
 
             return this.createError('object.schema', null, state, options);
@@ -633,7 +634,7 @@ internals.Object = class extends Any {
 
             const result = schema._validate(ref(value), null, options, value);
             if (!result.errors) {
-                return null;
+                return value;
             }
 
             const localState = Hoek.merge({}, state);
@@ -651,7 +652,7 @@ internals.Object = class extends Any {
         return this._test('type', name, function (value, state, options) {
 
             if (value instanceof constructor) {
-                return null;
+                return value;
             }
 
             return this.createError('object.type', { type: name }, state, options);
@@ -663,7 +664,7 @@ internals.Object = class extends Any {
         return this._test('ref', null, function (value, state, options) {
 
             if (Ref.isRef(value)) {
-                return null;
+                return value;
             }
 
             return this.createError('function.ref', null, state, options);
@@ -710,7 +711,7 @@ internals.groupChildren = function (children) {
 internals.with = function (value, peers, parent, state, options) {
 
     if (value === undefined) {
-        return null;
+        return value;
     }
 
     for (let i = 0; i < peers.length; ++i) {
@@ -722,14 +723,14 @@ internals.with = function (value, peers, parent, state, options) {
         }
     }
 
-    return null;
+    return value;
 };
 
 
 internals.without = function (value, peers, parent, state, options) {
 
     if (value === undefined) {
-        return null;
+        return value;
     }
 
     for (let i = 0; i < peers.length; ++i) {
@@ -741,7 +742,7 @@ internals.without = function (value, peers, parent, state, options) {
         }
     }
 
-    return null;
+    return value;
 };
 
 
@@ -758,7 +759,7 @@ internals.xor = function (value, peers, parent, state, options) {
     }
 
     if (present.length === 1) {
-        return null;
+        return value;
     }
 
     if (present.length === 0) {
@@ -775,7 +776,7 @@ internals.or = function (value, peers, parent, state, options) {
         const peer = peers[i];
         if (Object.prototype.hasOwnProperty.call(parent, peer) &&
             parent[peer] !== undefined) {
-            return null;
+            return value;
         }
     }
 
diff --git a/lib/string.js b/lib/string.js
index 1ba780396..717bbfe87 100755
--- a/lib/string.js
+++ b/lib/string.js
@@ -100,7 +100,7 @@ internals.String = class extends Any {
         return this._test('regex', pattern, function (value, state, options) {
 
             if (pattern.test(value)) {
-                return null;
+                return value;
             }
 
             return this.createError((name ? 'string.regex.name' : 'string.regex.base'), { name, pattern, value }, state, options);
@@ -112,7 +112,7 @@ internals.String = class extends Any {
         return this._test('alphanum', undefined, function (value, state, options) {
 
             if (/^[a-zA-Z0-9]+$/.test(value)) {
-                return null;
+                return value;
             }
 
             return this.createError('string.alphanum', { value }, state, options);
@@ -124,7 +124,7 @@ internals.String = class extends Any {
         return this._test('token', undefined, function (value, state, options) {
 
             if (/^\w+$/.test(value)) {
-                return null;
+                return value;
             }
 
             return this.createError('string.token', { value }, state, options);
@@ -151,7 +151,7 @@ internals.String = class extends Any {
             try {
                 const result = Isemail.validate(value, isEmailOptions);
                 if (result === true || result === 0) {
-                    return null;
+                    return value;
                 }
             }
             catch (e) { }
@@ -209,7 +209,7 @@ internals.String = class extends Any {
         return this._test('ip', ipOptions, function (value, state, options) {
 
             if (regex.test(value)) {
-                return null;
+                return value;
             }
 
             if (versions) {
@@ -269,7 +269,7 @@ internals.String = class extends Any {
         return this._test('uri', uriOptions, function (value, state, options) {
 
             if (regex.test(value)) {
-                return null;
+                return value;
             }
 
             if (customScheme) {
@@ -285,7 +285,7 @@ internals.String = class extends Any {
         return this._test('isoDate', undefined, function (value, state, options) {
 
             if (JoiDate._isIsoDate(value)) {
-                return null;
+                return value;
             }
 
             return this.createError('string.isoDate', { value }, state, options);
@@ -300,7 +300,7 @@ internals.String = class extends Any {
         return this._test('guid', undefined, function (value, state, options) {
 
             if (regex.test(value) || regex2.test(value)) {
-                return null;
+                return value;
             }
 
             return this.createError('string.guid', { value }, state, options);
@@ -314,7 +314,7 @@ internals.String = class extends Any {
         return this._test('hex', regex, function (value, state, options) {
 
             if (regex.test(value)) {
-                return null;
+                return value;
             }
 
             return this.createError('string.hex', { value }, state, options);
@@ -330,7 +330,7 @@ internals.String = class extends Any {
             if ((value.length <= 255 && regex.test(value)) ||
                 Net.isIPv6(value)) {
 
-                return null;
+                return value;
             }
 
             return this.createError('string.hostname', { value }, state, options);
@@ -344,7 +344,7 @@ internals.String = class extends Any {
             if (options.convert ||
                 value === value.toLocaleLowerCase()) {
 
-                return null;
+                return value;
             }
 
             return this.createError('string.lowercase', { value }, state, options);
@@ -361,7 +361,7 @@ internals.String = class extends Any {
             if (options.convert ||
                 value === value.toLocaleUpperCase()) {
 
-                return null;
+                return value;
             }
 
             return this.createError('string.uppercase', { value }, state, options);
@@ -378,7 +378,7 @@ internals.String = class extends Any {
             if (options.convert ||
                 value === value.trim()) {
 
-                return null;
+                return value;
             }
 
             return this.createError('string.trim', { value }, state, options);
@@ -446,7 +446,7 @@ internals.compare = function (type, compare) {
             }
 
             if (compare(value, compareTo, encoding)) {
-                return null;
+                return value;
             }
 
             return this.createError('string.' + type, { limit: compareTo, value, encoding }, state, options);
diff --git a/test/helper.js b/test/helper.js
index eba9f8ee9..7dd80af01 100755
--- a/test/helper.js
+++ b/test/helper.js
@@ -70,6 +70,7 @@ exports.validateOptions = function (schema, config, options, callback) {
     }
     catch (err) {
 
+        console.error(err.stack);
         // Reframe the error location, we don't care about the helper
         err.at = internals.thrownAt();
         throw err;
diff --git a/test/index.js b/test/index.js
index feb60ccb6..d0de2b1bd 100755
--- a/test/index.js
+++ b/test/index.js
@@ -2414,7 +2414,7 @@ describe('Joi', () => {
             const customJoi = Joi.extend({
                 coerce(value, state, options) {
 
-                    return null;
+                    return value;
                 },
                 name: 'myType'
             });
@@ -2432,7 +2432,7 @@ describe('Joi', () => {
             const customJoi = Joi.extend({
                 pre(value, state, options) {
 
-                    return null;
+                    return value;
                 },
                 name: 'myType'
             });