fix(positional): positional strings no longer drop decimals (#1761)

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

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'
 

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<Options>): DetailedArguments;
     camelCase(str: string): string;
     decamelize(str: string, joinString?: string): string;
+    looksLikeNumber(x: null | undefined | number | string): boolean;
 }
 export declare type StringFlag = Dictionary<string[]>;
 export declare type BooleanFlag = Dictionary<boolean>;

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,25 +1360,54 @@ 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<Arguments>, 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<Arguments>): 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) {}
 
     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<T extends Arguments | Promise<Arguments>> (argv: T): T
+  // The methods below are called after the parse in yargs-parser is complete
+  // and perform cleanup on the object generated:
+  _postProcess<T extends Arguments | Promise<Arguments>> (argv: T, populateDoubleDash: boolean, calledFromCommand?: boolean): T
+  _copyDoubleDash<T extends Arguments> (argv: T): T
+  _parsePositionalNumbers<T extends Arguments> (argv: T): T
+
   _getLoggerInstance (): LoggerInstance
   _getParseContext(): Object
   _hasOutput (): boolean
@@ -1740,9 +1774,15 @@ interface ParseCallback {
   (err: YError | string | undefined | null, argv: Arguments | Promise<Arguments>, 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 {

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",

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')
+})

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')
+    })
+  })
 })

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)
+    })
+  })
 })