Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix: Fix configuration validation and allow specifying custom rendere…
…rs (#572)

- Improve configuration validation
- use `yup` to validate
- add more tests

Fixes #567
  • Loading branch information
onigoetz authored and okonet committed Jan 28, 2019
1 parent 406a0c0 commit d5e738d
Show file tree
Hide file tree
Showing 5 changed files with 151 additions and 61 deletions.
4 changes: 2 additions & 2 deletions package.json
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
155 changes: 115 additions & 40 deletions src/getConfig.js
Expand Up @@ -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')

Expand All @@ -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
*
Expand All @@ -46,39 +78,69 @@ 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])}
}`)}
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])
}

/**
Expand Down Expand Up @@ -109,41 +171,54 @@ 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}
* @returns config {Object}
*/
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
}
Expand Down
22 changes: 14 additions & 8 deletions test/__snapshots__/getConfig.spec.js.snap
Expand Up @@ -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`] = `
Expand Down Expand Up @@ -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..."
`;
18 changes: 18 additions & 0 deletions test/getConfig.spec.js
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
})
})
13 changes: 2 additions & 11 deletions yarn.lock
Expand Up @@ -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==
Expand Down Expand Up @@ -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==
Expand Down

0 comments on commit d5e738d

Please sign in to comment.