From c7eb8fc351734b234d6e8d533d57a4c7bcb22787 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iiro=20Ja=CC=88ppinen?= Date: Thu, 22 Jul 2021 16:30:57 +0300 Subject: [PATCH] refactor: move config validation logging to validateConfig --- lib/index.js | 142 ++++++++---------- lib/messages.js | 11 +- lib/validateConfig.js | 18 ++- test/__snapshots__/index2.spec.js.snap | 10 -- .../__snapshots__/validateConfig.spec.js.snap | 111 +++++++++----- test/index.spec.js | 31 +--- test/index2.spec.js | 3 +- test/makeCmdTasks.spec.js | 7 +- test/validateOptions.spec.js | 2 +- 9 files changed, 156 insertions(+), 179 deletions(-) delete mode 100644 test/__snapshots__/index2.spec.js.snap diff --git a/lib/index.js b/lib/index.js index 0279bc91f..12ae1a19f 100644 --- a/lib/index.js +++ b/lib/index.js @@ -12,7 +12,6 @@ const { ConfigNotFoundError, GetBackupStashError, GitError, - InvalidOptionsError, } = require('./symbols') const validateConfig = require('./validateConfig') const validateOptions = require('./validateOptions') @@ -83,99 +82,78 @@ const lintStaged = async ( } = {}, logger = console ) => { - try { - await validateOptions({ shell }, logger) + await validateOptions({ shell }, logger) - debugLog('Loading config using `cosmiconfig`') + debugLog('Loading config using `cosmiconfig`') - const resolved = configObject - ? { config: configObject, filepath: '(input)' } - : await loadConfig(configPath) + const resolved = configObject + ? { config: configObject, filepath: '(input)' } + : await loadConfig(configPath) - if (resolved == null) { - throw ConfigNotFoundError - } + if (resolved == null) { + logger.error(`${ConfigNotFoundError.message}.`) + throw ConfigNotFoundError + } - debugLog('Successfully loaded config from `%s`:\n%O', resolved.filepath, resolved.config) + debugLog('Successfully loaded config from `%s`:\n%O', resolved.filepath, resolved.config) - // resolved.config is the parsed configuration object - // resolved.filepath is the path to the config file that was found - const config = validateConfig(resolved.config, logger) + // resolved.config is the parsed configuration object + // resolved.filepath is the path to the config file that was found + const config = validateConfig(resolved.config, logger) - if (debug) { - // Log using logger to be able to test through `consolemock`. - logger.log('Running lint-staged with the following config:') - logger.log(stringifyObject(config, { indent: ' ' })) - } else { - // We might not be in debug mode but `DEBUG=lint-staged*` could have - // been set. - debugLog('lint-staged config:\n%O', config) - } + if (debug) { + // Log using logger to be able to test through `consolemock`. + logger.log('Running lint-staged with the following config:') + logger.log(stringifyObject(config, { indent: ' ' })) + } else { + // We might not be in debug mode but `DEBUG=lint-staged*` could have + // been set. + debugLog('lint-staged config:\n%O', config) + } - // Unset GIT_LITERAL_PATHSPECS to not mess with path interpretation - debugLog('Unset GIT_LITERAL_PATHSPECS (was `%s`)', process.env.GIT_LITERAL_PATHSPECS) - delete process.env.GIT_LITERAL_PATHSPECS - - try { - const ctx = await runAll( - { - allowEmpty, - concurrent, - config, - cwd, - debug, - maxArgLength, - quiet, - relative, - shell, - stash, - verbose, - }, - logger - ) - debugLog('Tasks were executed successfully!') - printTaskOutput(ctx, logger) - return true - } catch (runAllError) { - if (runAllError && runAllError.ctx && runAllError.ctx.errors) { - const { ctx } = runAllError - if (ctx.errors.has(ApplyEmptyCommitError)) { - logger.warn(PREVENTED_EMPTY_COMMIT) - } else if (ctx.errors.has(GitError) && !ctx.errors.has(GetBackupStashError)) { - logger.error(GIT_ERROR) - if (ctx.shouldBackup) { - // No sense to show this if the backup stash itself is missing. - logger.error(RESTORE_STASH_EXAMPLE) - } - } + // Unset GIT_LITERAL_PATHSPECS to not mess with path interpretation + debugLog('Unset GIT_LITERAL_PATHSPECS (was `%s`)', process.env.GIT_LITERAL_PATHSPECS) + delete process.env.GIT_LITERAL_PATHSPECS - printTaskOutput(ctx, logger) - return false + try { + const ctx = await runAll( + { + allowEmpty, + concurrent, + config, + cwd, + debug, + maxArgLength, + quiet, + relative, + shell, + stash, + verbose, + }, + logger + ) + debugLog('Tasks were executed successfully!') + printTaskOutput(ctx, logger) + return true + } catch (runAllError) { + if (runAllError && runAllError.ctx && runAllError.ctx.errors) { + const { ctx } = runAllError + if (ctx.errors.has(ApplyEmptyCommitError)) { + logger.warn(PREVENTED_EMPTY_COMMIT) + } else if (ctx.errors.has(GitError) && !ctx.errors.has(GetBackupStashError)) { + logger.error(GIT_ERROR) + if (ctx.shouldBackup) { + // No sense to show this if the backup stash itself is missing. + logger.error(RESTORE_STASH_EXAMPLE) + } } - // Probably a compilation error in the config js file. Pass it up to the outer error handler for logging. - throw runAllError - } - } catch (lintStagedError) { - /** throw early because `validateOptions` options contains own logging */ - if (lintStagedError === InvalidOptionsError) { - throw InvalidOptionsError - } - - /** @todo move logging to `validateConfig` and remove this try/catch block */ - if (lintStagedError === ConfigNotFoundError) { - logger.error(`${lintStagedError.message}.`) - } else { - // It was probably a parsing error - logger.error(`Could not parse lint-staged config. - -${lintStagedError.message}`) + printTaskOutput(ctx, logger) + return false } - // Print helpful message for all errors - logger.error(`Please make sure you have created it correctly. -See https://github.com/okonet/lint-staged#configuration.`) - throw lintStagedError + // Probably a compilation error in the config js file. Pass it up to the outer error handler for logging. + throw runAllError } } diff --git a/lib/messages.js b/lib/messages.js index 1ec9f48cb..3a4f56d96 100644 --- a/lib/messages.js +++ b/lib/messages.js @@ -7,14 +7,11 @@ const format = require('stringify-object') const configurationError = (opt, helpMsg, value) => `${chalk.redBright(`${error} Validation Error:`)} - Invalid value for '${chalk.bold(opt)}'. - - ${helpMsg}. - - Configured value is: ${chalk.bold( + Invalid value for '${chalk.bold(opt)}': ${chalk.bold( format(value, { inlineCharacterLimit: Number.POSITIVE_INFINITY }) )} -` + + ${helpMsg}` const NOT_GIT_REPO = chalk.redBright(`${error} Current directory is not a git directory!`) @@ -47,7 +44,7 @@ const GIT_ERROR = `\n ${error} ${chalk.red(`lint-staged failed due to a git err const invalidOption = (name, value, message) => `${chalk.redBright(`${error} Validation Error:`)} - Invalid value for option ${chalk.bold(name)}: ${chalk.bold(value)} + Invalid value for option '${chalk.bold(name)}': ${chalk.bold(value)} ${message} diff --git a/lib/validateConfig.js b/lib/validateConfig.js index 5cf36f8f3..0cf0ed22e 100644 --- a/lib/validateConfig.js +++ b/lib/validateConfig.js @@ -87,11 +87,7 @@ const validateConfig = (config, logger) => { const testFn = TEST_DEPRECATED_KEYS.get(pattern) if (testFn(task)) { errors.push( - configurationError( - pattern, - 'Advanced configuration has been deprecated. For more info, please visit: https://github.com/okonet/lint-staged', - task - ) + configurationError(pattern, 'Advanced configuration has been deprecated.', task) ) } @@ -108,7 +104,7 @@ const validateConfig = (config, logger) => { errors.push( configurationError( pattern, - 'Should be a string, a function, or an array of strings and functions', + 'Should be a string, a function, or an array of strings and functions.', task ) ) @@ -127,7 +123,15 @@ const validateConfig = (config, logger) => { }, {}) if (errors.length) { - throw new Error(errors.join('\n')) + const message = errors.join('\n\n') + + logger.error(`Could not parse lint-staged config. + +${message} + +See https://github.com/okonet/lint-staged#configuration.`) + + throw new Error(message) } return validatedConfig diff --git a/test/__snapshots__/index2.spec.js.snap b/test/__snapshots__/index2.spec.js.snap deleted file mode 100644 index bab48e0a8..000000000 --- a/test/__snapshots__/index2.spec.js.snap +++ /dev/null @@ -1,10 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`lintStaged should catch errors from js function config 2`] = ` -" -ERROR Could not parse lint-staged config. - -failed config -ERROR Please make sure you have created it correctly. -See https://github.com/okonet/lint-staged#configuration." -`; diff --git a/test/__snapshots__/validateConfig.spec.js.snap b/test/__snapshots__/validateConfig.spec.js.snap index 321b081ec..049d1783e 100644 --- a/test/__snapshots__/validateConfig.spec.js.snap +++ b/test/__snapshots__/validateConfig.spec.js.snap @@ -9,12 +9,9 @@ exports[`validateConfig should not throw when config contains deprecated key but exports[`validateConfig should throw and should print validation errors for invalid config 1`] = ` "× Validation Error: - Invalid value for 'foo'. + Invalid value for 'foo': false - Should be a string, a function, or an array of strings and functions. - - Configured value is: false -" + Should be a string, a function, or an array of strings and functions." `; exports[`validateConfig should throw and should print validation errors for invalid config 1 1`] = `"Configuration should be an object or a function!"`; @@ -22,71 +19,107 @@ exports[`validateConfig should throw and should print validation errors for inva exports[`validateConfig should throw when detecting deprecated advanced configuration 1`] = ` "× Validation Error: - Invalid value for 'chunkSize'. + Invalid value for 'chunkSize': 10 - Advanced configuration has been deprecated. For more info, please visit: https://github.com/okonet/lint-staged. - - Configured value is: 10 + Advanced configuration has been deprecated. × Validation Error: - Invalid value for 'concurrent'. + Invalid value for 'concurrent': false - Advanced configuration has been deprecated. For more info, please visit: https://github.com/okonet/lint-staged. - - Configured value is: false + Advanced configuration has been deprecated. × Validation Error: - Invalid value for 'globOptions'. + Invalid value for 'globOptions': {matchBase: false} - Advanced configuration has been deprecated. For more info, please visit: https://github.com/okonet/lint-staged. - - Configured value is: {matchBase: false} + Advanced configuration has been deprecated. × Validation Error: - Invalid value for 'ignore'. + Invalid value for 'ignore': ['test.js'] - Advanced configuration has been deprecated. For more info, please visit: https://github.com/okonet/lint-staged. - - Configured value is: ['test.js'] + Advanced configuration has been deprecated. × Validation Error: - Invalid value for 'linters'. + Invalid value for 'linters': {'*.js': ['eslint']} - Advanced configuration has been deprecated. For more info, please visit: https://github.com/okonet/lint-staged. - - Configured value is: {'*.js': ['eslint']} + Advanced configuration has been deprecated. × Validation Error: - Invalid value for 'relative'. + Invalid value for 'relative': true - Advanced configuration has been deprecated. For more info, please visit: https://github.com/okonet/lint-staged. - - Configured value is: true + Advanced configuration has been deprecated. × Validation Error: - Invalid value for 'renderer'. + Invalid value for 'renderer': 'silent' - Advanced configuration has been deprecated. For more info, please visit: https://github.com/okonet/lint-staged. - - Configured value is: 'silent' + Advanced configuration has been deprecated. × Validation Error: - Invalid value for 'subTaskConcurrency'. + Invalid value for 'subTaskConcurrency': 10 - Advanced configuration has been deprecated. For more info, please visit: https://github.com/okonet/lint-staged. - - Configured value is: 10 -" + Advanced configuration has been deprecated." `; -exports[`validateConfig should throw when detecting deprecated advanced configuration 2`] = `""`; +exports[`validateConfig should throw when detecting deprecated advanced configuration 2`] = ` +" +ERROR Could not parse lint-staged config. + +× Validation Error: + + Invalid value for 'chunkSize': 10 + + Advanced configuration has been deprecated. + +× Validation Error: + + Invalid value for 'concurrent': false + + Advanced configuration has been deprecated. + +× Validation Error: + + Invalid value for 'globOptions': {matchBase: false} + + Advanced configuration has been deprecated. + +× Validation Error: + + Invalid value for 'ignore': ['test.js'] + + Advanced configuration has been deprecated. + +× Validation Error: + + Invalid value for 'linters': {'*.js': ['eslint']} + + Advanced configuration has been deprecated. + +× Validation Error: + + Invalid value for 'relative': true + + Advanced configuration has been deprecated. + +× Validation Error: + + Invalid value for 'renderer': 'silent' + + Advanced configuration has been deprecated. + +× Validation Error: + + Invalid value for 'subTaskConcurrency': 10 + + Advanced configuration has been deprecated. + +See https://github.com/okonet/lint-staged#configuration." +`; exports[`validateConfig should warn about \`*.{js}\` and return fixed config 1`] = ` " diff --git a/test/index.spec.js b/test/index.spec.js index f0dc62174..047e528e3 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -70,14 +70,7 @@ describe('lintStaged', () => { `[Error: Configuration should not be empty!]` ) - expect(mockedConsole.printHistory()).toMatchInlineSnapshot(` - " - ERROR Could not parse lint-staged config. - - Configuration should not be empty! - ERROR Please make sure you have created it correctly. - See https://github.com/okonet/lint-staged#configuration." - `) + expect(mockedConsole.printHistory()).toMatchInlineSnapshot(`""`) console = previousConsole }) @@ -132,14 +125,7 @@ describe('lintStaged', () => { `[Error: Configuration should not be empty!]` ) - expect(logger.printHistory()).toMatchInlineSnapshot(` - " - ERROR Could not parse lint-staged config. - - Configuration should not be empty! - ERROR Please make sure you have created it correctly. - See https://github.com/okonet/lint-staged#configuration." - `) + expect(logger.printHistory()).toMatchInlineSnapshot(`""`) }) it('should load config file when specified', async () => { @@ -222,9 +208,7 @@ describe('lintStaged', () => { expect(logger.printHistory()).toMatchInlineSnapshot(` " - ERROR Config could not be found. - ERROR Please make sure you have created it correctly. - See https://github.com/okonet/lint-staged#configuration." + ERROR Config could not be found." `) }) @@ -245,13 +229,6 @@ describe('lintStaged', () => { lintStaged({ configPath: nonExistentConfig, quiet: true }, logger) ).rejects.toThrowError() - expect(logger.printHistory()).toMatchInlineSnapshot(` - - ERROR Could not parse lint-staged config. - - ENOENT: no such file or directory, open 'fake-config-file.yml' - ERROR Please make sure you have created it correctly. - See https://github.com/okonet/lint-staged#configuration. - `) + expect(logger.printHistory()).toMatchInlineSnapshot(`""`) }) }) diff --git a/test/index2.spec.js b/test/index2.spec.js index bdeafcbfe..625623a6b 100644 --- a/test/index2.spec.js +++ b/test/index2.spec.js @@ -77,6 +77,7 @@ describe('lintStaged', () => { await expect(lintStaged({ config }, logger)).rejects.toThrowErrorMatchingInlineSnapshot( `"failed config"` ) - expect(logger.printHistory()).toMatchSnapshot() + + expect(logger.printHistory()).toMatchInlineSnapshot(`""`) }) }) diff --git a/test/makeCmdTasks.spec.js b/test/makeCmdTasks.spec.js index 508b6a811..d215c224c 100644 --- a/test/makeCmdTasks.spec.js +++ b/test/makeCmdTasks.spec.js @@ -111,12 +111,9 @@ describe('makeCmdTasks', () => { .toThrowErrorMatchingInlineSnapshot(` "× Validation Error: - Invalid value for '[Function]'. + Invalid value for '[Function]': null - Function task should return a string or an array of strings. - - Configured value is: null - " + Function task should return a string or an array of strings" `) }) diff --git a/test/validateOptions.spec.js b/test/validateOptions.spec.js index a888e4d8e..e8a8dd8c1 100644 --- a/test/validateOptions.spec.js +++ b/test/validateOptions.spec.js @@ -55,7 +55,7 @@ describe('validateOptions', () => { " ERROR × Validation Error: - Invalid value for option shell: /bin/sh + Invalid value for option 'shell': /bin/sh Promise.reject is not a constructor