Repository: yargs/yargs (js)

Issue:
Choices Invalid Value undefined
In version 7 choices started an error if one of the choices was not selected. This is fine but i did have situations in which i didn't want a default and adding undefined to the list of choices didnt make sense either. None of the other argument types are required why is choices.

Thanks.

Pull request description:
fix: 'undefined' default value for choices resulted in validation failing
similar to #899, populating keys with `undefined` resulted in it being impossible to have optional `choices`.

fixes #849 

Patch:
diff --git a/lib/validation.js b/lib/validation.js
index 2f9ff8e..74199d6 100644
--- a/lib/validation.js
+++ b/lib/validation.js
@@ -172,7 +172,8 @@ module.exports = function (yargs, usage, y18n) {
         options.choices.hasOwnProperty(key)) {
         [].concat(argv[key]).forEach(function (value) {
           // TODO case-insensitive configurability
-          if (options.choices[key].indexOf(value) === -1) {
+          if (options.choices[key].indexOf(value) === -1 &&
+              value !== undefined) {
             invalid[key] = (invalid[key] || []).concat(value)
           }
         })
@@ -308,9 +309,10 @@ module.exports = function (yargs, usage, y18n) {
 
   self.conflicting = function (argv) {
     var args = Object.getOwnPropertyNames(argv)
-
     args.forEach(function (arg) {
-      if (conflicting[arg] && args.indexOf(conflicting[arg]) !== -1) {
+      // we default keys to 'undefined' that have been configured, we should not
+      // apply conflicting check unless they are a value other than 'undefined'.
+      if (conflicting[arg] && args.indexOf(conflicting[arg]) !== -1 && argv[arg] !== undefined && argv[conflicting[arg]] !== undefined) {
         usage.fail(__('Arguments %s and %s are mutually exclusive', arg, conflicting[arg]))
       }
     })


Test patch:
diff --git a/test/validation.js b/test/validation.js
index 53c30bb..2d61c3a 100644
--- a/test/validation.js
+++ b/test/validation.js
@@ -158,6 +158,38 @@ describe('validation tests', function () {
         .argv
     })
 
+    it('should not fail if argument with conflict is provided, but not the argument it conflicts with', function () {
+      yargs(['command', '-f', '-c'])
+        .command('command')
+        .option('f', {
+          describe: 'a foo'
+        })
+        .option('b', {
+          describe: 'a bar'
+        })
+        .conflicts('f', 'b')
+        .fail(function (msg) {
+          expect.fail()
+        })
+        .argv
+    })
+
+    it('should not fail if conflicting argument is provided, without argument with conflict', function () {
+      yargs(['command', '-b', '-c'])
+          .command('command')
+          .option('f', {
+            describe: 'a foo'
+          })
+          .option('b', {
+            describe: 'a bar'
+          })
+          .conflicts('f', 'b')
+          .fail(function (msg) {
+            expect.fail()
+          })
+          .argv
+    })
+
     it('allows an object to be provided defining conflicting option pairs', function (done) {
       yargs(['-t', '-s'])
         .conflicts({
@@ -196,6 +228,28 @@ describe('validation tests', function () {
           })
           .argv
     })
+
+    it('should fail if alias of conflicting argument is provided', function (done) {
+      yargs(['-f', '--batman=99'])
+        .conflicts('f', 'b')
+        .alias('b', 'batman')
+        .fail(function (msg) {
+          msg.should.equal('Arguments f and b are mutually exclusive')
+          return done()
+        })
+        .argv
+    })
+
+    it('should fail if alias of argument with conflict is provided', function (done) {
+      yargs(['--foo', '-b'])
+        .conflicts('f', 'b')
+        .alias('foo', 'f')
+        .fail(function (msg) {
+          msg.should.equal('Arguments f and b are mutually exclusive')
+          return done()
+        })
+        .argv
+    })
   })
 
   describe('demand', function () {
@@ -403,6 +457,88 @@ describe('validation tests', function () {
         })
         .argv
     })
+
+    // addresses: https://github.com/yargs/yargs/issues/849
+    it('succeeds when demandOption is true and valid choice is provided', function (done) {
+      yargs('one -a 10 marsupial')
+        .command('one', 'level one', function (yargs) {
+          yargs
+            .options({
+              'a': {
+                demandOption: true,
+                choices: [10, 20]
+              }
+            })
+        }, function (argv) {
+          argv._[0].should.equal('one')
+          argv.a.should.equal(10)
+          return done()
+        })
+        .fail(function (msg) {
+          expect.fail()
+        })
+        .argv
+    })
+
+    // addresses: https://github.com/yargs/yargs/issues/849
+    it('fails when demandOption is true and choice is not provided', function (done) {
+      yargs('one -a 10 marsupial')
+        .command('one', 'level one', function (yargs) {
+          yargs
+            .options({
+              'c': {
+                choices: ['1', '2']
+              }
+            })
+            .demandOption('c')
+        }, function (argv) {
+          expect.fail()
+        })
+        .fail(function (msg) {
+          msg.should.equal('Missing required argument: c')
+          return done()
+        })
+        .argv
+    })
+
+    // addresses: https://github.com/yargs/yargs/issues/849
+    it('succeeds when demandOption is false and no choice is provided', function () {
+      yargs('one')
+        .command('one', 'level one', function (yargs) {
+          yargs
+            .options({
+              'a': {
+                demandOption: false,
+                choices: [10, 20]
+              }
+            })
+        }, function (argv) {
+          argv._[0].should.equal('one')
+        })
+        .fail(function (msg) {
+          expect.fail()
+        })
+        .argv
+    })
+
+    // addresses: https://github.com/yargs/yargs/issues/849
+    it('succeeds when demandOption is not provided and no choice is provided', function () {
+      yargs('one')
+        .command('one', 'level one', function (yargs) {
+          yargs
+            .options({
+              'a': {
+                choices: [10, 20]
+              }
+            })
+        }, function (argv) {
+          argv._[0].should.equal('one')
+        })
+        .fail(function (msg) {
+          expect.fail()
+        })
+        .argv
+    })
   })
 
   describe('config', function () {