Skip to content

Commit

Permalink
fix: Set implicit nargs=1 when type=number requiresArg=true (#1050)
Browse files Browse the repository at this point in the history
  • Loading branch information
evocateur authored and bcoe committed Jan 20, 2018
1 parent 6bad6a9 commit 2b56812
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 52 deletions.
4 changes: 2 additions & 2 deletions lib/validation.js
Expand Up @@ -57,10 +57,10 @@ module.exports = function validation (yargs, usage, y18n) {
// make sure that any args that require an
// value (--foo=bar), have a value.
self.missingArgumentValue = function missingArgumentValue (argv) {
const defaultValues = [true, false, '', undefined]
const options = yargs.getOptions()
if (options.requiresArg.length > 0) {
const missingRequiredArgs = []
const defaultValues = new Set([true, false, ''])

options.requiresArg.forEach((key) => {
// if the argument is not set in argv no need to check
Expand All @@ -71,7 +71,7 @@ module.exports = function validation (yargs, usage, y18n) {
// if a value is explicitly requested,
// flag argument as missing if it does not
// look like foo=bar was entered.
if (~defaultValues.indexOf(value) ||
if (defaultValues.has(value) ||
(Array.isArray(value) && !value.length)) {
missingRequiredArgs.push(key)
}
Expand Down
113 changes: 63 additions & 50 deletions test/validation.js
Expand Up @@ -369,46 +369,6 @@ describe('validation tests', () => {
.argv
})

it('fails when a required argument of type number is missing', (done) => {
yargs('-w')
.option('w', {type: 'number', requiresArg: true})
.fail((msg) => {
msg.should.equal('Missing argument value: w')
return done()
})
.argv
})

it('fails when a required argument of type string is missing', (done) => {
yargs('-w')
.option('w', {type: 'string', requiresArg: true})
.fail((msg) => {
msg.should.equal('Missing argument value: w')
return done()
})
.argv
})

it('fails when a required argument of type boolean is missing', (done) => {
yargs('-w')
.option('w', {type: 'boolean', requiresArg: true})
.fail((msg) => {
msg.should.equal('Missing argument value: w')
return done()
})
.argv
})

it('fails when a required argument of type array is missing', (done) => {
yargs('-w')
.option('w', {type: 'array', requiresArg: true})
.fail((msg) => {
msg.should.equal('Missing argument value: w')
return done()
})
.argv
})

it('fails when required arguments are present, but a command is missing', (done) => {
yargs('-w 10 -m wombat')
.demand(1, ['w', 'm'])
Expand All @@ -419,16 +379,6 @@ describe('validation tests', () => {
.argv
})

// see: https://github.com/yargs/yargs/issues/1041
it('does not fail if required argument is not provided', (done) => {
yargs('')
.option('w', {type: 'array', requiresArg: true})
.parse('', (err, argv, output) => {
expect(err).to.equal(null)
return done()
})
})

it('fails without a message if msg is null', (done) => {
yargs([])
.demand(1, null)
Expand Down Expand Up @@ -475,6 +425,69 @@ describe('validation tests', () => {
})
})

describe('requiresArg', () => {
it('fails when a required argument value of type number is missing', (done) => {
yargs()
.option('w', {type: 'number', requiresArg: true})
.parse('-w', (err, argv, output) => {
expect(err).to.exist
expect(err).to.have.property('message', 'Not enough arguments following: w')
return done()
})
})

it('fails when a required argument value of type string is missing', (done) => {
yargs()
.option('w', {type: 'string', requiresArg: true})
.parse('-w', (err, argv, output) => {
expect(err).to.exist
expect(err).to.have.property('message', 'Missing argument value: w')
return done()
})
})

it('fails when a required argument value of type boolean is missing', (done) => {
yargs()
.option('w', {type: 'boolean', requiresArg: true})
.parse('-w', (err, argv, output) => {
expect(err).to.exist
expect(err).to.have.property('message', 'Missing argument value: w')
return done()
})
})

it('fails when a required argument value of type array is missing', (done) => {
yargs()
.option('w', {type: 'array', requiresArg: true})
.parse('-w', (err, argv, output) => {
expect(err).to.exist
expect(err).to.have.property('message', 'Missing argument value: w')
return done()
})
})

// see: https://github.com/yargs/yargs/issues/1041
it('does not fail if argument with required value is not provided', (done) => {
yargs()
.option('w', {type: 'number', requiresArg: true})
.command('woo')
.parse('', (err, argv, output) => {
expect(err).to.equal(null)
return done()
})
})

it('does not fail if argument with required value is not provided to subcommand', (done) => {
yargs()
.option('w', {type: 'number', requiresArg: true})
.command('woo')
.parse('woo', (err, argv, output) => {
expect(err).to.equal(null)
return done()
})
})
})

describe('choices', () => {
it('fails with one invalid value', (done) => {
yargs(['--state', 'denial'])
Expand Down
12 changes: 12 additions & 0 deletions yargs.js
Expand Up @@ -964,6 +964,18 @@ function Yargs (processArgs, cwd, parentRequire) {

options.__ = y18n.__
options.configuration = pkgUp()['yargs'] || {}

// numbers are defaulted to `undefined`, which makes it impossible to verify requiresArg
// so _unlike_ other types, we will implicitly add them to narg when requiresArg is true
const numberTyped = new Set(options.number)
options.requiresArg
.filter(key => numberTyped.has(key))
.forEach(key => {
// if narg _and_ requiresArg are configured for an option,
// we should probably throw an error somewhere indicating conflicting configuration
options.narg[key] = 1
})

const parsed = Parser.detailed(args, options)
let argv = parsed.argv
if (parseContext) argv = Object.assign({}, argv, parseContext)
Expand Down

0 comments on commit 2b56812

Please sign in to comment.