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

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

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 =>

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
 };

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)}`
     );
   }
 

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["_"];

tests_integration/__tests__/__snapshots__/config-invalid.js.snap

@@ -7,15 +7,13 @@ Failed to parse \\"<cwd>/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`] = `

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"
+`;

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

tests_integration/cli/config-precedence/.prettierrc

@@ -0,0 +1 @@
+{"printWidth": 0.5}

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"