From 4435ecbc7bb606235a16664bd4549d03c96a932c Mon Sep 17 00:00:00 2001 From: Ika Date: Mon, 25 Sep 2017 14:04:25 -0500 Subject: [PATCH] fix(cli): validate options for every `config-precedence` (#2894) * fix(cli): validate options for every `config-precedence` * refactor: use camelcase * refactor: reduce duplicate code * refactor: rename function * refactor: rename parameter --- package.json | 1 + src/cli-constant.js | 8 +-- src/cli-util.js | 70 ++++++++++++------- src/cli-validator.js | 20 ++++-- src/cli.js | 5 +- .../__snapshots__/config-invalid.js.snap | 10 ++- .../with-config-precedence.js.snap | 15 ++++ .../__tests__/with-config-precedence.js | 27 +++++++ .../cli/config-precedence/.prettierrc | 1 + yarn.lock | 8 +-- 10 files changed, 119 insertions(+), 46 deletions(-) create mode 100644 tests_integration/cli/config-precedence/.prettierrc diff --git a/package.json b/package.json index b60713f7..80995760 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "dependencies": { "babel-code-frame": "7.0.0-alpha.12", "babylon": "7.0.0-beta.23", + "camelcase": "4.1.0", "chalk": "2.1.0", "cosmiconfig": "3.0.1", "dashify": "0.2.2", diff --git a/src/cli-constant.js b/src/cli-constant.js index bcff676c..39778be2 100644 --- a/src/cli-constant.js +++ b/src/cli-constant.js @@ -1,5 +1,7 @@ "use strict"; +const camelCase = require("camelcase"); + const CATEGORY_CONFIG = "Config"; const CATEGORY_EDITOR = "Editor"; const CATEGORY_FORMAT = "Format"; @@ -335,10 +337,6 @@ function dedent(str) { return str.replace(new RegExp(`^ {${spaces}}`, "gm"), "").trim(); } -function kebabToCamel(str) { - return str.replace(/-([a-z])/g, (_, char) => char.toUpperCase()); -} - function normalizeDetailedOptions(rawDetailedOptions) { const names = Object.keys(rawDetailedOptions).sort(); @@ -351,7 +349,7 @@ function normalizeDetailedOptions(rawDetailedOptions) { option.forwardToApi && (typeof option.forwardToApi === "string" ? option.forwardToApi - : kebabToCamel(name)), + : camelCase(name)), choices: option.choices && option.choices.map(choice => diff --git a/src/cli-util.js b/src/cli-util.js index acd3f8cd..e0eba7be 100644 --- a/src/cli-util.js +++ b/src/cli-util.js @@ -1,6 +1,7 @@ "use strict"; const path = require("path"); +const camelCase = require("camelcase"); const dashify = require("dashify"); const minimist = require("minimist"); const getStream = require("get-stream"); @@ -152,12 +153,16 @@ function getOptionsOrDie(argv, filePath) { function getOptionsForFile(argv, filePath) { const options = getOptionsOrDie(argv, filePath); - return applyConfigPrecedence(argv, options); + return applyConfigPrecedence( + argv, + options && normalizeConfig("api", options, constant.detailedOptionMap) + ); } function parseArgsToOptions(argv, overrideDefaults) { return getOptions( - normalizeArgv( + normalizeConfig( + "cli", minimist( argv.__args, Object.assign({ @@ -519,7 +524,7 @@ function getOptionDefaultValue(optionName) { return option.default; } - const optionCamelName = kebabToCamel(optionName); + const optionCamelName = camelCase(optionName); if (optionCamelName in apiDefaultOptions) { return apiDefaultOptions[optionCamelName]; } @@ -527,10 +532,6 @@ function getOptionDefaultValue(optionName) { return undefined; } -function kebabToCamel(str) { - return str.replace(/-([a-z])/g, (_, char) => char.toUpperCase()); -} - function indent(str, spaces) { return str.replace(/^/gm, " ".repeat(spaces)); } @@ -543,29 +544,36 @@ function groupBy(array, getKey) { }, Object.create(null)); } -function normalizeArgv(rawArgv, options) { +/** @param {'api' | 'cli'} type */ +function normalizeConfig(type, rawConfig, options) { + if (type === "api" && rawConfig === null) { + return null; + } + options = options || {}; const consoleWarn = options.warning === false ? () => {} : console.warn; const normalized = {}; - Object.keys(rawArgv).forEach(key => { - const rawValue = rawArgv[key]; + Object.keys(rawConfig).forEach(rawKey => { + const rawValue = rawConfig[rawKey]; - if (key === "_") { - normalized[key] = rawValue; + const key = type === "cli" ? rawKey : dashify(rawKey); + + if (type === "cli" && key === "_") { + normalized[rawKey] = rawValue; return; } - if (key.length === 1) { + if (type === "cli" && key.length === 1) { // do nothing with alias return; } const option = constant.detailedOptionMap[key]; - if (option === undefined) { + if (type === "cli" && option === undefined) { // unknown option return; } @@ -575,12 +583,12 @@ function normalizeArgv(rawArgv, options) { if (option.exception !== undefined) { if (typeof option.exception === "function") { if (option.exception(value)) { - normalized[key] = value; + normalized[rawKey] = value; return; } } else { if (value === option.exception) { - normalized[key] = value; + normalized[rawKey] = value; return; } } @@ -588,31 +596,42 @@ function normalizeArgv(rawArgv, options) { switch (option.type) { case "int": - validator.validateIntOption(value, option); - normalized[key] = Number(value); + validator.validateIntOption(type, value, option); + normalized[rawKey] = Number(value); break; case "choice": - validator.validateChoiceOption(value, option); - normalized[key] = value; + validator.validateChoiceOption(type, value, option); + normalized[rawKey] = value; break; default: - normalized[key] = value; + normalized[rawKey] = value; break; } }); return normalized; + function getOptionName(option) { + return type === "cli" ? `--${option.name}` : camelCase(option.name); + } + + function getRedirectName(option, choice) { + return type === "cli" + ? `--${option.name}=${choice.redirect}` + : `{ ${camelCase(option.name)}: ${JSON.stringify(choice.redirect)} }`; + } + function getValue(rawValue, option) { + const optionName = getOptionName(option); if (rawValue && option.deprecated) { - let warning = `\`--${option.name}\` is deprecated.`; + let warning = `\`${optionName}\` is deprecated.`; if (typeof option.deprecated === "string") { warning += ` ${option.deprecated}`; } consoleWarn(warning); } - const value = option.getter(rawValue, rawArgv); + const value = option.getter(rawValue, rawConfig); if (option.type === "choice") { const choice = option.choices.find(choice => choice.value === rawValue); @@ -621,8 +640,9 @@ function normalizeArgv(rawArgv, options) { rawValue === "" ? "without an argument" : `with value \`${rawValue}\``; + const redirectName = getRedirectName(option, choice); consoleWarn( - `\`--${option.name}\` ${warningDescription} is deprecated. Prettier now treats it as: \`--${option.name}=${choice.redirect}\`.` + `\`${optionName}\` ${warningDescription} is deprecated. Prettier now treats it as: \`${redirectName}\`.` ); return choice.redirect; } @@ -639,5 +659,5 @@ module.exports = { formatFiles, createUsage, createDetailedUsage, - normalizeArgv + normalizeConfig }; diff --git a/src/cli-validator.js b/src/cli-validator.js index 1da7b77c..59bd7ef3 100644 --- a/src/cli-validator.js +++ b/src/cli-validator.js @@ -1,5 +1,7 @@ "use strict"; +const camelCase = require("camelcase"); + function validateArgv(argv) { if (argv["write"] && argv["debug-check"]) { console.error("Cannot use --write and --debug-check together."); @@ -12,18 +14,26 @@ function validateArgv(argv) { } } -function validateIntOption(value, option) { - if (!/^\d+$/.test(value)) { +function getOptionName(type, option) { + return type === "cli" ? `--${option.name}` : camelCase(option.name); +} + +function validateIntOption(type, value, option) { + if (!/^\d+$/.test(value) || (type === "api" && typeof value !== "number")) { + const optionName = getOptionName(type, option); throw new Error( - `Invalid --${option.name} value.\nExpected an integer, but received: ${value}` + `Invalid ${optionName} value.\n` + + `Expected an integer, but received: ${JSON.stringify(value)}` ); } } -function validateChoiceOption(value, option) { +function validateChoiceOption(type, value, option) { if (!option.choices.some(choice => choice.value === value)) { + const optionName = getOptionName(type, option); throw new Error( - `Invalid option for --${option.name}.\nExpected ${getJoinedChoices()}, but received: "${value}"` + `Invalid option for ${optionName}.\n` + + `Expected ${getJoinedChoices()}, but received: ${JSON.stringify(value)}` ); } diff --git a/src/cli.js b/src/cli.js index 7369256d..ed8b8ce1 100644 --- a/src/cli.js +++ b/src/cli.js @@ -8,7 +8,10 @@ const util = require("./cli-util"); const validator = require("./cli-validator"); function run(args) { - const argv = util.normalizeArgv(minimist(args, constant.minimistOptions)); + const argv = util.normalizeConfig( + "cli", + minimist(args, constant.minimistOptions) + ); argv.__args = args; argv.__filePatterns = argv["_"]; diff --git a/tests_integration/__tests__/__snapshots__/config-invalid.js.snap b/tests_integration/__tests__/__snapshots__/config-invalid.js.snap index 6f911812..8d1e6608 100644 --- a/tests_integration/__tests__/__snapshots__/config-invalid.js.snap +++ b/tests_integration/__tests__/__snapshots__/config-invalid.js.snap @@ -7,15 +7,13 @@ Failed to parse \\"/tests_integration/cli/config/invalid/file/.prettierrc\\ `; exports[`throw error with invalid config option (int) 1`] = ` -"Error: Invalid --tab-width value. -Expected an integer, but received: 0.5 -" +"Invalid tabWidth value. +Expected an integer, but received: 0.5" `; exports[`throw error with invalid config option (trailingComma) 1`] = ` -"Error: Invalid option for --trailing-comma. -Expected \\"none\\", \\"es5\\" or \\"all\\", but received: \\"wow\\" -" +"Invalid option for trailingComma. +Expected \\"none\\", \\"es5\\" or \\"all\\", but received: \\"wow\\"" `; exports[`throw error with invalid config precedence option (configPrecedence) 1`] = ` diff --git a/tests_integration/__tests__/__snapshots__/with-config-precedence.js.snap b/tests_integration/__tests__/__snapshots__/with-config-precedence.js.snap index d9e6081d..5836ec46 100644 --- a/tests_integration/__tests__/__snapshots__/with-config-precedence.js.snap +++ b/tests_integration/__tests__/__snapshots__/with-config-precedence.js.snap @@ -145,3 +145,18 @@ function rcYaml() { } " `; + +exports[`CLI validate options with --config-precedence cli-override 1`] = ` +"Invalid printWidth value. +Expected an integer, but received: 0.5" +`; + +exports[`CLI validate options with --config-precedence file-override 1`] = ` +"Invalid printWidth value. +Expected an integer, but received: 0.5" +`; + +exports[`CLI validate options with --config-precedence prefer-file 1`] = ` +"Invalid printWidth value. +Expected an integer, but received: 0.5" +`; diff --git a/tests_integration/__tests__/with-config-precedence.js b/tests_integration/__tests__/with-config-precedence.js index e5dcc306..30e2441a 100644 --- a/tests_integration/__tests__/with-config-precedence.js +++ b/tests_integration/__tests__/with-config-precedence.js @@ -72,3 +72,30 @@ test("CLI overrides gets applied when no config exists with --config-precedence expect(output.stdout).toMatchSnapshot(); expect(output.status).toEqual(0); }); + +test("CLI validate options with --config-precedence cli-override", () => { + const output = runPrettier("cli/config-precedence", [ + "--config-precedence", + "cli-override" + ]); + expect(output.stderr).toMatchSnapshot(); + expect(output.status).not.toEqual(0); +}); + +test("CLI validate options with --config-precedence file-override", () => { + const output = runPrettier("cli/config-precedence", [ + "--config-precedence", + "file-override" + ]); + expect(output.stderr).toMatchSnapshot(); + expect(output.status).not.toEqual(0); +}); + +test("CLI validate options with --config-precedence prefer-file", () => { + const output = runPrettier("cli/config-precedence", [ + "--config-precedence", + "prefer-file" + ]); + expect(output.stderr).toMatchSnapshot(); + expect(output.status).not.toEqual(0); +}); diff --git a/tests_integration/cli/config-precedence/.prettierrc b/tests_integration/cli/config-precedence/.prettierrc new file mode 100644 index 00000000..f821fffb --- /dev/null +++ b/tests_integration/cli/config-precedence/.prettierrc @@ -0,0 +1 @@ +{"printWidth": 0.5} \ No newline at end of file diff --git a/yarn.lock b/yarn.lock index e45420fe..a76f3c74 100644 --- a/yarn.lock +++ b/yarn.lock @@ -864,6 +864,10 @@ callsites@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/callsites/-/callsites-2.0.0.tgz#06eb84f00eea413da86affefacbffb36093b3c50" +camelcase@4.1.0, camelcase@^4.1.0: + version "4.1.0" + resolved "https://registry.yarnpkg.com/camelcase/-/camelcase-4.1.0.tgz#d545635be1e33c542649c69173e5de6acfae34dd" + camelcase@^1.0.2: version "1.2.1" resolved "https://registry.yarnpkg.com/camelcase/-/camelcase-1.2.1.tgz#9bb5304d2e0b56698b2c758b08a3eaa9daa58a39" @@ -872,10 +876,6 @@ camelcase@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/camelcase/-/camelcase-3.0.0.tgz#32fc4b9fcdaf845fcdf7e73bb97cac2261f0ab0a" -camelcase@^4.1.0: - version "4.1.0" - resolved "https://registry.yarnpkg.com/camelcase/-/camelcase-4.1.0.tgz#d545635be1e33c542649c69173e5de6acfae34dd" - caseless@~0.11.0: version "0.11.0" resolved "https://registry.yarnpkg.com/caseless/-/caseless-0.11.0.tgz#715b96ea9841593cc33067923f5ec60ebda4f7d7"