Skip to content

Commit

Permalink
feat: reworking how numbers are parsed (yargs#104)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: strings that fail `Number.isSafeInteger()` are no longer coerced into numbers.
  • Loading branch information
bcoe committed Oct 5, 2017
1 parent d137227 commit fba00eb
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 15 deletions.
39 changes: 25 additions & 14 deletions index.js
Expand Up @@ -270,9 +270,7 @@ function parse (args, opts) {
}
}
} else {
argv._.push(
flags.strings['_'] || !isNumber(arg) ? arg : Number(arg)
)
argv._.push(maybeCoerceNumber('_', arg))
}
}

Expand Down Expand Up @@ -348,10 +346,8 @@ function parse (args, opts) {
function setArg (key, val) {
unsetDefaulted(key)

if (/-/.test(key) && !(flags.aliases[key] && flags.aliases[key].length) && configuration['camel-case-expansion']) {
var c = camelCase(key)
flags.aliases[key] = [c]
newAliases[c] = true
if (/-/.test(key) && configuration['camel-case-expansion']) {
addNewAlias(key, camelCase(key))
}

var value = processValue(key, val)
Expand Down Expand Up @@ -396,17 +392,23 @@ function parse (args, opts) {
}
}

function addNewAlias (key, alias) {
if (!(flags.aliases[key] && flags.aliases[key].length)) {
flags.aliases[key] = [alias]
newAliases[alias] = true
}
if (!(flags.aliases[alias] && flags.aliases[alias].length)) {
addNewAlias(alias, key)
}
}

function processValue (key, val) {
// handle parsing boolean arguments --foo=true --bar false.
if (checkAllAliases(key, flags.bools) || checkAllAliases(key, flags.counts)) {
if (typeof val === 'string') val = val === 'true'
}

var value = val
if (!checkAllAliases(key, flags.strings) && !checkAllAliases(key, flags.coercions)) {
if (isNumber(val)) value = Number(val)
if (!isUndefined(val) && !isNumber(val) && checkAllAliases(key, flags.numbers)) value = NaN
}
var value = maybeCoerceNumber(key, val)

// increment a count given as arg (either no value or value parsed as boolean)
if (checkAllAliases(key, flags.counts) && (isUndefined(value) || typeof value === 'boolean')) {
Expand All @@ -421,6 +423,14 @@ function parse (args, opts) {
return value
}

function maybeCoerceNumber (key, value) {
if (!checkAllAliases(key, flags.strings) && !checkAllAliases(key, flags.coercions)) {
const shouldCoerceNumber = isNumber(value) && configuration['parse-numbers'] && (Number.isSafeInteger(parseInt(value)))
if (shouldCoerceNumber || (!isUndefined(value) && checkAllAliases(key, flags.numbers))) value = Number(value)
}
return value
}

// set args from config.json file, this should be
// applied last so that defaults can be applied.
function setConfig (argv) {
Expand Down Expand Up @@ -602,7 +612,9 @@ function parse (args, opts) {
flags.aliases[key].concat(key).forEach(function (x) {
if (/-/.test(x) && configuration['camel-case-expansion']) {
var c = camelCase(x)
flags.aliases[key].push(c)
if (flags.aliases[key].indexOf(c) === -1) {
flags.aliases[key].push(c)
}
newAliases[c] = true
}
})
Expand Down Expand Up @@ -664,7 +676,6 @@ function parse (args, opts) {
}

function isNumber (x) {
if (!configuration['parse-numbers']) return false
if (typeof x === 'number') return true
if (/^0x[0-9a-f]+$/i.test(x)) return true
return /^[-+]?(?:\d+(?:\.\d*)?|\.\d+)(e[-+]?\d+)?$/.test(x)
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -29,7 +29,7 @@
"chai": "^3.5.0",
"coveralls": "^2.11.12",
"mocha": "^3.0.1",
"nyc": "^10.0.0",
"nyc": "^11.2.1",
"standard": "^10.0.2",
"standard-version": "^4.0.0"
},
Expand Down
24 changes: 24 additions & 0 deletions test/yargs-parser.js
Expand Up @@ -1955,6 +1955,17 @@ describe('yargs-parser', function () {
})
expect(parsed._[0]).to.equal('5')
})

it('parses number if option explicitly set to number type', function () {
var parsed = parser(['--foo', '5', '--bar', '6'], {
number: 'bar',
configuration: {
'parse-numbers': false
}
})
expect(parsed['foo']).to.equal('5')
expect(parsed['bar']).to.equal(6)
})
})

describe('boolean negation', function () {
Expand Down Expand Up @@ -2440,4 +2451,17 @@ describe('yargs-parser', function () {
})
argv.a.should.deep.equal(['a.txt', 'b.txt'])
})

// see: https://github.com/yargs/yargs/issues/963
it('does not magically convert numeric strings larger than Number.MAX_SAFE_INTEGER', () => {
const argv = parser([ '--foo', '93940495950949399948393' ])
argv.foo.should.equal('93940495950949399948393')
})

it('converts numeric options larger than Number.MAX_SAFE_INTEGER to number', () => {
const argv = parser([ '--foo', '93940495950949399948393' ], {
number: ['foo']
})
argv.foo.should.equal(9.39404959509494e+22)
})
})

0 comments on commit fba00eb

Please sign in to comment.