diff --git a/package.json b/package.json index 715c9cd134..468ad1702e 100644 --- a/package.json +++ b/package.json @@ -51,6 +51,7 @@ "bunyan": "1.8.5", "camelcase": "4.0.0", "debounce": "1.0.0", + "decamelize": "1.2.0", "es6-error": "4.0.2", "es6-promisify": "5.0.0", "event-to-promise": "0.7.0", @@ -65,9 +66,9 @@ "parse-json": "2.2.0", "regenerator-runtime": "0.10.1", "require-uncached": "1.0.3", - "stream-to-promise": "2.2.0", "sign-addon": "0.2.1", "source-map-support": "0.4.11", + "stream-to-promise": "2.2.0", "tmp": "0.0.30", "update-notifier": "1.0.3", "watchpack": "1.2.0", diff --git a/src/config.js b/src/config.js index eba576809f..f9ea09baef 100644 --- a/src/config.js +++ b/src/config.js @@ -2,6 +2,8 @@ import path from 'path'; import requireUncached from 'require-uncached'; +import camelCase from 'camelcase'; +import decamelize from 'decamelize'; import {createLogger} from './util/logger'; import {UsageError} from './errors'; @@ -12,18 +14,24 @@ type ApplyConfigToArgvParams = {| argv: Object, configObject: Object, defaultValues: Object, + configFileName: string, |}; export function applyConfigToArgv({ argv, configObject, defaultValues = {}, + configFileName, }: ApplyConfigToArgvParams): Object { const newArgv = {...argv}; for (const option in configObject) { // we assume the value was set on the CLI if the default value is // not the same as that on the argv object as there is a very rare chance - // this happening + // of this happening + if (camelCase(option) !== option) { + throw new UsageError(`The config option "${option}" must be ` + + `specified in camel case: "${camelCase(option)}"`); + } const wasValueSetOnCLI = typeof(argv[option]) !== 'undefined' && (argv[option] !== defaultValues[option]); if (wasValueSetOnCLI) { @@ -31,10 +39,9 @@ export function applyConfigToArgv({ `configuration: ${option}=${configObject[option]}`); continue; } - if (!argv.hasOwnProperty(option)) { - log.debug(`Ignoring configuration: ${option}=${configObject[option]} ` + - 'because this is an unknown option'); - continue; + if (!argv.hasOwnProperty(decamelize(option, '-'))) { + throw new UsageError(`The config file at ${configFileName} specified ` + + `an unknown option: "${option}"`); } newArgv[option] = configObject[option]; } diff --git a/tests/unit/test.config.js b/tests/unit/test.config.js index 82761aa33b..053b01a7d4 100644 --- a/tests/unit/test.config.js +++ b/tests/unit/test.config.js @@ -45,6 +45,11 @@ function makeArgv({ }; } +const applyConf = (params) => applyConfigToArgv({ + configFileName: 'some/path/to/config.js', + ...params, +}); + describe('config', () => { describe('applyConfigToArgv', () => { @@ -64,7 +69,7 @@ describe('config', () => { const configObject = { sourceDir: '/configured/source/dir', }; - const newArgv = applyConfigToArgv({argv, configObject, defaultValues}); + const newArgv = applyConf({argv, configObject, defaultValues}); assert.strictEqual(newArgv.sourceDir, cmdLineSrcDir); }); @@ -82,7 +87,7 @@ describe('config', () => { const configObject = { sourceDir: '/configured/source/dir', }; - const newArgv = applyConfigToArgv({argv, configObject, defaultValues}); + const newArgv = applyConf({argv, configObject, defaultValues}); assert.strictEqual(newArgv.sourceDir, configObject.sourceDir); }); @@ -102,7 +107,7 @@ describe('config', () => { const configObject = { sourceDir: '/configured/source/dir', }; - const newArgv = applyConfigToArgv({argv, configObject, defaultValues}); + const newArgv = applyConf({argv, configObject, defaultValues}); assert.strictEqual(newArgv.sourceDir, cmdLineSrcDir); }); @@ -115,12 +120,16 @@ describe('config', () => { demand: false, default: 'default/value/option/definition', }, + 'artifacts-dir': { + type: 'string', + demand: false, + }, }, }); const configObject = { - foo: '/configured/foo', + artifactsDir: '/configured/artifacts/dir', }; - const newArgv = applyConfigToArgv({argv, configObject, defaultValues}); + const newArgv = applyConf({argv, configObject, defaultValues}); assert.strictEqual(newArgv.sourceDir, 'default/value/option/definition'); }); @@ -135,12 +144,16 @@ describe('config', () => { demand: false, default: 'default/value/option/definition', }, + 'artifacts-dir': { + type: 'string', + demand: false, + }, }, }); const configObject = { - foo: '/configured/foo', + artifactsDir: '/configured/artifacts/dir', }; - const newArgv = applyConfigToArgv({argv, configObject, defaultValues}); + const newArgv = applyConf({argv, configObject, defaultValues}); assert.strictEqual(newArgv.sourceDir, cmdLineSrcDir); }); @@ -157,7 +170,7 @@ describe('config', () => { const configObject = { overwriteFiles: true, }; - const newArgv = applyConfigToArgv({argv, configObject, defaultValues}); + const newArgv = applyConf({argv, configObject, defaultValues}); assert.strictEqual(newArgv.overwriteFiles, true); }); @@ -173,7 +186,7 @@ describe('config', () => { const configObject = { overwriteFiles: true, }; - const newArgv = applyConfigToArgv({argv, configObject, defaultValues}); + const newArgv = applyConf({argv, configObject, defaultValues}); assert.strictEqual(newArgv.overwriteFiles, true); }); @@ -189,7 +202,7 @@ describe('config', () => { const configObject = { overwriteFiles: false, }; - const newArgv = applyConfigToArgv({argv, configObject, defaultValues}); + const newArgv = applyConf({argv, configObject, defaultValues}); assert.strictEqual(newArgv.overwriteFiles, true); }); @@ -201,12 +214,15 @@ describe('config', () => { 'source-dir': { type: 'string', }, + 'verbose': { + type: 'boolean', + }, }, }); const configObject = { - foo: true, + verbose: true, }; - const newArgv = applyConfigToArgv({argv, configObject, defaultValues}); + const newArgv = applyConf({argv, configObject, defaultValues}); assert.strictEqual(newArgv.sourceDir, cmdLineSrcDir); }); @@ -223,7 +239,7 @@ describe('config', () => { const configObject = { numberOfRetries: 1, }; - const newArgv = applyConfigToArgv({argv, configObject, defaultValues}); + const newArgv = applyConf({argv, configObject, defaultValues}); assert.strictEqual(newArgv.numberOfRetries, 1); }); @@ -240,7 +256,7 @@ describe('config', () => { const configObject = { numberOfRetries: 1, }; - const newArgv = applyConfigToArgv({argv, configObject, defaultValues}); + const newArgv = applyConf({argv, configObject, defaultValues}); assert.strictEqual(newArgv.numberOfRetries, 0); }); @@ -256,9 +272,46 @@ describe('config', () => { const configObject = { sourceDir: '/configured/directory', }; - const newArgv = applyConfigToArgv({argv, configObject, defaultValues}); + const newArgv = applyConf({argv, configObject, defaultValues}); assert.strictEqual(newArgv.sourceDir, '/configured/directory'); }); + + it('throws an error when an option is not camel cased', () => { + const {argv, defaultValues} = makeArgv({ + globalOpt: { + 'source-dir': { + type: 'string', + demand: false, + }, + }, + }); + const configObject = { + 'source-dir': 'fake/value/', + }; + assert.throws(() => { + applyConf({argv, configObject, defaultValues}); + }, UsageError, 'UsageError: The config option "source-dir" must be ' + + 'specified in camel case: "sourceDir"'); + }); + + it('throws an error when an option is invalid', () => { + const {argv, defaultValues} = makeArgv({ + globalOpt: { + 'source-dir': { + type: 'string', + demand: false, + }, + }, + }); + const configFileName = 'fake/path/to/config'; + const configObject = { + artifactsDir: 'fake/artifacts/dir', + }; + assert.throws(() => { + applyConf({argv, configObject, defaultValues, configFileName}); + }, UsageError, 'UsageError: The config file at fake/path/to/config ' + + 'specified an unknown option: "artifactsDir"'); + }); }); describe('loadJSConfigFile', () => {