refactor: use symbols for errors

lib/gitWorkflow.js

@@ -6,6 +6,14 @@ const path = require('path')
 const chunkFiles = require('./chunkFiles')
 const execGit = require('./execGit')
 const { readFile, unlink, writeFile } = require('./file')
+const {
+  GitError,
+  RestoreOriginalStateError,
+  ApplyEmptyCommitError,
+  GetBackupStashError,
+  HideUnstagedChangesError,
+  RestoreUnstagedChangesError
+} = require('./symbols')
 
 const MERGE_HEAD = 'MERGE_HEAD'
 const MERGE_MODE = 'MERGE_MODE'
@@ -47,8 +55,9 @@ const GIT_DIFF_ARGS = [
 ]
 const GIT_APPLY_ARGS = ['-v', '--whitespace=nowarn', '--recount', '--unidiff-zero']
 
-const handleError = (error, ctx) => {
-  ctx.gitError = true
+const handleError = (error, ctx, symbol) => {
+  ctx.errors.add(GitError)
+  if (symbol) ctx.errors.add(symbol)
   throw error
 }
 
@@ -87,7 +96,7 @@ class GitWorkflow {
     const stashes = await this.execGit(['stash', 'list'])
     const index = stashes.split('\n').findIndex((line) => line.includes(STASH))
     if (index === -1) {
-      ctx.gitGetBackupStashError = true
+      ctx.errors.add(GetBackupStashError)
       throw new Error('lint-staged automatic backup is missing!')
     }
     return `stash@{${index}}`
@@ -219,8 +228,7 @@ class GitWorkflow {
        * `git checkout --force` doesn't throw errors, so it shouldn't be possible to get here.
        * If this does fail, the handleError method will set ctx.gitError and lint-staged will fail.
        */
-      ctx.gitHideUnstagedChangesError = true
-      handleError(error, ctx)
+      handleError(error, ctx, HideUnstagedChangesError)
     }
   }
 
@@ -252,8 +260,7 @@ class GitWorkflow {
     if (!stagedFilesAfterAdd && !this.allowEmpty) {
       // Tasks reverted all staged changes and the commit would be empty
       // Throw error to stop commit unless `--allow-empty` was used
-      ctx.gitApplyEmptyCommitError = true
-      handleError(new Error('Prevented an empty git commit!'), ctx)
+      handleError(new Error('Prevented an empty git commit!'), ctx, ApplyEmptyCommitError)
     }
   }
 
@@ -277,10 +284,10 @@ class GitWorkflow {
       } catch (threeWayApplyError) {
         debug('Error while restoring unstaged changes using 3-way merge:')
         debug(threeWayApplyError)
-        ctx.gitRestoreUnstagedChangesError = true
         handleError(
           new Error('Unstaged changes could not be restored due to a merge conflict!'),
-          ctx
+          ctx,
+          RestoreUnstagedChangesError
         )
       }
     }
@@ -306,8 +313,7 @@ class GitWorkflow {
 
       debug('Done restoring original state!')
     } catch (error) {
-      ctx.gitRestoreOriginalStateError = true
-      handleError(error, ctx)
+      handleError(error, ctx, RestoreOriginalStateError)
     }
   }
 

lib/resolveTaskFn.js

@@ -1,12 +1,13 @@
 'use strict'
 
 const chalk = require('chalk')
-const { parseArgsStringToArgv } = require('string-argv')
 const dedent = require('dedent')
 const execa = require('execa')
+const debug = require('debug')('lint-staged:task')
 const symbols = require('log-symbols')
+const { parseArgsStringToArgv } = require('string-argv')
 
-const debug = require('debug')('lint-staged:task')
+const { TaskError } = require('./symbols')
 
 const successMsg = (linter) => `${symbols.success} ${linter} passed!`
 
@@ -20,11 +21,11 @@ const successMsg = (linter) => `${symbols.success} ${linter} passed!`
  * @param {boolean} result.failed
  * @param {boolean} result.killed
  * @param {string} result.signal
- * @param {Object} context (see https://github.com/SamVerschueren/listr#context)
+ * @param {Object} ctx (see https://github.com/SamVerschueren/listr#context)
  * @returns {Error}
  */
-function makeErr(linter, result, context = {}) {
-  context.taskError = true
+const makeErr = (linter, result, ctx) => {
+  ctx.errors.add(TaskError)
   const { stdout, stderr, killed, signal } = result
 
   if (killed || (signal && signal !== '')) {

lib/runAll.js

@@ -13,6 +13,14 @@ const getStagedFiles = require('./getStagedFiles')
 const GitWorkflow = require('./gitWorkflow')
 const makeCmdTasks = require('./makeCmdTasks')
 const resolveGitRepo = require('./resolveGitRepo')
+const {
+  ApplyEmptyCommitError,
+  TaskError,
+  RestoreOriginalStateError,
+  GetBackupStashError,
+  GitError,
+  RestoreUnstagedChangesError
+} = require('./symbols')
 
 const debugLog = require('debug')('lint-staged:run')
 
@@ -31,40 +39,41 @@ const MESSAGES = {
 
 const shouldSkipApplyModifications = (ctx) => {
   // Should be skipped in case of git errors
-  if (ctx.gitError) {
+  if (ctx.errors.has(GitError)) {
     return MESSAGES.GIT_ERROR
   }
   // Should be skipped when tasks fail
-  if (ctx.taskError) {
+  if (ctx.errors.has(TaskError)) {
     return MESSAGES.TASK_ERROR
   }
 }
 
 const shouldSkipRevert = (ctx) => {
   // Should be skipped in case of unknown git errors
-  if (ctx.gitError && !ctx.gitApplyEmptyCommitError && !ctx.gitRestoreUnstagedChangesError) {
+  if (
+    ctx.errors.has(GitError) &&
+    !ctx.errors.has(ApplyEmptyCommitError) &&
+    !ctx.errors.has(RestoreUnstagedChangesError)
+  ) {
     return MESSAGES.GIT_ERROR
   }
 }
 
 const shouldSkipCleanup = (ctx) => {
   // Should be skipped in case of unknown git errors
-  if (ctx.gitError && !ctx.gitApplyEmptyCommitError && !ctx.gitRestoreUnstagedChangesError) {
+  if (
+    ctx.errors.has(GitError) &&
+    !ctx.errors.has(ApplyEmptyCommitError) &&
+    !ctx.errors.has(RestoreUnstagedChangesError)
+  ) {
     return MESSAGES.GIT_ERROR
   }
   // Should be skipped when reverting to original state fails
-  if (ctx.gitRestoreOriginalStateError) {
+  if (ctx.errors.has(RestoreOriginalStateError)) {
     return MESSAGES.GIT_ERROR
   }
 }
 
-const wasFailed = (context) => {
-  const itemCount = Object.keys(context).length
-  if (itemCount === 0) return false
-  if ('hasPartiallyStagedFiles' in context && itemCount === 1) return false
-  return true
-}
-
 /**
  * Executes all tasks and either resolves or rejects the promise
  *
@@ -132,7 +141,13 @@ const runAll = async (
   // Warn user when their command includes `git add`
   let hasDeprecatedGitAdd = false
 
+  const listrCtx = {
+    hasPartiallyStagedFiles: false,
+    errors: new Set([])
+  }
+
   const listrOptions = {
+    ctx: listrCtx,
     dateFormat: false,
     exitOnError: false,
     renderer: getRenderer({ debug, quiet })
@@ -168,9 +183,9 @@ const runAll = async (
           new Listr(subTasks, {
             // In sub-tasks we don't want to run concurrently
             // and we want to abort on errors
+            ...listrOptions,
             collapse: false,
             concurrent: false,
-            dateFormat: false,
             exitOnError: true
           }),
         skip: () => {
@@ -187,10 +202,10 @@ const runAll = async (
       // No need to show number of task chunks when there's only one
       title:
         chunkCount > 1 ? `Running tasks (chunk ${index + 1}/${chunkCount})...` : 'Running tasks...',
-      task: (ctx) => new Listr(chunkListrTasks, { ...listrOptions, ctx, concurrent }),
-      skip: (ctx = {}) => {
+      task: () => new Listr(chunkListrTasks, { ...listrOptions, concurrent }),
+      skip: () => {
         // Skip if the first step (backup) failed
-        if (ctx.gitError) return MESSAGES.GIT_ERROR
+        if (listrCtx.errors.has(GitError)) return MESSAGES.GIT_ERROR
         // Skip chunk when no every task is skipped (due to no matches)
         if (chunkListrTasks.every((task) => task.skip())) return 'No tasks to run.'
         return false
@@ -243,7 +258,9 @@ const runAll = async (
         task: (ctx) => git.restoreOriginalState(ctx),
         enabled: (ctx) =>
           shouldBackup &&
-          (ctx.taskError || ctx.gitApplyEmptyCommitError || ctx.gitRestoreUnstagedChangesError),
+          (ctx.errors.has(TaskError) ||
+            ctx.errors.has(ApplyEmptyCommitError) ||
+            ctx.errors.has(RestoreUnstagedChangesError)),
         skip: shouldSkipRevert
       },
       {
@@ -258,13 +275,13 @@ const runAll = async (
 
   const context = await runner.run()
 
-  if (wasFailed(context)) {
-    if (context.gitApplyEmptyCommitError) {
+  if (context.errors.size > 0) {
+    if (context.errors.has(ApplyEmptyCommitError)) {
       logger.warn(`
   ${symbols.warning} ${chalk.yellow(`lint-staged prevented an empty git commit.
   Use the --allow-empty option to continue, or check your task configuration`)}
   `)
-    } else if (context.gitError && !context.gitGetBackupStashError) {
+    } else if (context.errors.has(GitError) && !context.errors.has(GetBackupStashError)) {
       logger.error(`\n  ${symbols.error} ${chalk.red(`lint-staged failed due to a git error.`)}`)
 
       if (shouldBackup) {
@@ -283,6 +300,7 @@ const runAll = async (
 
 module.exports = runAll
 
+// Exported for testing
 module.exports.shouldSkip = {
   shouldSkipApplyModifications,
   shouldSkipRevert,

lib/symbols.js

@@ -0,0 +1,25 @@
+'use strict'
+
+const TaskError = Symbol('TaskError')
+
+const GitError = Symbol('GitError')
+
+const GetBackupStashError = Symbol('GetBackupStashError')
+
+const HideUnstagedChangesError = Symbol('HideUnstagedChangesError')
+
+const ApplyEmptyCommitError = Symbol('ApplyEmptyCommitError')
+
+const RestoreUnstagedChangesError = Symbol('RestoreUnstagedChangesError')
+
+const RestoreOriginalStateError = Symbol('RestoreOriginalStateError')
+
+module.exports = {
+  ApplyEmptyCommitError,
+  GetBackupStashError,
+  GitError,
+  HideUnstagedChangesError,
+  RestoreOriginalStateError,
+  RestoreUnstagedChangesError,
+  TaskError
+}

test/gitWorkflow.spec.js

@@ -72,7 +72,7 @@ describe('gitWorkflow', () => {
         gitConfigDir: path.resolve(cwd, './.git')
       })
       jest.doMock('execa', () => Promise.reject({}))
-      const ctx = {}
+      const ctx = { hasPartiallyStagedFiles: false, errors: new Set() }
       // mock a simple failure
       gitWorkflow.getPartiallyStagedFiles = () => ['foo']
       gitWorkflow.getHiddenFilepath = () => {
@@ -81,10 +81,14 @@ describe('gitWorkflow', () => {
       await expect(gitWorkflow.prepare(ctx, false)).rejects.toThrowErrorMatchingInlineSnapshot(
         `"test"`
       )
-      expect(ctx).toEqual({
-        gitError: true,
-        hasPartiallyStagedFiles: true
-      })
+      expect(ctx).toMatchInlineSnapshot(`
+        Object {
+          "errors": Set {
+            Symbol(GitError),
+          },
+          "hasPartiallyStagedFiles": true,
+        }
+      `)
     })
   })
 
@@ -94,14 +98,19 @@ describe('gitWorkflow', () => {
         gitDir: cwd,
         gitConfigDir: path.resolve(cwd, './.git')
       })
-      const ctx = {}
+      const ctx = { hasPartiallyStagedFiles: false, errors: new Set() }
       await expect(gitWorkflow.cleanup(ctx)).rejects.toThrowErrorMatchingInlineSnapshot(
         `"lint-staged automatic backup is missing!"`
       )
-      expect(ctx).toEqual({
-        gitError: true,
-        gitGetBackupStashError: true
-      })
+      expect(ctx).toMatchInlineSnapshot(`
+        Object {
+          "errors": Set {
+            Symbol(GetBackupStashError),
+            Symbol(GitError),
+          },
+          "hasPartiallyStagedFiles": false,
+        }
+      `)
     })
   })
 
@@ -113,14 +122,19 @@ describe('gitWorkflow', () => {
       })
       const totallyRandom = `totally_random_file-${Date.now().toString()}`
       gitWorkflow.partiallyStagedFiles = [totallyRandom]
-      const ctx = {}
+      const ctx = { hasPartiallyStagedFiles: false, errors: new Set() }
       await expect(gitWorkflow.hideUnstagedChanges(ctx)).rejects.toThrowErrorMatchingInlineSnapshot(
         `"error: pathspec '${totallyRandom}' did not match any file(s) known to git"`
       )
-      expect(ctx).toEqual({
-        gitError: true,
-        gitHideUnstagedChangesError: true
-      })
+      expect(ctx).toMatchInlineSnapshot(`
+        Object {
+          "errors": Set {
+            Symbol(GitError),
+            Symbol(HideUnstagedChangesError),
+          },
+          "hasPartiallyStagedFiles": false,
+        }
+      `)
     })
   })
 })