From a8528e6a6cace86b240c6a7e9245e8ca636161b2 Mon Sep 17 00:00:00 2001 From: "Benjamin E. Coe" Date: Sat, 18 Feb 2017 12:08:49 -0800 Subject: [PATCH] fix: address positional argument strict() bug introduced in #766 (#784) --- lib/command.js | 22 ++++++++++++++-------- lib/validation.js | 3 ++- test/command.js | 25 +++++++++++++++++++++++++ yargs.js | 6 +++--- 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/lib/command.js b/lib/command.js index e0eeb1594..89dadc1f9 100644 --- a/lib/command.js +++ b/lib/command.js @@ -140,6 +140,7 @@ module.exports = function (yargs, usage, validation) { // what does yargs look like after the buidler is run? var innerArgv = parsed.argv var innerYargs = null + var positionalMap = {} currentContext.commands.push(command) if (typeof commandHandler.builder === 'function') { @@ -173,15 +174,17 @@ module.exports = function (yargs, usage, validation) { aliases = innerYargs.parsed.aliases } - if (!yargs._hasOutput()) populatePositionals(commandHandler, innerArgv, currentContext, yargs) - - if (commandHandler.handler && !yargs._hasOutput()) { - commandHandler.handler(innerArgv) + if (!yargs._hasOutput()) { + positionalMap = populatePositionals(commandHandler, innerArgv, currentContext, yargs) } // we apply validation post-hoc, so that custom // checks get passed populated positional arguments. - yargs._runValidation(innerArgv, aliases) + yargs._runValidation(innerArgv, aliases, positionalMap) + + if (commandHandler.handler && !yargs._hasOutput()) { + commandHandler.handler(innerArgv) + } currentContext.commands.pop() numFiles = currentContext.files.length - numFiles @@ -196,25 +199,27 @@ module.exports = function (yargs, usage, validation) { argv._ = argv._.slice(context.commands.length) // nuke the current commands var demanded = commandHandler.demanded.slice(0) var optional = commandHandler.optional.slice(0) + var positionalMap = {} validation.positionalCount(demanded.length, argv._.length) while (demanded.length) { var demand = demanded.shift() - populatePositional(demand, argv, yargs) + populatePositional(demand, argv, yargs, positionalMap) } while (optional.length) { var maybe = optional.shift() - populatePositional(maybe, argv, yargs) + populatePositional(maybe, argv, yargs, positionalMap) } argv._ = context.commands.concat(argv._) + return positionalMap } // populate a single positional argument and its // aliases onto argv. - function populatePositional (positional, argv, yargs) { + function populatePositional (positional, argv, yargs, positionalMap) { // "positional" consists of the positional.cmd, an array representing // the positional's name and aliases, and positional.variadic // indicating whether or not it is a variadic array. @@ -229,6 +234,7 @@ module.exports = function (yargs, usage, validation) { if (value) argv[cmd] = value else argv[cmd] = value = argv._.shift() } + positionalMap[cmd] = true postProcessPositional(yargs, argv, cmd) addCamelCaseExpansions(argv, cmd) } diff --git a/lib/validation.js b/lib/validation.js index f523c46ea..cc0ea6514 100644 --- a/lib/validation.js +++ b/lib/validation.js @@ -116,7 +116,7 @@ module.exports = function (yargs, usage, y18n) { } // check for unknown arguments (strict-mode). - self.unknownArguments = function (argv, aliases) { + self.unknownArguments = function (argv, aliases, positionalMap) { const aliasLookup = {} const descriptions = usage.getDescriptions() const demandedOptions = yargs.getDemandedOptions() @@ -134,6 +134,7 @@ module.exports = function (yargs, usage, y18n) { if (key !== '$0' && key !== '_' && !descriptions.hasOwnProperty(key) && !demandedOptions.hasOwnProperty(key) && + !positionalMap.hasOwnProperty(key) && !aliasLookup.hasOwnProperty(key)) { unknown.push(key) } diff --git a/test/command.js b/test/command.js index ebdf881bc..9396e144e 100644 --- a/test/command.js +++ b/test/command.js @@ -986,6 +986,31 @@ describe('Command', function () { yargs.argv // parse and run command commandCalled.should.be.true }) + + // address regression introduced in #766, thanks @nexdrew! + it('does not fail strict check due to postional command arguments', function (done) { + yargs() + .strict() + .command('hi ', 'The hi command') + .parse('hi ben', function (err, argv, output) { + expect(err).to.equal(null) + return done() + }) + }) + + it('does not fire command if validation fails', function (done) { + var commandRun = false + yargs() + .strict() + .command('hi ', 'The hi command', function () {}, function (argv) { + commandRun = true + }) + .parse('hi ben --hello=world', function (err, argv, output) { + commandRun.should.equal(false) + err.message.should.equal('Unknown argument: hello') + return done() + }) + }) }) describe('types', function () { diff --git a/yargs.js b/yargs.js index b36812e5a..f39150db3 100644 --- a/yargs.js +++ b/yargs.js @@ -1051,18 +1051,18 @@ function Yargs (processArgs, cwd, parentRequire) { // if we're executed via bash completion, don't // bother with validation. if (!argv[completion.completionKey]) { - self._runValidation(argv, aliases) + self._runValidation(argv, aliases, {}) } } return setPlaceholderKeys(argv) } - self._runValidation = function (argv, aliases) { + self._runValidation = function (argv, aliases, positionalMap) { validation.nonOptionCount(argv) validation.missingArgumentValue(argv) validation.requiredArguments(argv) - if (strict) validation.unknownArguments(argv, aliases) + if (strict) validation.unknownArguments(argv, aliases, positionalMap) validation.customChecks(argv, aliases) validation.limitedChoices(argv) validation.implications(argv)