From e1a300f1293ad821c900284616337f080b207980 Mon Sep 17 00:00:00 2001 From: "Benjamin E. Coe" Date: Mon, 21 Sep 2020 16:09:36 -0700 Subject: [PATCH] fix(positional): positional strings no longer drop decimals (#1761) --- lib/command.ts | 2 +- lib/platform-shims/deno.ts | 2 +- lib/typings/yargs-parser-types.ts | 3 ++ lib/yargs-factory.ts | 68 ++++++++++++++++++++++++------- package.json | 2 +- test/deno/yargs.test.ts | 15 ++++++- test/esm/yargs-test.mjs | 13 ++++++ test/yargs.cjs | 34 +++++++++++++++- 8 files changed, 119 insertions(+), 20 deletions(-) diff --git a/lib/command.ts b/lib/command.ts index 2280c232f..008a44a3d 100644 --- a/lib/command.ts +++ b/lib/command.ts @@ -221,7 +221,7 @@ export function command ( // to simplify the parsing of positionals in commands, // we temporarily populate '--' rather than _, with arguments const populateDoubleDash = !!yargs.getOptions().configuration['populate--'] - if (!populateDoubleDash) yargs._copyDoubleDash(innerArgv) + yargs._postProcess(innerArgv, populateDoubleDash) innerArgv = applyMiddleware(innerArgv, yargs, middlewares, false) let handlerResult diff --git a/lib/platform-shims/deno.ts b/lib/platform-shims/deno.ts index 5102b144c..0c3057e32 100644 --- a/lib/platform-shims/deno.ts +++ b/lib/platform-shims/deno.ts @@ -6,7 +6,7 @@ import { basename, dirname, extname, posix } from 'https://deno.land/std/path/mo import cliui from 'https://deno.land/x/cliui@v7.0.0-deno/deno.ts' import escalade from 'https://deno.land/x/escalade@v3.0.3/sync.ts' -import Parser from 'https://deno.land/x/yargs_parser@v19.0.1-deno/deno.ts' +import Parser from 'https://deno.land/x/yargs_parser@v20.2.0-deno/deno.ts' import y18n from 'https://deno.land/x/y18n@v5.0.0-deno/deno.ts' import { YError } from '../../build/lib/yerror.js' diff --git a/lib/typings/yargs-parser-types.ts b/lib/typings/yargs-parser-types.ts index d6d8120bd..d2cc3f1bd 100644 --- a/lib/typings/yargs-parser-types.ts +++ b/lib/typings/yargs-parser-types.ts @@ -51,6 +51,8 @@ export interface Configuration { 'nargs-eats-options': boolean; /** The prefix to use for negated boolean variables. Default is `'no-'` */ 'negation-prefix': string; + /** Should positional values that look like numbers be parsed? Default is `true` */ + 'parse-positional-numbers': boolean; /** Should keys that look like numbers be treated as such? Default is `true` */ 'parse-numbers': boolean; /** Should unparsed flags be stored in -- or _? Default is `false` */ @@ -132,6 +134,7 @@ export interface Parser { detailed(args: ArgsInput, opts?: Partial): DetailedArguments; camelCase(str: string): string; decamelize(str: string, joinString?: string): string; + looksLikeNumber(x: null | undefined | number | string): boolean; } export declare type StringFlag = Dictionary; export declare type BooleanFlag = Dictionary; diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index d87087337..2115390ea 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -6,7 +6,7 @@ import { CommandInstance, CommandHandler, CommandBuilderDefinition, CommandBuilder, CommandHandlerCallback, FinishCommandHandler, command as Command, CommandHandlerDefinition } from './command.js' import { Dictionary, assertNotStrictEqual, KeyOf, DictionaryKeyof, ValueOf, objectKeys, assertSingleKey, RequireDirectoryOptions, PlatformShim, RequireType } from './typings/common-types.js' import { - Arguments as ParserArguments, + ArgsOutput, DetailedArguments as ParserDetailedArguments, Configuration as ParserConfiguration, Options as ParserOptions, @@ -1218,7 +1218,7 @@ function Yargs (processArgs: string | string[] = [], cwd = shim.process.cwd(), p 'populate--': true }) const parsed = shim.Parser.detailed(args, Object.assign({}, options, { - configuration: config + configuration: { 'parse-positional-numbers': false, ...config } })) as DetailedArguments let argv = parsed.argv as Arguments @@ -1235,7 +1235,7 @@ function Yargs (processArgs: string | string[] = [], cwd = shim.process.cwd(), p // are two passes through the parser. If completion // is being performed short-circuit on the first pass. if (shortCircuit) { - return (populateDoubleDash || _calledFromCommand) ? argv : self._copyDoubleDash(argv) + return self._postProcess(argv, populateDoubleDash, _calledFromCommand) } // if there's a handler associated with a @@ -1269,7 +1269,7 @@ function Yargs (processArgs: string | string[] = [], cwd = shim.process.cwd(), p // the deepest command first; we keep track of the position in the // argv._ array that is currently being executed. const innerArgv = command.runCommand(cmd, self, parsed, i + 1) - return populateDoubleDash ? innerArgv : self._copyDoubleDash(innerArgv) + return self._postProcess(innerArgv, populateDoubleDash) } else if (!firstUnknownCommand && cmd !== completionCommand) { firstUnknownCommand = cmd break @@ -1279,7 +1279,7 @@ function Yargs (processArgs: string | string[] = [], cwd = shim.process.cwd(), p // run the default command, if defined if (command.hasDefaultCommand() && !skipDefaultCommand) { const innerArgv = command.runCommand(null, self, parsed) - return populateDoubleDash ? innerArgv : self._copyDoubleDash(innerArgv) + return self._postProcess(innerArgv, populateDoubleDash) } // recommend a command if recommendCommands() has @@ -1297,7 +1297,7 @@ function Yargs (processArgs: string | string[] = [], cwd = shim.process.cwd(), p } } else if (command.hasDefaultCommand() && !skipDefaultCommand) { const innerArgv = command.runCommand(null, self, parsed) - return populateDoubleDash ? innerArgv : self._copyDoubleDash(innerArgv) + return self._postProcess(innerArgv, populateDoubleDash) } // we must run completions first, a user might @@ -1316,7 +1316,7 @@ function Yargs (processArgs: string | string[] = [], cwd = shim.process.cwd(), p self.exit(0) }) - return (populateDoubleDash || _calledFromCommand) ? argv : self._copyDoubleDash(argv) + return self._postProcess(argv, !populateDoubleDash, _calledFromCommand) } // Handle 'help' and 'version' options @@ -1360,18 +1360,33 @@ function Yargs (processArgs: string | string[] = [], cwd = shim.process.cwd(), p else throw err } - return (populateDoubleDash || _calledFromCommand) ? argv : self._copyDoubleDash(argv) + return self._postProcess(argv, populateDoubleDash, _calledFromCommand) + } + + // Applies a couple post processing steps that are easier to perform + // as a final step, rather than + self._postProcess = function (argv: Arguments | Promise, populateDoubleDash: boolean, calledFromCommand = false): any { + if (isPromise(argv)) return argv + if (calledFromCommand) return argv + if (!populateDoubleDash) { + argv = self._copyDoubleDash(argv) + } + const parsePositionalNumbers = self.getParserConfiguration()['parse-positional-numbers'] || self.getParserConfiguration()['parse-positional-numbers'] === undefined + if (parsePositionalNumbers) { + argv = self._parsePositionalNumbers(argv) + } + return argv } // to simplify the parsing of positionals in commands, // we temporarily populate '--' rather than _, with arguments // after the '--' directive. After the parse, we copy these back. - self._copyDoubleDash = function (argv: Arguments | Promise): any { - if (isPromise(argv) || !argv._ || !argv['--']) return argv + self._copyDoubleDash = function (argv: Arguments): any { + if (!argv._ || !argv['--']) return argv argv._.push.apply(argv._, argv['--']) - // TODO(bcoe): refactor command parsing such that this delete is not - // necessary: https://github.com/yargs/yargs/issues/1482 + // We catch an error here, in case someone has called Object.seal() + // on the parsed object, see: https://github.com/babel/babel/pull/10733 try { delete argv['--'] } catch (_err) {} @@ -1379,6 +1394,20 @@ function Yargs (processArgs: string | string[] = [], cwd = shim.process.cwd(), p return argv } + // We wait to coerce numbers for positionals until after the initial parse. + // This allows commands to configure number parsing on a positional by + // positional basis: + self._parsePositionalNumbers = function (argv: Arguments): any { + const args: (string | number)[] = argv['--'] ? argv['--'] : argv._ + + for (let i = 0, arg; (arg = args[i]) !== undefined; i++) { + if (shim.Parser.looksLikeNumber(arg) && Number.isSafeInteger(Math.floor(parseFloat(`${arg}`)))) { + args[i] = Number(arg) + } + } + return argv + } + self._runValidation = function runValidation (argv, aliases, positionalMap, parseErrors, isDefaultCommand = false) { if (parseErrors) throw new YError(parseErrors.message) validation.nonOptionCount(argv) @@ -1425,7 +1454,12 @@ export interface YargsInstance { argv: Arguments customScriptName: boolean parsed: DetailedArguments | false - _copyDoubleDash> (argv: T): T + // The methods below are called after the parse in yargs-parser is complete + // and perform cleanup on the object generated: + _postProcess> (argv: T, populateDoubleDash: boolean, calledFromCommand?: boolean): T + _copyDoubleDash (argv: T): T + _parsePositionalNumbers (argv: T): T + _getLoggerInstance (): LoggerInstance _getParseContext(): Object _hasOutput (): boolean @@ -1740,9 +1774,15 @@ interface ParseCallback { (err: YError | string | undefined | null, argv: Arguments | Promise, output: string): void } -export interface Arguments extends ParserArguments { +export interface Arguments { /** The script name or node command */ $0: string + /** Non-option arguments */ + _: ArgsOutput; + /** Arguments after the end-of-options flag `--` */ + '--'?: ArgsOutput; + /** All remaining options */ + [argName: string]: any; } export interface DetailedArguments extends ParserDetailedArguments { diff --git a/package.json b/package.json index 2a7d8c115..f359d72b2 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,7 @@ "require-directory": "^2.1.1", "string-width": "^4.2.0", "y18n": "^5.0.1", - "yargs-parser": "^20.0.0" + "yargs-parser": "^20.2.0" }, "devDependencies": { "@types/chai": "^4.2.11", diff --git a/test/deno/yargs.test.ts b/test/deno/yargs.test.ts index 6204886cc..dbd5169b8 100644 --- a/test/deno/yargs.test.ts +++ b/test/deno/yargs.test.ts @@ -1,10 +1,11 @@ /* global Deno */ import { + assertEquals, assertMatch } from 'https://deno.land/std/testing/asserts.ts' import yargs from '../../deno.ts' -import { Arguments } from '../../types.ts' +import { Arguments, YargsType } from '../../types.ts' Deno.test('demandCommand(1) throw error if no command provided', () => { let err: Error|null = null @@ -25,3 +26,15 @@ Deno.test('guesses version # based on package.json', () => { }) assertMatch('' + output, /[0-9]+\.[0-9]+\.[0-9]+/) }) + +// Handling of strings that look like numbers, see: +// https://github.com/yargs/yargs/issues/1758 +Deno.test('does not drop .0 if positional is configured as string', async () => { + const argv = await yargs(['cmd', '33.0']) + .command('cmd [str]', 'a command', (yargs: YargsType) => { + return yargs.positional('str', { + type: 'string' + }) + }).parse() as Arguments + assertEquals(argv.str, '33.0') +}) diff --git a/test/esm/yargs-test.mjs b/test/esm/yargs-test.mjs index b22ceb3c5..c4a21635d 100644 --- a/test/esm/yargs-test.mjs +++ b/test/esm/yargs-test.mjs @@ -21,4 +21,17 @@ describe('ESM', () => { assert.match(err.message, /not supported yet for ESM/) }) }) + // Handling of strings that look like numbers, see: + // https://github.com/yargs/yargs/issues/1758 + describe('bug #1758', () => { + it('does not drop .0 if positional is configured as string', () => { + const argv = yargs('cmd 33.0') + .command('cmd [str]', 'a command', (yargs) => { + return yargs.positional('str', { + type: 'string' + }) + }).parse() + assert.strictEqual(argv.str, '33.0') + }) + }) }) diff --git a/test/yargs.cjs b/test/yargs.cjs index d2326cd71..729507700 100644 --- a/test/yargs.cjs +++ b/test/yargs.cjs @@ -1803,14 +1803,14 @@ describe('yargs dsl tests', () => { const argv = yargs('--foo.bar 1 --no-baz 2') .parserConfiguration({ 'unknown-options-as-args': true }) .parse() - argv._.should.deep.eql(['--foo.bar', '1', '--no-baz', '2']) + argv._.should.deep.eql(['--foo.bar', 1, '--no-baz', 2]) const argv2 = yargs('foo --foo.bar --cool 1 --no-baz 2') .command('foo', 'my foo command', (yargs) => { yargs.boolean('cool') }, () => {}) .parserConfiguration({ 'unknown-options-as-args': true }) .parse() - argv2._.should.deep.eql(['foo', '--foo.bar', '1', '--no-baz', '2']) + argv2._.should.deep.eql(['foo', '--foo.bar', 1, '--no-baz', 2]) argv2.cool.should.equal(true) }) }) @@ -2595,4 +2595,34 @@ describe('yargs dsl tests', () => { }).to.throw(/yargs supports a minimum Node.js version of 55/) delete process.env.YARGS_MIN_NODE_VERSION }) + + // Handling of strings that look like numbers, see: + // https://github.com/yargs/yargs/issues/1758 + describe('bug #1758', () => { + it('does not drop .0 if flag is configured as string', () => { + const argv = yargs('cmd --foo 33.0') + .command('cmd [str]', 'a command', (yargs) => { + return yargs.option('foo', { + type: 'string' + }) + }).parse() + argv.foo.should.equal('33.0') + }) + + it('does not drop .0 if positional is configured as string', () => { + const argv = yargs('cmd 33.0') + .command('cmd [str]', 'a command', (yargs) => { + return yargs.positional('str', { + type: 'string' + }) + }).parse() + argv.str.should.equal('33.0') + }) + + it('continues to parse values in _ as numbers, when they look like numbers', () => { + const argv = yargs('hello 33.0').parse() + argv._[0].should.equal('hello') + argv._[1].should.equal(33) + }) + }) })