diff --git a/lib/cli/options.js b/lib/cli/options.js index fc0c951a8c..09351957b4 100644 --- a/lib/cli/options.js +++ b/lib/cli/options.js @@ -10,7 +10,12 @@ const fs = require('fs'); const ansi = require('ansi-colors'); const yargsParser = require('yargs-parser'); -const {types, aliases} = require('./run-option-metadata'); +const { + types, + aliases, + isMochaFlag, + expectedTypeForFlag +} = require('./run-option-metadata'); const {ONE_AND_DONE_ARGS} = require('./one-and-dones'); const mocharc = require('../mocharc.json'); const {list} = require('./run-helpers'); @@ -18,7 +23,12 @@ const {loadConfig, findConfig} = require('./config'); const findUp = require('find-up'); const debug = require('debug')('mocha:cli:options'); const {isNodeFlag} = require('./node-flags'); -const {createUnparsableFileError} = require('../errors'); +const { + createUnparsableFileError, + createInvalidArgumentTypeError, + createUnsupportedError +} = require('../errors'); +const {isNumeric} = require('../utils'); /** * The `yargs-parser` namespace @@ -93,6 +103,44 @@ const nargOpts = types.array .concat(types.string, types.number) .reduce((acc, arg) => Object.assign(acc, {[arg]: 1}), {}); +/** + * Throws either "UNSUPPORTED" error or "INVALID_ARG_TYPE" error for numeric positional arguments. + * @param {string[]} allArgs - Stringified args passed to mocha cli + * @param {number} numericArg - Numeric positional arg for which error must be thrown + * @param {Object} parsedResult - Result from `yargs-parser` + * @private + * @ignore + */ +const createErrorForNumericPositionalArg = ( + numericArg, + allArgs, + parsedResult +) => { + // A flag for `numericArg` exists if: + // 1. A mocha flag immediately preceeded the numericArg in `allArgs` array and + // 2. `numericArg` value could not be assigned to this flag by `yargs-parser` because of incompatible datatype. + const flag = allArgs.find((arg, index) => { + const normalizedArg = arg.replace(/^--?/, ''); + return ( + isMochaFlag(arg) && + allArgs[index + 1] === String(numericArg) && + parsedResult[normalizedArg] !== String(numericArg) + ); + }); + + if (flag) { + throw createInvalidArgumentTypeError( + `Mocha flag '${flag}' given invalid option: '${numericArg}'`, + numericArg, + expectedTypeForFlag(flag) + ); + } else { + throw createUnsupportedError( + `Option ${numericArg} is unsupported by the mocha cli` + ); + } +}; + /** * Wrapper around `yargs-parser` which applies our settings * @param {string|string[]} args - Arguments to parse @@ -104,24 +152,20 @@ const nargOpts = types.array const parse = (args = [], defaultValues = {}, ...configObjects) => { // save node-specific args for special handling. // 1. when these args have a "=" they should be considered to have values - // 2. if they don't, they just boolean flags + // 2. if they don't, they are just boolean flags // 3. to avoid explicitly defining the set of them, we tell yargs-parser they // are ALL boolean flags. // 4. we can then reapply the values after yargs-parser is done. - const nodeArgs = (Array.isArray(args) ? args : args.split(' ')).reduce( - (acc, arg) => { - const pair = arg.split('='); - let flag = pair[0]; - if (isNodeFlag(flag, false)) { - flag = flag.replace(/^--?/, ''); - return arg.includes('=') - ? acc.concat([[flag, pair[1]]]) - : acc.concat([[flag, true]]); - } - return acc; - }, - [] - ); + const allArgs = Array.isArray(args) ? args : args.split(' '); + const nodeArgs = allArgs.reduce((acc, arg) => { + const pair = arg.split('='); + let flag = pair[0]; + if (isNodeFlag(flag, false)) { + flag = flag.replace(/^--?/, ''); + return acc.concat([[flag, arg.includes('=') ? pair[1] : true]]); + } + return acc; + }, []); const result = yargsParser.detailed(args, { configuration, @@ -140,6 +184,15 @@ const parse = (args = [], defaultValues = {}, ...configObjects) => { process.exit(1); } + const numericPositionalArg = result.argv._.find(arg => isNumeric(arg)); + if (numericPositionalArg) { + createErrorForNumericPositionalArg( + numericPositionalArg, + allArgs, + result.argv + ); + } + // reapply "=" arg values from above nodeArgs.forEach(([key, value]) => { result.argv[key] = value; diff --git a/lib/cli/run-option-metadata.js b/lib/cli/run-option-metadata.js index 83aa70dd7a..df967097a7 100644 --- a/lib/cli/run-option-metadata.js +++ b/lib/cli/run-option-metadata.js @@ -114,3 +114,24 @@ const ALL_MOCHA_FLAGS = Object.keys(TYPES).reduce((acc, key) => { exports.isMochaFlag = flag => { return ALL_MOCHA_FLAGS.has(flag.replace(/^--?/, '')); }; + +/** + * Returns expected yarg option type for a given mocha flag. + * @param {string} flag - Flag to check (can be with or without leading dashes "--"") + * @returns {string | undefined} - If flag is a valid mocha flag, the expected type of argument for this flag is returned, otherwise undefined is returned. + * @private + */ +exports.expectedTypeForFlag = flag => { + const normalizedName = flag.replace(/^--?/, ''); + + // If flag is an alias, get it's full name. + const aliases = exports.aliases; + const fullFlagName = + Object.keys(aliases).find(flagName => + aliases[flagName].includes(normalizedName) + ) || normalizedName; + + return Object.keys(TYPES).find(flagType => + TYPES[flagType].includes(fullFlagName) + ); +}; diff --git a/lib/utils.js b/lib/utils.js index 31b313a6e0..89b21a32d6 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -689,3 +689,10 @@ exports.breakCircularDeps = inputObj => { return _breakCircularDeps(inputObj); }; + +/** + * Checks if provided input can be parsed as a JavaScript Number. + */ +exports.isNumeric = input => { + return !isNaN(parseFloat(input)); +}; diff --git a/test/node-unit/cli/mocha-flags.spec.js b/test/node-unit/cli/mocha-flags.spec.js new file mode 100644 index 0000000000..29647dea05 --- /dev/null +++ b/test/node-unit/cli/mocha-flags.spec.js @@ -0,0 +1,27 @@ +'use strict'; + +const { + types, + expectedTypeForFlag +} = require('../../../lib/cli/run-option-metadata'); + +describe('mocha-flags', function () { + describe('expectedTypeForFlag()', function () { + Object.entries(types).forEach(([dataType, flags]) => { + flags.forEach(flag => { + it(`returns expected ${flag}'s type as ${dataType}`, function () { + expect(expectedTypeForFlag(flag), 'to equal', dataType); + }); + }); + }); + + it('returns undefined for node flags', function () { + expect(expectedTypeForFlag('--throw-deprecation'), 'to equal', undefined); + expect(expectedTypeForFlag('throw-deprecation'), 'to equal', undefined); + }); + + it('returns undefined for unsupported flags', function () { + expect(expectedTypeForFlag('--foo'), 'to equal', undefined); + }); + }); +}); diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js index 7c846a37ed..f5ce73da15 100644 --- a/test/node-unit/cli/options.spec.js +++ b/test/node-unit/cli/options.spec.js @@ -3,6 +3,7 @@ const sinon = require('sinon'); const rewiremock = require('rewiremock/node'); const {ONE_AND_DONE_ARGS} = require('../../../lib/cli/one-and-dones'); +const {constants} = require('../../../lib/errors'); const modulePath = require.resolve('../../../lib/cli/options'); const mocharcPath = require.resolve('../../../lib/mocharc.json'); @@ -676,5 +677,108 @@ describe('options', function () { ]); }); }); + + describe('"numeric arguments"', function () { + const numericArg = 123; + + const unsupportedError = arg => { + return { + message: `Option ${arg} is unsupported by the mocha cli`, + code: constants.UNSUPPORTED + }; + }; + + const invalidArgError = (flag, arg, expectedType = 'string') => { + return { + message: `Mocha flag '${flag}' given invalid option: '${arg}'`, + code: constants.INVALID_ARG_TYPE, + argument: arg, + actual: 'number', + expected: expectedType + }; + }; + + beforeEach(function () { + readFileSync = sinon.stub(); + findConfig = sinon.stub(); + loadConfig = sinon.stub(); + findupSync = sinon.stub(); + loadOptions = proxyLoadOptions({ + readFileSync, + findConfig, + loadConfig, + findupSync + }); + }); + + it('throws UNSUPPORTED error when numeric option is passed to cli', function () { + expect( + () => loadOptions(`${numericArg}`), + 'to throw', + unsupportedError(numericArg) + ); + }); + + it('throws INVALID_ARG_TYPE error when numeric argument is passed to mocha flag that does not accept numeric value', function () { + const flag = '--delay'; + expect( + () => loadOptions(`${flag} ${numericArg}`), + 'to throw', + invalidArgError(flag, numericArg, 'boolean') + ); + }); + + it('throws INVALID_ARG_TYPE error when incompatible flag does not have preceding "--"', function () { + const flag = 'delay'; + expect( + () => loadOptions(`${flag} ${numericArg}`), + 'to throw', + invalidArgError(flag, numericArg, 'boolean') + ); + }); + + it('shows correct flag in error when multiple mocha flags have numeric values', function () { + const flag = '--delay'; + expect( + () => + loadOptions( + `--timeout ${numericArg} ${flag} ${numericArg} --retries ${numericArg}` + ), + 'to throw', + invalidArgError(flag, numericArg, 'boolean') + ); + }); + + it('throws UNSUPPORTED error when numeric arg is passed to unsupported flag', function () { + const invalidFlag = 'foo'; + expect( + () => loadOptions(`${invalidFlag} ${numericArg}`), + 'to throw', + unsupportedError(numericArg) + ); + }); + + it('does not throw error if numeric value is passed to a compatible mocha flag', function () { + expect(() => loadOptions(`--retries ${numericArg}`), 'not to throw'); + }); + + it('does not throw error if numeric value is passed to a node options', function () { + expect( + () => + loadOptions( + `--secure-heap-min=${numericArg} --conditions=${numericArg}` + ), + 'not to throw' + ); + }); + + it('does not throw error if numeric value is passed to string flag', function () { + expect(() => loadOptions(`--grep ${numericArg}`), 'not to throw'); + }); + + it('does not throw error if numeric value is passed to an array flag', function () { + expect(() => loadOptions(`--spec ${numericArg}`), 'not to throw'); + }); + }); }); }); diff --git a/test/node-unit/utils.spec.js b/test/node-unit/utils.spec.js index 1381244a33..06fe287bf2 100644 --- a/test/node-unit/utils.spec.js +++ b/test/node-unit/utils.spec.js @@ -45,5 +45,25 @@ describe('utils', function () { ); }); }); + describe('isNumeric()', function () { + it('returns true for a number type', function () { + expect(utils.isNumeric(42), 'to equal', true); + }); + it('returns true for a string that can be parsed as a number', function () { + expect(utils.isNumeric('42'), 'to equal', true); + }); + it('returns false for a string that cannot be parsed as a number', function () { + expect(utils.isNumeric('foo'), 'to equal', false); + }); + it('returns false for empty string', function () { + expect(utils.isNumeric(''), 'to equal', false); + }); + it('returns false for empty string with many whitespaces', function () { + expect(utils.isNumeric(' '), 'to equal', false); + }); + it('returns true for stringified zero', function () { + expect(utils.isNumeric('0'), 'to equal', true); + }); + }); }); });