Skip to content

Commit

Permalink
feat: add .strict() mode to error on unknown flags/args (#51)
Browse files Browse the repository at this point in the history
* Add strict mode for unknown options and arguments
* Fix a broken unit test
* Default help command should turn strict mode off
* Improve unknown option getters and add test cases
  • Loading branch information
elliot-nelson authored Mar 29, 2020
1 parent 82e0a8e commit de1eefb
Show file tree
Hide file tree
Showing 4 changed files with 162 additions and 5 deletions.
34 changes: 29 additions & 5 deletions api.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class Api {
commandType: this.getCommand
}
this._showHelpByDefault = 'showHelpByDefault' in opts ? opts.showHelpByDefault : false
this._strictMode = 'strictMode' in opts ? opts.strictMode : false
this._magicCommandAdded = false
this._modulesSeen = opts.modulesSeen || []
this.configure(opts)
Expand All @@ -61,8 +62,8 @@ class Api {
return this
}

newChild (commandName) {
return new Api({
newChild (commandName, childOptions) {
return new Api(Object.assign({
factories: this._factories,
utils: this.utils,
pathLib: this.pathLib,
Expand All @@ -71,8 +72,9 @@ class Api {
parentName: this.name,
modulesSeen: this._modulesSeen.slice(),
helpOpts: this._assignHelpOpts({}, this.helpOpts),
showHelpByDefault: this._showHelpByDefault
})
showHelpByDefault: this._showHelpByDefault,
strictMode: this._strictMode
}, childOptions))
}

_assignHelpOpts (target, source) {
Expand Down Expand Up @@ -284,6 +286,24 @@ class Api {
return this
}

strict (boolean) {
this._strictMode = boolean !== false
return this
}

addStrictModeErrors (context) {
if (this._strictMode) {
const unknownOptions = context.getUnknownSlurpedOptions()
if (unknownOptions.length > 0) {
context.cliMessage(`Unknown options: ${unknownOptions.map(u => u.raw).join(', ')}`)
}
const unknownArguments = context.getUnknownArguments()
if (unknownArguments.length > 0) {
context.cliMessage(`Unknown arguments: ${unknownArguments.join(' ')}`)
}
}
}

// complex types
commandDirectory (dir, opts) {
if (typeof dir === 'object') {
Expand Down Expand Up @@ -597,6 +617,10 @@ class Api {
if (this._showHelpByDefault && !context.details.args.length) context.deferHelp() // preemptively request help

return this.parseFromContext(context).then(whenDone => {
if (!context.commandHandlerRun && !context.output) {
this.addStrictModeErrors(context)
}

if (context.helpRequested && !context.output) {
context.addDeferredHelp(this.initHelpBuffer())
} else if (context.versionRequested && !context.output) {
Expand Down Expand Up @@ -632,7 +656,7 @@ class Api {
this._magicCommandAdded = true
this._internalCommand(Api.DEFAULT_COMMAND_INDICATOR, (argv, context) => {
context.deferHelp().addDeferredHelp(this.initHelpBuffer())
}).configure({ api: this.newChild(Api.DEFAULT_COMMAND_INDICATOR) }, false)
}).configure({ api: this.newChild(Api.DEFAULT_COMMAND_INDICATOR, { strictMode: false }) }, false)
}

// add known types to context
Expand Down
14 changes: 14 additions & 0 deletions context.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class Context {
this.code = 0
this.output = ''
this.argv = {}
this.knownArgv = {}
this.details = { args: [], types: [] }
this.errors = []
this.messages = []
Expand Down Expand Up @@ -299,10 +300,23 @@ class Context {
if (tr.datatype === 'command') return undefined // do not add command aliases to argv
tr.aliases.forEach(alias => {
this.argv[alias] = tr.value
this.knownArgv[alias] = tr.value
})
})
}

getUnknownArguments () {
if (!Array.isArray(this.argv._)) return []
const endOptions = this.argv._.indexOf('--')
return this.argv._.slice(0, endOptions === -1 ? this.argv._.length : endOptions)
}

getUnknownSlurpedOptions () {
return Object.keys(this.argv).filter(key => !(key in this.knownArgv)).map(key => {
return this.slurped.find(arg => arg.parsed.some(p => p.key === key))
})
}

toResult () {
return {
code: this.code,
Expand Down
118 changes: 118 additions & 0 deletions test/test-parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,124 @@ tap.test('parse > a required string should not allow empty value', t => {
})
})

tap.test('parse > strict mode prevents unknown options and arguments', t => {
const api = Api.get()
.positional('[foo] [bar]')
.string('-n, --name <string>')
.number('-a, --age <number>')
.strict()
return api.parse('--name Fred --aeg 24').then(result => {
// Unknown flags are detected
t.equal(result.code, 1)
t.match(result.output, /Unknown options: --aeg/)
t.equal(result.errors.length, 0)
return api.parse('send 111 222 333')
}).then(result => {
t.equal(result.code, 1)
t.match(result.output, /Unknown arguments: 222 333/)
t.equal(result.errors.length, 0)
})
})

tap.test('parse > strict mode prevents unknown options and arguments in command blocks', t => {
const api = Api.get()
.string('-n, --name <string>')
.number('-a, --age <number>')
.command('send <studentid>')
.strict()
return api.parse('send 111 --name Fred --aeg 24 --hello').then(result => {
// Unknown flags are detected
t.equal(result.code, 1)
t.match(result.output, /Unknown options: --aeg, --hello/)
t.equal(result.errors.length, 0)
return api.parse('send 111 222 333 --hello')
}).then(result => {
t.equal(result.code, 2)
t.match(result.output, /Unknown arguments: 222 333/)
t.match(result.output, /Unknown options: --hello/)
t.equal(result.errors.length, 0)
return api.parse('random')
}).then(result => {
t.equal(result.code, 1)
t.match(result.output, /Unknown arguments: random/)
t.equal(result.errors.length, 0)
})
})

tap.test('parse > strict mode still allows showHelpByDefault to work', t => {
let ranCommand = false
const api = Api.get({ name: 'program' })
.string('-n, --name <string>')
.number('-a, --age <number>')
.command('send <studentid>', argv => {
ranCommand = true
})
.strict()
.showHelpByDefault()
.outputSettings({ maxWidth: 41 })
return api.parse('').then(result => {
t.equal(result.code, 0)
t.equal(result.errors.length, 0)
t.equal(result.output, [
'Usage: program <command> <args> [options]',
'',
'Commands:',
' send <studentid>',
'',
'Options:',
' -n, --name <string> [string]',
' -a, --age <number> [number]'
].join('\n'))
t.equal(ranCommand, false)
return api.parse('send 111 222 --nmae Fred --age 24')
}).then(result => {
t.equal(result.code, 2)
t.equal(result.errors.length, 0)
t.equal(result.output, [
'Usage: program send <studentid> [options]',
'',
'Arguments:',
' <studentid> [required] [string]',
'',
'Options:',
' -n, --name <string> [string]',
' -a, --age <number> [number]',
'',
'Unknown options: --nmae',
'Unknown arguments: 222'
].join('\n'))
t.equal(ranCommand, false)
return api.parse('send 111 --name Fred --age 24')
}).then(result => {
t.equal(result.code, 0)
t.equal(ranCommand, true)
})
})

tap.test('parse > strict mode ignores options/arguments after --', t => {
let commandArguments
const api = Api.get()
.string('-n, --name <string>')
.number('-a, --age <number>')
.command('send <studentid>', argv => {
commandArguments = argv._
})
.strict()
return api.parse('send 111 -n Fred --aeg 24 222 -- 333 --force').then(result => {
t.equal(result.code, 2)
t.match(result.output, /Unknown options: --aeg/)
t.match(result.output, /Unknown arguments: 222/)
t.equal(result.errors.length, 0)
t.equal(commandArguments, undefined)
return api.parse('send 111 -n Fred --age 24 -- 222 333 --force')
}).then(result => {
t.equal(result.code, 0)
t.equal(result.errors.length, 0)
t.equal(result.output, '')
t.strictSame(commandArguments, ['--', '222', '333', '--force'])
})
})

tap.test('parse > coerced types', t => {
const promises = []

Expand Down
1 change: 1 addition & 0 deletions types/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ class TypeCommand extends Type {
// only run innermost command handler
if (context.commandHandlerRun) return this.resolve()
context.commandHandlerRun = true
this.api.addStrictModeErrors(context)
if (context.helpRequested || context.messages.length) {
// console.log('command.js postParse > adding deferred help, implicit:', match.implicit)
if (!context.output) context.addDeferredHelp(this.api.initHelpBuffer())
Expand Down

0 comments on commit de1eefb

Please sign in to comment.