Repository: okonet/lint-staged
Pull request #681 (2019-08-05 16:56:10)

fix: validateConfig validates function task return values
This PR adds more precise validation of function tasks, which should be of the form `(filenames: string[]) => string | string[]`. Specifically, the return value is validated by supplying the task with an array of single filename `['[filename]']`.

#### Fixes:
- [x] https://github.com/okonet/lint-staged/issues/680

codecov[bot]: # [Codecov](https://codecov.io/gh/okonet/lint-staged/pull/681?src=pr&el=h1) Report
> Merging [#681](https://codecov.io/gh/okonet/lint-staged/pull/681?src=pr&el=desc) into [master](https://codecov.io/gh/okonet/lint-staged/commit/710be313809901e6fe3275c476b6f82241093989?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/okonet/lint-staged/pull/681/graphs/tree.svg?width=650&token=LSoEgsjk4O&height=150&src=pr)](https://codecov.io/gh/okonet/lint-staged/pull/681?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #681      +/-   ##
==========================================
+ Coverage   99.67%   99.67%   +<.01%     
==========================================
  Files          11       11              
  Lines         305      311       +6     
  Branches       57       60       +3     
==========================================
+ Hits          304      310       +6     
  Misses          1        1
```


| [Impacted Files](https://codecov.io/gh/okonet/lint-staged/pull/681?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/validateConfig.js](https://codecov.io/gh/okonet/lint-staged/pull/681/diff?src=pr&el=tree#diff-c3JjL3ZhbGlkYXRlQ29uZmlnLmpz) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/okonet/lint-staged/pull/681?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/okonet/lint-staged/pull/681?src=pr&el=footer). Last update [710be31...6ea4a96](https://codecov.io/gh/okonet/lint-staged/pull/681?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/validateConfig.js b/src/validateConfig.js
index 795199c..0c5b8db 100644
--- a/src/validateConfig.js
+++ b/src/validateConfig.js
@@ -46,40 +46,55 @@ module.exports = function validateConfig(config) {
   if (!config || typeof config !== 'object') {
     errors.push('Configuration should be an object!')
   } else {
-    const globs = Object.keys(config)
+    const entries = Object.entries(config)
 
-    if (globs.length === 0) {
+    if (entries.length === 0) {
       errors.push('Configuration should not be empty!')
     }
 
-    globs.forEach(key => {
-      if (TEST_DEPRECATED_KEYS.has(key)) {
-        const testFn = TEST_DEPRECATED_KEYS.get(key)
-        if (testFn(config[key])) {
+    entries.forEach(([pattern, task]) => {
+      if (TEST_DEPRECATED_KEYS.has(pattern)) {
+        const testFn = TEST_DEPRECATED_KEYS.get(pattern)
+        if (testFn(task)) {
           errors.push(
             createError(
-              key,
+              pattern,
               'Advanced configuration has been deprecated. For more info, please visit: https://github.com/okonet/lint-staged',
-              config[key]
+              task
             )
           )
         }
       }
 
       if (
-        (!Array.isArray(config[key]) ||
-          config[key].some(item => typeof item !== 'string' && typeof item !== 'function')) &&
-        typeof config[key] !== 'string' &&
-        typeof config[key] !== 'function'
+        (!Array.isArray(task) ||
+          task.some(item => typeof item !== 'string' && typeof item !== 'function')) &&
+        typeof task !== 'string' &&
+        typeof task !== 'function'
       ) {
         errors.push(
           createError(
-            key,
+            pattern,
             'Should be a string, a function, or an array of strings and functions',
-            config[key]
+            task
           )
         )
       }
+
+      entries.forEach(([, task]) => {
+        if (typeof task !== 'function') return
+        const resolved = task(['[filename]'])
+        if (typeof resolved === 'string') return
+        if (!Array.isArray(resolved) || resolved.some(subtask => typeof subtask !== 'string')) {
+          errors.push(
+            createError(
+              task,
+              'Function task should return a string or an array of strings',
+              resolved
+            )
+          )
+        }
+      })
     })
   }
 


Test patch:
diff --git a/test/__snapshots__/validateConfig.spec.js.snap b/test/__snapshots__/validateConfig.spec.js.snap
index 3026e73..fe5213f 100644
--- a/test/__snapshots__/validateConfig.spec.js.snap
+++ b/test/__snapshots__/validateConfig.spec.js.snap
@@ -1,9 +1,11 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`validateConfig should not throw and should print nothing for function linter 1`] = `""`;
+exports[`validateConfig should not throw and should print nothing for function task 1`] = `""`;
 
 exports[`validateConfig should not throw and should print nothing for valid config 1`] = `""`;
 
+exports[`validateConfig should not throw when config contains deprecated key but with valid task 1`] = `""`;
+
 exports[`validateConfig should throw and should print validation errors for invalid config 1`] = `
 "● Validation Error:
 
@@ -148,3 +150,15 @@ Please refer to https://github.com/okonet/lint-staged#configuration for more inf
 `;
 
 exports[`validateConfig should throw when detecting deprecated advanced configuration 2`] = `""`;
+
+exports[`validateConfig should throw when function task returns incorrect values 1`] = `
+"● Validation Error:
+
+  Invalid value for 'filenames => filenames.map(file => [\`eslint --fix \${file}\`, \`git add \${file}\`])'.
+
+  Function task should return a string or an array of strings.
+ 
+  Configured value is: [['eslint --fix [filename]', 'git add [filename]']]
+
+Please refer to https://github.com/okonet/lint-staged#configuration for more information..."
+`;
diff --git a/test/validateConfig.spec.js b/test/validateConfig.spec.js
index 992f9e0..c1949ea 100644
--- a/test/validateConfig.spec.js
+++ b/test/validateConfig.spec.js
@@ -36,7 +36,7 @@ describe('validateConfig', () => {
     expect(console.printHistory()).toMatchSnapshot()
   })
 
-  it('should not throw and should print nothing for function linter', () => {
+  it('should not throw and should print nothing for function task', () => {
     expect(() =>
       validateConfig({
         '*.js': filenames => {
@@ -49,6 +49,13 @@ describe('validateConfig', () => {
     expect(console.printHistory()).toMatchSnapshot()
   })
 
+  it('should throw when function task returns incorrect values', () => {
+    const invalidConfig = {
+      '*.js': filenames => filenames.map(file => [`eslint --fix ${file}`, `git add ${file}`])
+    }
+    expect(() => validateConfig(invalidConfig)).toThrowErrorMatchingSnapshot()
+  })
+
   it('should throw when detecting deprecated advanced configuration', () => {
     const advancedConfig = {
       chunkSize: 10,
@@ -66,4 +73,12 @@ describe('validateConfig', () => {
     expect(() => validateConfig(advancedConfig)).toThrowErrorMatchingSnapshot()
     expect(console.printHistory()).toMatchSnapshot()
   })
+
+  it('should not throw when config contains deprecated key but with valid task', () => {
+    const stillValidConfig = {
+      concurrent: 'my command'
+    }
+    expect(() => validateConfig(stillValidConfig)).not.toThrow()
+    expect(console.printHistory()).toMatchSnapshot()
+  })
 })