From d5e738db25eb4c8747d7f90e617771aaa3213912 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Goetz?= Date: Mon, 28 Jan 2019 12:42:32 +0100 Subject: [PATCH] fix: Fix configuration validation and allow specifying custom renderers (#572) - Improve configuration validation - use `yup` to validate - add more tests Fixes #567 --- package.json | 4 +- src/getConfig.js | 155 ++++++++++++++++------ test/__snapshots__/getConfig.spec.js.snap | 22 +-- test/getConfig.spec.js | 18 +++ yarn.lock | 13 +- 5 files changed, 151 insertions(+), 61 deletions(-) diff --git a/package.json b/package.json index a5e468804..3f42efab9 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,6 @@ "g-status": "^2.0.2", "is-glob": "^4.0.0", "is-windows": "^1.0.2", - "jest-validate": "^23.5.0", "listr": "^0.14.2", "lodash": "^4.17.5", "log-symbols": "^2.2.0", @@ -50,7 +49,8 @@ "please-upgrade-node": "^3.0.2", "staged-git-files": "1.1.2", "string-argv": "^0.0.2", - "stringify-object": "^3.2.2" + "stringify-object": "^3.2.2", + "yup": "^0.26.10" }, "devDependencies": { "babel-core": "^6.26.3", diff --git a/src/getConfig.js b/src/getConfig.js index 0c8d05172..db3d2ba03 100644 --- a/src/getConfig.js +++ b/src/getConfig.js @@ -6,10 +6,12 @@ const chalk = require('chalk') const format = require('stringify-object') const intersection = require('lodash/intersection') const defaultsDeep = require('lodash/defaultsDeep') +const isArray = require('lodash/isArray') +const isFunction = require('lodash/isFunction') const isObject = require('lodash/isObject') -const { validate, logValidationWarning } = require('jest-validate') -const { unknownOptionWarning } = require('jest-validate/build/warnings') +const isString = require('lodash/isString') const isGlob = require('is-glob') +const yup = require('yup') const debug = require('debug')('lint-staged:cfg') @@ -32,6 +34,36 @@ const defaultConfig = { relative: false } +/** + * Configuration schema to validate configuration + */ +const schema = yup.object().shape({ + concurrent: yup.boolean().default(defaultConfig.concurrent), + chunkSize: yup + .number() + .positive() + .default(defaultConfig.chunkSize), + globOptions: yup.object().shape({ + matchBase: yup.boolean().default(defaultConfig.globOptions.matchBase), + dot: yup.boolean().default(defaultConfig.globOptions.dot) + }), + linters: yup.object(), + ignore: yup.array().of(yup.string()), + subTaskConcurrency: yup + .number() + .positive() + .integer() + .default(defaultConfig.subTaskConcurrency), + renderer: yup + .mixed() + .test( + 'renderer', + "Should be 'update', 'verbose' or a function.", + value => value === 'update' || value === 'verbose' || isFunction(value) + ), + relative: yup.boolean().default(defaultConfig.relative) +}) + /** * Check if the config is "simple" i.e. doesn't contains any of full config keys * @@ -46,26 +78,57 @@ function isSimple(config) { ) } +const logDeprecation = (opt, helpMsg) => + console.warn(`● Deprecation Warning: + + Option ${chalk.bold(opt)} was removed. + + ${helpMsg} + + Please remove ${chalk.bold(opt)} from your configuration. + +Please refer to https://github.com/okonet/lint-staged#configuration for more information...`) + +const logUnknown = (opt, helpMsg, value) => + console.warn(`● Validation Warning: + + Unknown option ${chalk.bold(`"${opt}"`)} with value ${chalk.bold( + format(value, { inlineCharacterLimit: Number.POSITIVE_INFINITY }) + )} was found in the config root. + + ${helpMsg} + +Please refer to https://github.com/okonet/lint-staged#configuration for more information...`) + +const formatError = helpMsg => `● Validation Error: + + ${helpMsg} + +Please refer to https://github.com/okonet/lint-staged#configuration for more information...` + +const createError = (opt, helpMsg, value) => + formatError(`Invalid value for '${chalk.bold(opt)}'. + + ${helpMsg}. + + Configured value is: ${chalk.bold( + format(value, { inlineCharacterLimit: Number.POSITIVE_INFINITY }) + )}`) + /** - * Custom jest-validate reporter for unknown options + * Reporter for unknown options * @param config - * @param example * @param option - * @param options * @returns {void} */ -function unknownValidationReporter(config, example, option, options) { +function unknownValidationReporter(config, option) { /** * If the unkonwn property is a glob this is probably * a typical mistake of mixing simple and advanced configs */ if (isGlob(option)) { // prettier-ignore - const message = ` Unknown option ${chalk.bold(`"${option}"`)} with value ${chalk.bold( - format(config[option], { inlineCharacterLimit: Number.POSITIVE_INFINITY }) - )} was found in the config root. - - You are probably trying to mix simple and advanced config formats. Adding + const message = `You are probably trying to mix simple and advanced config formats. Adding ${chalk.bold(`"linters": { "${option}": ${JSON.stringify(config[option])} @@ -73,12 +136,11 @@ function unknownValidationReporter(config, example, option, options) { will fix it and remove this message.` - const { comment } = options - const name = options.title.warning - return logValidationWarning(name, message, comment) + return logUnknown(option, message, config[option]) } - // If it is not glob pattern, use default jest-validate reporter - return unknownOptionWarning(config, example, option, options) + + // If it is not glob pattern, simply notify of unknown value + return logUnknown(option, '', config[option]) } /** @@ -109,12 +171,6 @@ function getConfig(sourceConfig, debugMode) { return config } -const optRmMsg = (opt, helpMsg) => ` Option ${chalk.bold(opt)} was removed. - - ${helpMsg} - - Please remove ${chalk.bold(opt)} from your configuration.` - /** * Runs config validation. Throws error if the config is not valid. * @param config {Object} @@ -122,28 +178,47 @@ const optRmMsg = (opt, helpMsg) => ` Option ${chalk.bold(opt)} was removed. */ function validateConfig(config) { debug('Validating config') - const exampleConfig = { - ...defaultConfig, - linters: { - '*.js': ['eslint --fix', 'git add'], - '*.css': 'stylelint' - } - } const deprecatedConfig = { - gitDir: () => optRmMsg('gitDir', "lint-staged now automatically resolves '.git' directory."), - verbose: () => - optRmMsg('verbose', `Use the command line flag ${chalk.bold('--debug')} instead.`) + gitDir: "lint-staged now automatically resolves '.git' directory.", + verbose: `Use the command line flag ${chalk.bold('--debug')} instead.` } - validate(config, { - exampleConfig, - deprecatedConfig, - unknown: unknownValidationReporter, - recursive: false, - comment: - 'Please refer to https://github.com/okonet/lint-staged#configuration for more information...' - }) + const errors = [] + + try { + schema.validateSync(config, { abortEarly: false, strict: true }) + } catch (error) { + error.errors.forEach(message => errors.push(formatError(message))) + } + + if (isObject(config.linters)) { + Object.keys(config.linters).forEach(key => { + if ( + (!isArray(config.linters[key]) || config.linters[key].some(item => !isString(item))) && + !isString(config.linters[key]) + ) { + errors.push( + createError(`linters[${key}]`, 'Should be a string or an array of strings', key) + ) + } + }) + } + + Object.keys(config) + .filter(key => !defaultConfig.hasOwnProperty(key)) + .forEach(option => { + if (deprecatedConfig.hasOwnProperty(option)) { + logDeprecation(option, deprecatedConfig[option]) + return + } + + unknownValidationReporter(config, option) + }) + + if (errors.length) { + throw new Error(errors.join('\n')) + } return config } diff --git a/test/__snapshots__/getConfig.spec.js.snap b/test/__snapshots__/getConfig.spec.js.snap index 6f3146751..0d6103f31 100644 --- a/test/__snapshots__/getConfig.spec.js.snap +++ b/test/__snapshots__/getConfig.spec.js.snap @@ -56,6 +56,8 @@ Object { exports[`validateConfig should not throw and should print nothing for advanced valid config 1`] = `""`; +exports[`validateConfig should not throw and should print nothing for custom renderer 1`] = `""`; + exports[`validateConfig should not throw and should print nothing for simple valid config 1`] = `""`; exports[`validateConfig should not throw and should print validation warnings for mixed config 1`] = ` @@ -100,15 +102,19 @@ Please refer to https://github.com/okonet/lint-staged#configuration for more inf exports[`validateConfig should throw and should print validation errors for invalid config 1`] = ` "● Validation Error: - Option \\"chunkSize\\" must be of type: - number - but instead received: - string + chunkSize must be a \`number\` type, but the final value was: \`\\"string\\"\`. - Example: - { - \\"chunkSize\\": 9007199254740991 - } +Please refer to https://github.com/okonet/lint-staged#configuration for more information..." +`; + +exports[`validateConfig should throw and should print validation errors for invalid linter config 1`] = ` +"● Validation Error: + + Invalid value for 'linters[*.js]'. + + Should be a string or an array of strings. + + Configured value is: '*.js' Please refer to https://github.com/okonet/lint-staged#configuration for more information..." `; diff --git a/test/getConfig.spec.js b/test/getConfig.spec.js index 2bf4d1085..d106974d0 100644 --- a/test/getConfig.spec.js +++ b/test/getConfig.spec.js @@ -170,6 +170,16 @@ describe('validateConfig', () => { expect(() => validateConfig(getConfig(invalidAdvancedConfig))).toThrowErrorMatchingSnapshot() }) + it('should throw and should print validation errors for invalid linter config', () => { + const invalidAdvancedConfig = { + foo: false, + linters: { + '*.js': 1 + } + } + expect(() => validateConfig(getConfig(invalidAdvancedConfig))).toThrowErrorMatchingSnapshot() + }) + it('should not throw and should print validation warnings for mixed config', () => { const invalidMixedConfig = { concurrent: false, @@ -211,4 +221,12 @@ describe('validateConfig', () => { expect(() => validateConfig(getConfig(validAdvancedConfig))).not.toThrow() expect(console.printHistory()).toMatchSnapshot() }) + + it('should not throw and should print nothing for custom renderer', () => { + const validAdvancedConfig = { + renderer: () => {} + } + expect(() => validateConfig(getConfig(validAdvancedConfig))).not.toThrow() + expect(console.printHistory()).toMatchSnapshot() + }) }) diff --git a/yarn.lock b/yarn.lock index 695afc0bd..a5bac1ec2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1299,16 +1299,7 @@ core-util-is@1.0.2, core-util-is@~1.0.0: resolved "https://registry.yarnpkg.com/core-util-is/-/core-util-is-1.0.2.tgz#b5fd54220aa2bc5ab57aab7140c940754503c1a7" integrity sha1-tf1UIgqivFq1eqtxQMlAdUUDwac= -cosmiconfig@5.0.6: - version "5.0.6" - resolved "https://registry.yarnpkg.com/cosmiconfig/-/cosmiconfig-5.0.6.tgz#dca6cf680a0bd03589aff684700858c81abeeb39" - integrity sha512-6DWfizHriCrFWURP1/qyhsiFvYdlJzbCzmtFWh744+KyWsJo5+kPzUZZaMRSSItoYc0pxFX7gEO7ZC1/gN/7AQ== - dependencies: - is-directory "^0.3.1" - js-yaml "^3.9.0" - parse-json "^4.0.0" - -cosmiconfig@^5.0.6: +cosmiconfig@^5.0.2, cosmiconfig@^5.0.6: version "5.0.7" resolved "https://registry.yarnpkg.com/cosmiconfig/-/cosmiconfig-5.0.7.tgz#39826b292ee0d78eda137dfa3173bd1c21a43b04" integrity sha512-PcLqxTKiDmNT6pSpy4N6KtuPwb53W+2tzNvwOZw0WH9N6O0vLIBq0x8aj8Oj75ere4YcGi48bDFCL+3fRJdlNA== @@ -3362,7 +3353,7 @@ jest-util@^23.4.0: slash "^1.0.0" source-map "^0.6.0" -jest-validate@^23.5.0, jest-validate@^23.6.0: +jest-validate@^23.6.0: version "23.6.0" resolved "https://registry.yarnpkg.com/jest-validate/-/jest-validate-23.6.0.tgz#36761f99d1ed33fcd425b4e4c5595d62b6597474" integrity sha512-OFKapYxe72yz7agrDAWi8v2WL8GIfVqcbKRCLbRG9PAxtzF9b1SEDdTpytNDN12z2fJynoBwpMpvj2R39plI2A==