Repository: mattallty/Caporal.js (ts)

Issue:
Support promise based validation
Like with the completion feature, it would be nice to support promise based validation of arguments/options.

### Example usage

***Update 5/1/19*** Usage now matches up with how the function validator is supposed to work, had misunderstood this.
``` typescript
function fetchAllNames(): Promise<string[]> {
  ...
}

command('commandname')
  .argument(
    '<name>', 
    'name from a dynamic list of possible names', 
    arg => fetchAllNames().then(names => {
      if (names.includes(arg)) {
        return arg;
      }
      throw new Error(`Valid names are [${names.join(', ')]`);
    })
  )
```



Pull request description:
Promise based validation of arguments and options
Resolves #137 

Adding support for returning a promise from function based argument and option validators

- [x] Argument support
- [x] Option support (WIP)

I chose to continue to allow `validator.validate(?)` to be able to return non-promise results to reduce the external impact, both in the tests and actual usage, of this change. Doing so makes the internals a bit more complex since we have to check whether the result of several calls are promises or not, then take appropriate action. I'm not sure if going the other route, converting `validator.validate(?)` to always return a promise, would have been a breaking change for users but it would have certainly required modifying many of the existing tests.

Patch:
diff --git a/lib/command.js b/lib/command.js
index fc63ac2..56d2f50 100644
--- a/lib/command.js
+++ b/lib/command.js
@@ -1,6 +1,7 @@
 "use strict";
 
 const GetterSetter = require('./utils').GetterSetter;
+const isPromise = require('./utils').isPromise;
 const Option = require('./option');
 const Argument = require('./argument');
 const UnknownOptionError = require('./error/unknown-option');
@@ -243,14 +244,34 @@ class Command extends GetterSetter {
    */
   _validateArgs(args) {
     return Object.keys(args).reduce((acc, key) => {
-      const arg = this._findArgument(key);
-      const value = args[key];
-      try {
-        acc[key] = arg._validate(value);
-      } catch(e) {
-        throw new InvalidArgumentValueError(key, value, this, e, this._program);
+      const addArg = (_acc, _key, _arg) => {
+        _acc[_key] = _arg;
+        return _acc;
       }
-      return acc;
+
+      const throwAsInvalid = (_key, _value, _e) => {
+        throw new InvalidArgumentValueError(_key, _value, this, _e, this._program);
+      }
+
+      const validateArgs = (_acc, _key) => {
+        const arg = this._findArgument(_key);
+        const value = args[_key];
+        try {
+          const result = arg._validate(value);
+          if (isPromise(result)) {
+            return result
+              .then(_arg => addArg(_acc, _key, _arg))
+              .catch(e => throwAsInvalid(_key, value, e))
+          } else {
+            return addArg(_acc, _key, result);
+          }
+        } catch(e) {
+          throwAsInvalid(_key, value, e);
+        }
+      }
+      return isPromise(acc)
+        ? acc.then(_arg => validateArgs(_arg, key))
+        : validateArgs(acc, key);
     }, {});
   }
 
@@ -281,20 +302,40 @@ class Command extends GetterSetter {
    */
   _validateOptions(options) {
     return Object.keys(options).reduce((acc, key) => {
-      if (Command.NATIVE_OPTIONS.indexOf(key) !== -1) {
-        return acc;
-      }
-      const value = acc[key];
-      const opt = this._findOption(key);
-      if (!opt) {
-        throw new UnknownOptionError(key, this, this._program);
+      const addOpt = (_acc, _key, _opt) => {
+        _acc[key] = _opt;
+        return _acc;
       }
-      try {
-        acc[key] = opt._validate(value);
-      } catch(e) {
-        throw new InvalidOptionValueError(key, value, this, e, this._program);
+
+      const throwAsInvalid = (_key, _value, _e) => {
+        throw new InvalidOptionValueError(_key, _value, this, _e, this._program);
       }
-      return acc;
+
+      const validateOptions = (_acc, _key) => {
+        if (Command.NATIVE_OPTIONS.indexOf(key) !== -1) {
+          return _acc;
+        }
+        const value = _acc[key];
+        const opt = this._findOption(key);
+        if (!opt) {
+          throw new UnknownOptionError(key, this, this._program);
+        }
+        try {
+          const result = opt._validate(value);
+          if (isPromise(result)) {
+            return result
+              .then(_opt => addOpt(_acc, _key, _opt))
+              .catch(e => throwAsInvalid(_key, value, e));
+          } else {
+            return addOpt(_acc, _key, result);
+          }
+        } catch(e) {
+          throwAsInvalid(_key, value, e);
+        }
+      };
+      return isPromise(acc)
+        ? acc.then(_acc => validateOptions(_acc, key))
+        : validateOptions(acc, key);
     }, options);
   }
 
@@ -350,15 +391,43 @@ class Command extends GetterSetter {
     args = this._argsArrayToObject(args);
     // arguments validation
     args = this._validateArgs(args);
-    // check required options
-    options = this._checkRequiredOptions(options);
-    // options validation
-    options = this._validateOptions(options);
-    // add long notation if exists
-    options = this._addLongNotationToOptions(options);
-    // camelcase options
-    options = this._camelCaseOptions(options);
-    return {args, options};
+
+    const processOptions = (opts) => {
+      opts = this._checkRequiredOptions(opts);
+      // options validation
+      opts = this._validateOptions(opts);
+
+      const finishProcessingOptions = (_opts) => {
+        // add long notation if exists
+        opts = this._addLongNotationToOptions(opts);
+        // camelcase options
+        opts = this._camelCaseOptions(opts);
+
+        return opts
+      }
+
+      return isPromise(opts)
+        ? opts.then(_opts => finishProcessingOptions(_opts))
+        : finishProcessingOptions(opts);
+    }
+
+    const finish = (_args, opts) => {
+      opts = processOptions(opts);
+      if (isPromise(opts)) {
+        return opts.then(_opts => ({
+          args: _args,
+          options: _opts
+        }))
+      } else {
+        return {args: _args, options: opts}
+      }
+    }
+
+    if (isPromise(args)) {
+      return args.then(_args => finish(_args, options))
+    } else {
+      return finish(args, options);
+    }
   }
 
 
@@ -415,7 +484,7 @@ class Command extends GetterSetter {
     return response
       .catch(err => {
         err = err instanceof Error ? err : new Error(err);
-        return this._program.fatalError(err);
+        throw this._program.fatalError(err);
       })
   }
 
diff --git a/lib/program.js b/lib/program.js
index 6166825..e59d0fa 100644
--- a/lib/program.js
+++ b/lib/program.js
@@ -1,6 +1,7 @@
 "use strict";
 
 const GetterSetter = require('./utils').GetterSetter;
+const isPromise = require('./utils').isPromise;
 const Command = require('./command');
 const parseArgs = require('micromist');
 const Help = require('./help');
@@ -163,6 +164,13 @@ class Program extends GetterSetter {
 
     try {
       validated = cmd._validateCall(argsCopy, options);
+      if (isPromise(validated)) {
+        return validated
+          .then(v => cmd._run(v.args, v.options))
+          .catch(err => {
+            throw this.fatalError(err);
+          })
+      }
     } catch(err) {
       return this.fatalError(err);
     }
diff --git a/lib/utils.js b/lib/utils.js
index 4e0bd4b..380dc26 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -35,3 +35,11 @@ exports.getDashedOption = function getDashedOption(option) {
   return '--' + option;
 };
 
+
+/**
+ *
+ * @param {Object} obj
+ */
+exports.isPromise = function isPromise(obj) {
+  return obj && typeof obj.then === 'function';
+}
diff --git a/lib/validator.js b/lib/validator.js
index a52d6df..0c03baa 100644
--- a/lib/validator.js
+++ b/lib/validator.js
@@ -2,6 +2,7 @@
 
 const constants = require('./constants');
 const ValidationError = require('./error/validation-error');
+const isPromise = require('./utils').isPromise;
 
 class Validator {
 
@@ -138,15 +139,21 @@ class Validator {
    * @private
    */
   _validateWithFunction(value) {
-    try {
-      return this._validator(value);
-    } catch(e) {
+    const throwAsInvalid = (_value, _e) => {
       throw new ValidationError(
         "Function validation failed",
-        {validator: this._validator, value, originalError: e},
+        {validator: this._validator, _value, originalError: _e},
         this._program
       );
     }
+    try {
+      const result = this._validator(value);
+      if (isPromise(result)) {
+        return result.catch(e => throwAsInvalid(value, e));
+      }
+    } catch(e) {
+      throwAsInvalid(value, e);
+    }
   }
 
   /**


Test patch:
diff --git a/tests/actions.js b/tests/actions.js
index 76af41e..ba44d63 100644
--- a/tests/actions.js
+++ b/tests/actions.js
@@ -91,13 +91,11 @@ describe('Setting up a async action', () => {
       .command('foo', 'My foo')
       .action(stub);
 
-    program.parse(makeArgv('foo'));
-
-    setImmediate(function () {
+    program.parse(makeArgv('foo')).then(() => {}).catch(() => {}).then(() => {
       should(stub.callCount).be.eql(1);
       should(fatalError.callCount).be.eql(1);
       done()
-    });
+    })
 
   });
   it(`should fatalError() for a rejected promise (error object)`, (done) => {
@@ -115,9 +113,7 @@ describe('Setting up a async action', () => {
       .command('foo', 'My foo')
       .action(stub);
 
-    program.parse(makeArgv('foo'));
-
-    setImmediate(function () {
+    program.parse(makeArgv('foo')).then(() => {}).catch(() => {}).then(() => {
       should(stub.callCount).be.eql(1);
       should(fatalError.callCount).be.eql(1);
       done()
diff --git a/tests/arg-validation.js b/tests/arg-validation.js
index a0b6196..18aade8 100644
--- a/tests/arg-validation.js
+++ b/tests/arg-validation.js
@@ -76,6 +76,83 @@ describe("Argument validation", function() {
     should(this.fatalError.callCount).eql(0);
   });
 
+  it(`should throw InvalidArgumentValueError for an invalid required argument value (Function validator)`, function() {
+    program
+      .command('foo', 'Fooooo')
+      .argument('<foo>', 'My bar', arg => {
+        const options = ["bim", "bam", "boom"];
+        if (options.includes(arg)) {
+          return arg;
+        }
+        throw new Error();
+      })
+      .action(this.action);
+
+    program.parse(makeArgv(['foo', '827E92']));
+    should(this.fatalError.callCount).eql(1);
+    should(this.fatalError.calledWith(sinon.match.instanceOf(InvalidArgumentValueError))).be.ok();
+  });
+
+  it(`should not throw InvalidArgumentValueError for a valid required argument value (Function validator)`, function() {
+    program
+      .command('foo', 'Fooooo')
+      .argument('<foo>', 'My bar', arg => {
+        const options = ["bim", "bam", "boom"];
+        if (options.includes(arg)) {
+          return arg;
+        }
+        throw new Error();
+      })
+      .action(this.action);
+
+    program.parse(makeArgv(['foo', 'bam']));
+    should(this.fatalError.callCount).eql(0);
+  });
+
+  it(`should throw InvalidArgumentValueError for an invalid required argument value (Promise validator)`, function(done) {
+    program
+      .command('foo', 'Fooooo')
+      .argument('<foo>', 'My bar', arg => new Promise((resolve, reject) => {
+        const options = ["bim", "bam", "boom"];
+        setTimeout(() => {
+          if (options.includes(arg)) {
+            resolve(arg)
+          } else {
+            reject(new Error())
+          }
+        }, 10);
+      }))
+      .action(this.action);
+
+    program.parse(makeArgv(['foo', '827E92']))
+      .catch(e => {
+        should(this.fatalError.callCount).eql(1);
+        should(this.fatalError.calledWith(sinon.match.instanceOf(InvalidArgumentValueError))).be.ok();
+        done()
+      });
+  });
+
+  it(`should not throw InvalidArgumentValueError for a valid required argument value (Promise validator)`, function(done) {
+    program
+      .command('foo', 'Fooooo')
+      .argument('<foo>', 'My bar', arg => new Promise((resolve, reject) => {
+        const options = ["bim", "bam", "boom"];
+        setTimeout(() => {
+          if (options.includes(arg)) {
+            resolve(arg)
+          } else {
+            reject(new Error())
+          }
+        }, 10);
+      }))
+      .action(this.action);
+
+    program.parse(makeArgv(['foo', 'bam'])).then(arg => {
+      should(this.fatalError.callCount).eql(0);
+      done();
+    })
+  });
+
   it(`should take default value if not passed when setting up a default argument value`, function() {
     program
       .command('foo', 'Fooooo')
diff --git a/tests/option-validation.js b/tests/option-validation.js
index df8f455..a045de0 100644
--- a/tests/option-validation.js
+++ b/tests/option-validation.js
@@ -11,14 +11,22 @@ program
 
 
 describe('Passing --option invalid-value', () => {
+  var error;
 
-  ['regex', 'function', 'STRING', 'INT', 'BOOL', 'FLOAT', 'LIST(int)', 'LIST(bool)', 'LIST(float)', 'LIST(repeated)'].forEach(function(checkType) {
-    it(`should throw an error for ${checkType} check`, () => {
+  beforeEach(() => {
+    program.action(function() {});
+    error = sinon.stub(program, "fatalError", function(err) {
+      should(err.name).eql('InvalidOptionValueError');
+    });
+  });
 
-      const error = sinon.stub(program, "fatalError", function(err) {
-        should(err.name).eql('InvalidOptionValueError');
-      });
+  afterEach(() => {
+    error.restore();
+    program.reset();
+  });
 
+  ['regex', 'function', 'STRING', 'INT', 'BOOL', 'FLOAT', 'LIST(int)', 'LIST(bool)', 'LIST(float)', 'LIST(repeated)'].forEach(function(checkType) {
+    it(`should throw an error for ${checkType} check`, () => {
       program.action(function() {});
 
       if (checkType === 'regex') {
@@ -71,23 +79,49 @@ describe('Passing --option invalid-value', () => {
       const count = error.callCount;
 
       should(count).be.eql(1);
+    });
+  });
+
+  it(`should throw an error for promise check`, done => {
+    program.action(function() {});
 
-      program.reset();
-      error.restore();
+    program.option('-t, --time <time-in-secs>', 'Time in seconds, superior to zero', function(val) {
+      return new Promise((resolve, reject) => {
+        setTimeout(() => {
+          const o = parseInt(val);
+          if (isNaN(o) || o <= 0) {
+            reject(new Error("FOOOO"));
+          }
+          resolve(o);
+        }, 10);
+      })
     });
+    // then.catch.then to ensure the assertion is made whether the promise resolves or not (simulates `finally` behavior for node 6 & 8)
+    program.parse(makeArgv(['-t', 'i-am-invalid'])).then(() => {}).catch(() => {}).then(() => {
+      const count = error.callCount;
+
+      should(count).be.eql(1);
+      done();
+    })
   });
 });
 
 
 describe('Passing --option valid-value', () => {
+  var error;
 
-  ['regex', 'function', 'STRING', 'INT', 'BOOL', 'BOOL(implicit)', 'FLOAT', 'LIST(int)', 'LIST(bool)', 'LIST(float)'].forEach(function(checkType) {
-    it(`should succeed for ${checkType} check`, () => {
-
-      const error = sinon.stub(program, "fatalError");
+  beforeEach(() => {
+    program.action(function() {});
+    error = sinon.stub(program, "fatalError");
+  });
 
-      program.action(function() {});
+  afterEach(() => {
+    error.restore();
+    program.reset();
+  });
 
+  ['regex', 'function', 'STRING', 'INT', 'BOOL', 'BOOL(implicit)', 'FLOAT', 'LIST(int)', 'LIST(bool)', 'LIST(float)'].forEach(function(checkType) {
+    it(`should succeed for ${checkType} check`, () => {
       if (checkType === 'regex') {
         program.option('-t, --time <time-in-secs>', 'Time in seconds', /^\d+$/);
         program.parse(makeArgv(['-t', '234']));
@@ -138,10 +172,30 @@ describe('Passing --option valid-value', () => {
       const count = error.callCount;
 
       should(count).be.eql(0);
+    });
+  });
+
+  it(`should succeed for promise check`, done => {
+    program.action(function() {});
 
-      program.reset();
-      error.restore();
+    program.option('-t, --time <time-in-secs>', 'Time in seconds, superior to zero', function(val) {
+      return new Promise((resolve, reject) => {
+        setTimeout(() => {
+          const o = parseInt(val);
+          if (isNaN(o) || o <= 0) {
+            reject(new Error("FOOOO"));
+          }
+          resolve(o);
+        }, 10);
+      })
     });
+    // then.catch.then to ensure the assertion is made whether the promise resolves or not (simulates `finally` behavior for node 6 & 8)
+    program.parse(makeArgv(['-t', '2'])).then(() => {}).catch(() => {}).then(() => {
+      const count = error.callCount;
+      should(count).be.eql(0);
+
+      done();
+    })
   });
 });
 
diff --git a/tests/validator.js b/tests/validator.js
index fb429af..7c2a486 100644
--- a/tests/validator.js
+++ b/tests/validator.js
@@ -77,3 +77,33 @@ describe('Setting up an option with an non-Array validator', () => {
     program.reset();
   });
 });
+
+describe('Setting up an option with a function validator', () => {
+
+  it(`should return empty array for validator.getChoices()`, () => {
+
+    program
+      .command('foo')
+      .option('-t <time-in-secs>', 'my option', opt => opt)
+      .action(function() {});
+
+    program.parse(makeArgv(['foo', '-t', '2982']));
+    should(program.getCommands()[0]._options[0]._validator.getChoices()).be.eql([]);
+    program.reset();
+  })
+})
+
+describe('Setting up an option with a promise validator', () => {
+
+  it(`should return empty array for validator.getChoices()`, () => {
+
+    program
+      .command('foo')
+      .option('-t <time-in-secs>', 'my option', opt => Promise.resolve(opt))
+      .action(function() {});
+
+    program.parse(makeArgv(['foo', '-t', '2982']));
+    should(program.getCommands()[0]._options[0]._validator.getChoices()).be.eql([]);
+    program.reset();
+  })
+})