fix: Avoid stashing if no staged files has been changed (#570) (#573)

* fix: add check for changed lint-staged files before stashing changes

- add test to check if stashing is skipped if no lint-staged files are changed

Fixes #570

* fix: simplify implementation, add comment as requested and remove "only" from "it.only"

Fixes #570

* fix: update test snapshot for "runAll should resolve the promise with no files" as stashing/linters are no longer executed if no staged files match the linters configuration

* fix: replace Array.forEach with Array.every, add message if lint-staged is skipped and update snapshots

Fixes #570

* fix: remove "No linters executed" and "in lint-staged.linters" copy

src/runAll.js

@@ -62,51 +62,54 @@ module.exports = function runAll(config) {
       renderer
     }
 
-    if (tasks.length) {
-      // Do not terminate main Listr process on SIGINT
-      process.on('SIGINT', () => {})
+    // If all of the configured "linters" should be skipped
+    // avoid executing any lint-staged logic
+    if (tasks.every(task => task.skip())) {
+      console.log('No staged files match any of provided globs.')
+      return 'No tasks to run.'
+    }
 
-      return new Listr(
-        [
-          {
-            title: 'Stashing changes...',
-            skip: async () => {
-              const hasPSF = await git.hasPartiallyStagedFiles()
-              if (!hasPSF) {
-                return 'No partially staged files found...'
-              }
-              return false
-            },
-            task: ctx => {
-              ctx.hasStash = true
-              return git.gitStashSave()
+    // Do not terminate main Listr process on SIGINT
+    process.on('SIGINT', () => {})
+
+    return new Listr(
+      [
+        {
+          title: 'Stashing changes...',
+          skip: async () => {
+            const hasPSF = await git.hasPartiallyStagedFiles()
+            if (!hasPSF) {
+              return 'No partially staged files found...'
             }
+            return false
           },
-          {
-            title: 'Running linters...',
-            task: () =>
-              new Listr(tasks, {
-                ...listrBaseOptions,
-                concurrent,
-                exitOnError: !concurrent // Wait for all errors when running concurrently
-              })
-          },
-          {
-            title: 'Updating stash...',
-            enabled: ctx => ctx.hasStash,
-            skip: ctx =>
-              ctx.hasErrors && 'Skipping stash update since some tasks exited with errors',
-            task: () => git.updateStash()
-          },
-          {
-            title: 'Restoring local changes...',
-            enabled: ctx => ctx.hasStash,
-            task: () => git.gitStashPop()
+          task: ctx => {
+            ctx.hasStash = true
+            return git.gitStashSave()
           }
-        ],
-        listrBaseOptions
-      ).run()
-    }
-    return 'No tasks to run.'
+        },
+        {
+          title: 'Running linters...',
+          task: () =>
+            new Listr(tasks, {
+              ...listrBaseOptions,
+              concurrent,
+              exitOnError: !concurrent // Wait for all errors when running concurrently
+            })
+        },
+        {
+          title: 'Updating stash...',
+          enabled: ctx => ctx.hasStash,
+          skip: ctx => ctx.hasErrors && 'Skipping stash update since some tasks exited with errors',
+          task: () => git.updateStash()
+        },
+        {
+          title: 'Restoring local changes...',
+          enabled: ctx => ctx.hasStash,
+          task: () => git.gitStashPop()
+        }
+      ],
+      listrBaseOptions
+    ).run()
   })
 }

test/__snapshots__/runAll.spec.js.snap

@@ -31,14 +31,7 @@ LOG Running linters... [completed]"
 
 exports[`runAll should resolve the promise with no files 1`] = `
 "
-LOG Stashing changes... [started]
-LOG Stashing changes... [skipped]
-LOG → No partially staged files found...
-LOG Running linters... [started]
-LOG Running tasks for *.js [started]
-LOG Running tasks for *.js [skipped]
-LOG → No staged files match *.js
-LOG Running linters... [completed]"
+LOG No staged files match any of provided globs."
 `;
 
 exports[`runAll should skip linters and stash update but perform working copy restore if terminated 1`] = `
@@ -83,6 +76,40 @@ LOG Running tasks for *.js [completed]
 LOG Running linters... [completed]"
 `;
 
+exports[`runAll should skip stashing changes if no lint-staged files are changed 1`] = `
+"
+LOG No staged files match any of provided globs."
+`;
+
+exports[`runAll should skip updating stash if there are errors during linting 1`] = `
+"
+LOG Stashing changes... [started]
+LOG Stashing changes... [completed]
+LOG Running linters... [started]
+LOG Running tasks for *.js [started]
+LOG echo \\"sample\\" [started]
+LOG echo \\"sample\\" [failed]
+LOG →
+LOG Running tasks for *.js [failed]
+LOG →
+LOG Running linters... [failed]
+LOG Updating stash... [started]
+LOG Updating stash... [skipped]
+LOG → Skipping stash update since some tasks exited with errors
+LOG Restoring local changes... [started]
+LOG Restoring local changes... [completed]
+LOG {
+  name: 'ListrError',
+  errors: [
+    {
+      privateMsg: '\\\\n\\\\n\\\\n× echo \\"sample\\" found some errors. Please fix them and try committing again.\\\\n\\\\nLinter finished with error',
+      context: {hasStash: true, hasErrors: true}
+    }
+  ],
+  context: {hasStash: true, hasErrors: true}
+}"
+`;
+
 exports[`runAll should skip updating stash if there are errors during linting 1`] = `
 "
 LOG Stashing changes... [started]

test/runAll.spec.js

@@ -161,4 +161,31 @@ describe('runAll', () => {
       expect(err).toEqual('test')
     }
   })
+
+  it('should skip stashing changes if no lint-staged files are changed', async () => {
+    expect.assertions(4)
+    hasPartiallyStagedFiles.mockImplementationOnce(() => Promise.resolve(true))
+    sgfMock.mockImplementationOnce((params, callback) => {
+      callback(null, [{ filename: 'sample.java', status: 'Modified' }])
+    })
+    execa.mockImplementationOnce(() =>
+      Promise.resolve({
+        stdout: '',
+        stderr: 'Linter finished with error',
+        code: 1,
+        failed: true,
+        cmd: 'mock cmd'
+      })
+    )
+
+    try {
+      await runAll(getConfig({ linters: { '*.js': ['echo "sample"'] } }))
+    } catch (err) {
+      console.log(err)
+    }
+    expect(console.printHistory()).toMatchSnapshot()
+    expect(gitStashSave).toHaveBeenCalledTimes(0)
+    expect(updateStash).toHaveBeenCalledTimes(0)
+    expect(gitStashPop).toHaveBeenCalledTimes(0)
+  })
 })