Feature: Config validation

index.js

@@ -1,2 +1,2 @@
 #!/usr/bin/env node
-require('./src')
+require('./src')()

package.json

@@ -23,7 +23,8 @@
   },
   "pre-commit": "pre-commit",
   "jest": {
-    "testEnvironment": "node"
+    "testEnvironment": "node",
+    "setupFiles": ["./testSetup.js"]
   },
   "greenkeeper": {
     "ignore": [
@@ -55,31 +56,36 @@
   "homepage": "https://github.com/okonet/lint-staged#readme",
   "dependencies": {
     "app-root-path": "^2.0.0",
+    "chalk": "^2.1.0",
     "cosmiconfig": "^1.1.0",
     "execa": "^0.8.0",
+    "is-glob": "^4.0.0",
+    "jest-validate": "^20.0.3",
     "listr": "^0.12.0",
-    "lodash.chunk": "^4.2.0",
+    "lodash": "^4.17.4",
     "minimatch": "^3.0.0",
     "npm-which": "^3.0.1",
     "p-map": "^1.1.1",
-    "staged-git-files": "0.0.4"
+    "staged-git-files": "0.0.4",
+    "stringify-object": "^3.2.0"
   },
   "devDependencies": {
     "babel-jest": "^20.0.0",
     "babel-preset-env": "^1.6.0",
     "commitizen": "^2.9.6",
+    "consolemock": "^0.2.1",
     "cz-conventional-changelog": "^2.0.0",
     "eslint": "^4.5.0",
     "eslint-config-okonet": "^5.0.1",
     "eslint-plugin-node": "^5.1.1",
-    "expect": "^1.20.2",
-    "is-promise": "^2.1.0",
-    "jest": "^20.0.1",
+    "jest": "^20.0.4",
+    "jest-cli": "^20.0.4",
     "jsonlint": "^1.6.2",
     "npm-check": "^5.2.2",
     "pre-commit": "^1.1.3",
     "prettier": "1.5.3",
-    "remove-lockfiles": "^1.1.1"
+    "remove-lockfiles": "^1.1.1",
+    "strip-ansi": "^3.0.1"
   },
   "config": {
     "commitizen": {

src/generateTasks.js

@@ -1,26 +1,23 @@
 'use strict'
 
+const path = require('path')
 const minimatch = require('minimatch')
-
-const readConfigOption = require('./readConfigOption')
+const getConfig = require('./getConfig').getConfig
+const resolveGitDir = require('./resolveGitDir')
 
 module.exports = function generateTasks(config, files) {
-  const linters = config.linters !== undefined ? config.linters : config
-  const resolve = file => files[file]
+  const normalizedConfig = getConfig(config) // Ensure we have a normalized config
+  const linters = normalizedConfig.linters
+  const gitDir = normalizedConfig.gitDir
+  const globOptions = normalizedConfig.globOptions
   return Object.keys(linters).map(pattern => {
     const commands = linters[pattern]
-    const globOptions = readConfigOption(config, 'globOptions', {})
-    const filter = minimatch.filter(
-      pattern,
-      Object.assign(
-        {
-          matchBase: true,
-          dot: true
-        },
-        globOptions
-      )
-    )
-    const fileList = Object.keys(files).filter(filter).map(resolve)
+    const filter = minimatch.filter(pattern, globOptions)
+    const fileList = files
+      // We want to filter before resolving paths
+      .filter(filter)
+      // Return absolute path after the filter is run
+      .map(file => path.resolve(resolveGitDir(gitDir), file))
     return {
       pattern,
       commands,

src/getConfig.js

@@ -0,0 +1,137 @@
+/* eslint no-console: 0 */
+/* eslint no-prototype-builtins: 0 */
+const chalk = require('chalk')
+const format = require('stringify-object')
+const intersection = require('lodash/intersection')
+const defaultsDeep = require('lodash/defaultsDeep')
+const isObject = require('lodash/isObject')
+const validate = require('jest-validate').validate
+const logValidationWarning = require('jest-validate').logValidationWarning
+const unknownOptionWarning = require('jest-validate/build/warnings').unknownOptionWarning
+const isGlob = require('is-glob')
+
+/**
+ * Default config object
+ *
+ * @type {{concurrent: boolean, chunkSize: number, gitDir: string, globOptions: {matchBase: boolean, dot: boolean}, linters: {}, subTaskConcurrency: number, renderer: string, verbose: boolean}}
+ */
+const defaultConfig = {
+  concurrent: true,
+  chunkSize: Number.MAX_SAFE_INTEGER,
+  gitDir: '.',
+  globOptions: {
+    matchBase: true,
+    dot: true
+  },
+  linters: {},
+  subTaskConcurrency: 1,
+  renderer: 'update',
+  verbose: false
+}
+
+/**
+ * Check if the config is "simple" i.e. doesn't contains any of full config keys
+ *
+ * @param config
+ * @returns {boolean}
+ */
+function isSimple(config) {
+  return (
+    isObject(config) &&
+    !config.hasOwnProperty('linters') &&
+    intersection(Object.keys(defaultConfig), Object.keys(config)).length === 0
+  )
+}
+
+/**
+ * Custom jest-validate reporter for unknown options
+ * @param config
+ * @param example
+ * @param option
+ * @param options
+ * @returns {void}
+ */
+function unknownValidationReporter(config, example, option, options) {
+  /**
+   * If the unkonwn property is a glob this is probably
+   * a typical mistake of mixing simple and advanced configs
+   */
+  if (isGlob(option)) {
+    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 
+
+  ${chalk.bold(`"linters": {
+    "${option}": ${JSON.stringify(config[option])}
+  }`)}
+
+  will fix it and remove this message.`
+
+    const comment = options.comment
+    const name = (options.title && options.title.warning) || 'WARNING'
+    return logValidationWarning(name, message, comment)
+  }
+  // If it is not glob pattern, use default jest-validate reporter
+  return unknownOptionWarning(config, example, option, options)
+}
+
+/**
+ * For a given configuration object that we retrive from .lintstagedrc or package.json
+ * construct a full configuration with all options set.
+ *
+ * This is a bit tricky since we support 2 different syntxes: simple and full
+ * For simple config, only the `linters` configuration is provided.
+ *
+ * @param {Object} sourceConfig
+ * @returns {{
+ *  concurrent: boolean, chunkSize: number, gitDir: string, globOptions: {matchBase: boolean, dot: boolean}, linters: {}, subTaskConcurrency: number, renderer: string, verbose: boolean
+ * }}
+ */
+function getConfig(sourceConfig) {
+  const config = defaultsDeep(
+    {}, // Do not mutate sourceConfig!!!
+    isSimple(sourceConfig) ? { linters: sourceConfig } : sourceConfig,
+    defaultConfig
+  )
+
+  // Check if renderer is set in sourceConfig and if not, set accordingly to verbose
+  if (isObject(sourceConfig) && !sourceConfig.hasOwnProperty('renderer')) {
+    config.renderer = config.verbose ? 'verbose' : 'update'
+  }
+
+  return config
+}
+
+/**
+ * Runs config validation. Throws error if the config is not valid.
+ * @param config {Object}
+ * @returns config {Object}
+ */
+function validateConfig(config) {
+  const exampleConfig = Object.assign({}, defaultConfig, {
+    linters: {
+      '*.js': ['eslint --fix', 'git add'],
+      '*.css': 'stylelint'
+    }
+  })
+
+  const validation = validate(config, {
+    exampleConfig,
+    unknown: unknownValidationReporter,
+    comment:
+      'Please refer to https://github.com/okonet/lint-staged#configuration for more information...'
+  })
+
+  if (!validation.isValid) {
+    throw new Error('lint-staged config is invalid... Aborting.')
+  }
+
+  return config
+}
+
+module.exports = {
+  getConfig,
+  validateConfig
+}

src/index.js

@@ -1,104 +1,63 @@
 /* eslint no-console: 0 */
-/* eslint no-process-exit: 0 */
+/* eslint import/no-dynamic-require: 0 */
 
 'use strict'
 
-const path = require('path')
-const sgf = require('staged-git-files')
 const appRoot = require('app-root-path')
-const Listr = require('listr')
 const cosmiconfig = require('cosmiconfig')
+const stringifyObject = require('stringify-object')
+const getConfig = require('./getConfig').getConfig
+const validateConfig = require('./getConfig').validateConfig
+const printErrors = require('./printErrors')
+const runAll = require('./runAll')
 
-const packageJson = require(appRoot.resolve('package.json')); // eslint-disable-line
-const runScript = require('./runScript')
-const generateTasks = require('./generateTasks')
-const readConfigOption = require('./readConfigOption')
+// Find the right package.json at the root of the project
+// TODO: Test if it should be aware of `gitDir`
+const packageJson = require(appRoot.resolve('package.json'))
 
 // Force colors for packages that depend on https://www.npmjs.com/package/supports-color
 // but do this only in TTY mode
 if (process.stdout.isTTY) {
   process.env.FORCE_COLOR = true
 }
 
-cosmiconfig('lint-staged', {
-  rc: '.lintstagedrc',
-  rcExtensions: true
-})
-  .then(result => {
-    // result.config is the parsed configuration object
-    // result.filepath is the path to the config file that was found
-    const config = result.config
-
-    const verbose = config.verbose
-    // Output config in verbose mode
-    if (verbose) console.log(config)
-    const concurrent = readConfigOption(config, 'concurrent', true)
-    const renderer = verbose ? 'verbose' : 'update'
-    const gitDir = config.gitDir ? path.resolve(config.gitDir) : process.cwd()
-    sgf.cwd = gitDir
-
-    sgf('ACM', (err, files) => {
-      if (err) {
-        console.error(err)
-        process.exit(1)
+/**
+ * Root lint-staged function that is called from .bin
+ */
+module.exports = function lintStaged() {
+  cosmiconfig('lint-staged', {
+    rc: '.lintstagedrc',
+    rcExtensions: true
+  })
+    .then(result => {
+      // result.config is the parsed configuration object
+      // result.filepath is the path to the config file that was found
+      const config = validateConfig(getConfig(result.config))
+
+      if (config.verbose) {
+        console.log(`
+Running lint-staged with the following config:
+${stringifyObject(config)}
+`)
       }
 
-      const resolvedFiles = {}
-      files.forEach(file => {
-        const absolute = path.resolve(gitDir, file.filename)
-        const relative = path.relative(gitDir, absolute)
-        resolvedFiles[relative] = absolute
-      })
-
-      const tasks = generateTasks(config, resolvedFiles).map(task => ({
-        title: `Running tasks for ${task.pattern}`,
-        task: () =>
-          new Listr(
-            runScript(task.commands, task.fileList, packageJson, {
-              gitDir,
-              verbose,
-              config
-            }),
-            {
-              // In sub-tasks we don't want to run concurrently
-              // and we want to abort on errors
-              concurrent: false,
-              exitOnError: true
-            }
-          ),
-        skip: () => {
-          if (task.fileList.length === 0) {
-            return `No staged files match ${task.pattern}`
-          }
-          return false
-        }
-      }))
-
-      if (tasks.length) {
-        new Listr(tasks, {
-          concurrent,
-          renderer,
-          exitOnError: !concurrent // Wait for all errors when running concurrently
+      runAll(packageJson, config)
+        .then(() => {
+          // No errors, exiting with 0
+          process.exitCode = 0
+        })
+        .catch(error => {
+          // Errors detected, printing and exiting with non-zero
+          printErrors(error)
+          process.exitCode = 1
         })
-          .run()
-          .catch(error => {
-            if (Array.isArray(error.errors)) {
-              error.errors.forEach(lintError => {
-                console.error(lintError.message)
-              })
-            } else {
-              console.log(error.message)
-            }
-            process.exit(1)
-          })
-      }
     })
-  })
-  .catch(parsingError => {
-    console.error(`Could not parse lint-staged config.
+    .catch(parsingError => {
+      console.error(`Could not parse lint-staged config.
 Make sure you have created it. See https://github.com/okonet/lint-staged#readme.
 
 ${parsingError}
 `)
-    process.exit(1)
-  })
+      process.exitCode = 1
+    })
+}

src/printErrors.js

@@ -0,0 +1,13 @@
+/* eslint no-console: 0 */
+
+'use strict'
+
+module.exports = function printErrors(errorInstance) {
+  if (Array.isArray(errorInstance.errors)) {
+    errorInstance.errors.forEach(lintError => {
+      console.error(lintError.message)
+    })
+  } else {
+    console.error(errorInstance.message)
+  }
+}