From 63b17242fca1e891a53574f59349f9e8cf61176f Mon Sep 17 00:00:00 2001 From: Mael Le Guen Date: Wed, 15 Apr 2020 11:20:41 +0200 Subject: [PATCH 1/4] WIP: add test to reproduce #1625 --- test/validation.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/validation.js b/test/validation.js index f562a93a3..eea94d532 100644 --- a/test/validation.js +++ b/test/validation.js @@ -347,6 +347,18 @@ describe('validation tests', () => { expect.fail('no parsing failure') }) + it('fails in strict mode with extra positionals for default command', (done) => { + yargs(['jumping', 'fast']) + .command('$0 ', 'kangaroo handlers') + .strict() + .fail((msg) => { + msg.should.equal('Unknown argument: fast') + return done() + }) + .parse() + expect.fail('no parsing failure') + }) + it('does not fail in strict mode when no commands configured', () => { const argv = yargs('koala') .demand(1) From 68cfe3d8eaa925b9470d7cda40d1abd01a21ca9d Mon Sep 17 00:00:00 2001 From: Mael Le Guen Date: Wed, 15 Apr 2020 11:23:10 +0200 Subject: [PATCH 2/4] fix(strict mode): report default command unknown arguments --- lib/command.js | 2 +- lib/validation.ts | 4 ++-- yargs.js | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/command.js b/lib/command.js index 00bd18666..fe9236b06 100644 --- a/lib/command.js +++ b/lib/command.js @@ -197,7 +197,7 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) { // we apply validation post-hoc, so that custom // checks get passed populated positional arguments. - if (!yargs._hasOutput()) yargs._runValidation(innerArgv, aliases, positionalMap, yargs.parsed.error) + if (!yargs._hasOutput()) yargs._runValidation(innerArgv, aliases, positionalMap, yargs.parsed.error, !command) if (commandHandler.handler && !yargs._hasOutput()) { yargs._setHasOutput() diff --git a/lib/validation.ts b/lib/validation.ts index 8277e4374..8e2516c13 100644 --- a/lib/validation.ts +++ b/lib/validation.ts @@ -114,7 +114,7 @@ export function validation (yargs: YargsInstance, usage: UsageInstance, y18n: Y1 } // check for unknown arguments (strict-mode). - self.unknownArguments = function unknownArguments (argv, aliases, positionalMap) { + self.unknownArguments = function unknownArguments (argv, aliases, positionalMap, isDefaultCommand) { const commandKeys = yargs.getCommandInstance().getCommands() const unknown: string[] = [] const currentContext = yargs.getContext() @@ -129,7 +129,7 @@ export function validation (yargs: YargsInstance, usage: UsageInstance, y18n: Y1 } }) - if ((currentContext.commands.length > 0) || (commandKeys.length > 0)) { + if ((currentContext.commands.length > 0) || (commandKeys.length > 0) || isDefaultCommand) { argv._.slice(currentContext.commands.length).forEach((key) => { if (commandKeys.indexOf(key) === -1) { unknown.push(key) diff --git a/yargs.js b/yargs.js index 81ab52ed9..9f640721a 100644 --- a/yargs.js +++ b/yargs.js @@ -1261,7 +1261,7 @@ function Yargs (processArgs, cwd, parentRequire) { return argv } - self._runValidation = function runValidation (argv, aliases, positionalMap, parseErrors) { + self._runValidation = function runValidation (argv, aliases, positionalMap, parseErrors, isDefaultCommand = false) { if (parseErrors) throw new YError(parseErrors.message) validation.nonOptionCount(argv) validation.requiredArguments(argv) @@ -1270,7 +1270,7 @@ function Yargs (processArgs, cwd, parentRequire) { failedStrictCommands = validation.unknownCommands(argv) } if (strict && !failedStrictCommands) { - validation.unknownArguments(argv, aliases, positionalMap) + validation.unknownArguments(argv, aliases, positionalMap, isDefaultCommand) } validation.customChecks(argv, aliases) validation.limitedChoices(argv) From d9627babf522135392e802923f607020c2521f9a Mon Sep 17 00:00:00 2001 From: Mael Le Guen Date: Wed, 15 Apr 2020 11:25:11 +0200 Subject: [PATCH 3/4] fix(tests): add missing expect.fail on some tests expecting to fail For them to fail cleanly if parsing did not fail instead of timing out. --- test/validation.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/validation.js b/test/validation.js index eea94d532..8d438eaa2 100644 --- a/test/validation.js +++ b/test/validation.js @@ -320,6 +320,7 @@ describe('validation tests', () => { return done() }) .parse() + expect.fail('no parsing failure') }) it('fails in strict mode with invalid command', (done) => { @@ -333,6 +334,7 @@ describe('validation tests', () => { return done() }) .parse() + expect.fail('no parsing failure') }) it('fails in strict mode with extra positionals', (done) => { From 17b411be66227c2c39c96e7641f391af92b35bcf Mon Sep 17 00:00:00 2001 From: Mael Le Guen Date: Fri, 17 Apr 2020 09:57:51 +0200 Subject: [PATCH 4/4] fix(validation): add isDefaultCommand to ValidationInstance.unknownArguments --- lib/validation.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/validation.ts b/lib/validation.ts index 8e2516c13..731e18caa 100644 --- a/lib/validation.ts +++ b/lib/validation.ts @@ -405,7 +405,7 @@ export function validation (yargs: YargsInstance, usage: UsageInstance, y18n: Y1 } /** Instance of the validation module. */ -interface ValidationInstance { +export interface ValidationInstance { check(f: CustomCheck['func'], global: boolean): void conflicting(argv: Arguments): void conflicts(key: string, value: string | string[]): void @@ -425,7 +425,12 @@ interface ValidationInstance { requiredArguments(argv: Arguments): void reset(localLookup: Dictionary): ValidationInstance unfreeze(): void - unknownArguments(argv: Arguments, aliases: DetailedArguments['aliases'], positionalMap: Dictionary): void + unknownArguments( + argv: Arguments, + aliases: DetailedArguments['aliases'], + positionalMap: Dictionary, + isDefaultCommand: boolean + ): void unknownCommands(argv: Arguments): boolean }