From c804f0db78e56b44341cc7a91878c27b1b68b9f2 Mon Sep 17 00:00:00 2001
From: Landon Yarrington <33426811+jly36963@users.noreply.github.com>
Date: Sun, 11 Jul 2021 10:01:29 -0600
Subject: [PATCH] fix: strict should fail unknown arguments (#1977)

---
 lib/validation.ts   | 28 ++++++++++++++++++++++++----
 test/validation.cjs | 37 +++++++++++++++++++++++++++++++++++--
 2 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/lib/validation.ts b/lib/validation.ts
index 4a4e89f8f..c64fe9380 100644
--- a/lib/validation.ts
+++ b/lib/validation.ts
@@ -154,7 +154,7 @@ export function validation(
 
     Object.keys(argv).forEach(key => {
       if (
-        specialKeys.indexOf(key) === -1 &&
+        !specialKeys.includes(key) &&
         !Object.prototype.hasOwnProperty.call(positionalMap, key) &&
         !Object.prototype.hasOwnProperty.call(
           yargs.getInternalMethods().getParseContext(),
@@ -173,13 +173,33 @@ export function validation(
         isDefaultCommand)
     ) {
       argv._.slice(currentContext.commands.length).forEach(key => {
-        if (commandKeys.indexOf('' + key) === -1) {
+        if (!commandKeys.includes('' + key)) {
           unknown.push('' + key);
         }
       });
     }
 
-    if (unknown.length > 0) {
+    // https://github.com/yargs/yargs/issues/1861
+    if (checkPositionals) {
+      // Check for non-option args that are not in currentContext.commands
+      // Take into account expected args from commands and yargs.demand(number)
+      const demandedCommands = yargs.getDemandedCommands();
+      const maxNonOptDemanded = demandedCommands._?.max || 0;
+      const expected = currentContext.commands.length + maxNonOptDemanded;
+      if (expected < argv._.length) {
+        argv._.slice(expected).forEach(key => {
+          key = String(key);
+          if (
+            !currentContext.commands.includes(key) &&
+            !unknown.includes(key)
+          ) {
+            unknown.push(key);
+          }
+        });
+      }
+    }
+
+    if (unknown.length) {
       usage.fail(
         __n(
           'Unknown argument: %s',
@@ -201,7 +221,7 @@ export function validation(
 
     if (currentContext.commands.length > 0 || commandKeys.length > 0) {
       argv._.slice(currentContext.commands.length).forEach(key => {
-        if (commandKeys.indexOf('' + key) === -1) {
+        if (!commandKeys.includes('' + key)) {
           unknown.push('' + key);
         }
       });
diff --git a/test/validation.cjs b/test/validation.cjs
index def19aa7d..ab5bb5bdb 100644
--- a/test/validation.cjs
+++ b/test/validation.cjs
@@ -330,6 +330,29 @@ describe('validation tests', () => {
       expect.fail('no parsing failure');
     });
 
+    // addresses: https://github.com/yargs/yargs/issues/1861
+    it('fails in strict mode when no commands defined but command is passed', done => {
+      yargs
+        .strict()
+        .fail(msg => {
+          msg.should.equal('Unknown argument: foo');
+          done();
+        })
+        .parse('foo');
+      expect.fail('no parsing failure');
+    });
+
+    it('fails because of undefined command and not because of argument after --', done => {
+      yargs
+        .strict()
+        .fail(msg => {
+          msg.should.equal('Unknown argument: foo');
+          done();
+        })
+        .parse('foo -- hello');
+      expect.fail('no parsing failure');
+    });
+
     it('fails in strict mode with invalid command', done => {
       yargs(['koala'])
         .command('wombat', 'wombat burrows')
@@ -916,7 +939,7 @@ describe('validation tests', () => {
     });
 
     it('does not fail for hidden options', () => {
-      const args = yargs('--foo hey')
+      const args = yargs('--foo')
         .strict()
         .option('foo', {boolean: true, describe: false})
         .fail(msg => {
@@ -926,8 +949,18 @@ describe('validation tests', () => {
       args.foo.should.equal(true);
     });
 
+    it('does not fail for hidden options but does for unknown arguments', () => {
+      const args = yargs('--foo hey')
+        .strict()
+        .option('foo', {boolean: true, describe: false})
+        .fail(msg => {
+          msg.should.equal('Unknown argument: hey');
+        })
+        .parse();
+    });
+
     it('does not fail if an alias is provided, rather than option itself', () => {
-      const args = yargs('--cat hey')
+      const args = yargs('--cat')
         .strict()
         .option('foo', {boolean: true, describe: false})
         .alias('foo', 'bar')