Skip to content

Commit

Permalink
fix: address positional argument strict() bug introduced in #766
Browse files Browse the repository at this point in the history
  • Loading branch information
Benjamin Coe committed Feb 18, 2017
1 parent cef98c8 commit 2f8b7cc
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 9 deletions.
16 changes: 11 additions & 5 deletions lib/command.js
Expand Up @@ -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') {
Expand Down Expand Up @@ -173,15 +174,17 @@ module.exports = function (yargs, usage, validation) {
aliases = innerYargs.parsed.aliases
}

if (!yargs._hasOutput()) populatePositionals(commandHandler, innerArgv, currentContext, yargs)
if (!yargs._hasOutput()) {
positionalMap = populatePositionals(commandHandler, innerArgv, currentContext, yargs)
}

if (commandHandler.handler && !yargs._hasOutput()) {
commandHandler.handler(innerArgv)
}

// we apply validation post-hoc, so that custom
// checks get passed populated positional arguments.
yargs._runValidation(innerArgv, aliases)
yargs._runValidation(innerArgv, aliases, positionalMap)

currentContext.commands.pop()
numFiles = currentContext.files.length - numFiles
Expand All @@ -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.
Expand All @@ -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)
}
Expand Down
3 changes: 2 additions & 1 deletion lib/validation.js
Expand Up @@ -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()
Expand All @@ -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)
}
Expand Down
11 changes: 11 additions & 0 deletions test/command.js
Expand Up @@ -986,6 +986,17 @@ 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 <name>', 'The hi command')
.parse('hi ben', function (err, argv, output) {
expect(err).to.equal(null)
return done()
})
})
})

describe('types', function () {
Expand Down
6 changes: 3 additions & 3 deletions yargs.js
Expand Up @@ -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)
Expand Down

0 comments on commit 2f8b7cc

Please sign in to comment.