Fix #988 - Support --fix-dry-run for ESLint where available, and --fix-to-stdout for eslint_d

2 files changed, 114 insertions, 3 deletions

diff --git a/autoload/ale/fixers/eslint.vim b/autoload/ale/fixers/eslint.vim
index ad9e9e06..969ac202 100644
--- a/autoload/ale/fixers/eslint.vim
+++ b/autoload/ale/fixers/eslint.vim
@@ -3,12 +3,43 @@
 
 function! ale#fixers#eslint#Fix(buffer) abort
     let l:executable = ale#handlers#eslint#GetExecutable(a:buffer)
+
+    let l:command = ale#semver#HasVersion(l:executable)
+    \   ? ''
+    \   : ale#node#Executable(a:buffer, l:executable) . ' --version'
+
+    return {
+    \   'command': l:command,
+    \   'chain_with': 'ale#fixers#eslint#ApplyFixForVersion',
+    \}
+endfunction
+
+function! ale#fixers#eslint#ApplyFixForVersion(buffer, version_output) abort
+    let l:executable = ale#handlers#eslint#GetExecutable(a:buffer)
+    let l:version = ale#semver#GetVersion(l:executable, a:version_output)
+
     let l:config = ale#handlers#eslint#FindConfig(a:buffer)
 
     if empty(l:config)
         return 0
     endif
 
+    " Use --fix-to-stdout with eslint_d
+    if l:executable =~# 'eslint_d$' && ale#semver#GTE(l:version, [3, 19, 0])
+        return {
+        \   'command': ale#node#Executable(a:buffer, l:executable)
+        \       . ' --stdin-filename %s --stdin --fix-to-stdout',
+        \}
+    endif
+
+    " 4.9.0 is the first version with --fix-dry-run
+    if ale#semver#GTE(l:version, [4, 9, 0])
+        return {
+        \   'command': ale#node#Executable(a:buffer, l:executable)
+        \       . ' --stdin-filename %s --stdin --fix-dry-run',
+        \}
+    endif
+
     return {
     \   'command': ale#node#Executable(a:buffer, l:executable)
     \       . ' -c ' . ale#Escape(l:config)
diff --git a/test/fixers/test_eslint_fixer_callback.vader b/test/fixers/test_eslint_fixer_callback.vader
index d4783fc7..60bb859e 100644
--- a/test/fixers/test_eslint_fixer_callback.vader
+++ b/test/fixers/test_eslint_fixer_callback.vader
@@ -3,10 +3,12 @@ Before:
 
 After:
   call ale#test#RestoreDirectory()
+  call ale#semver#ResetVersionCache()
 
 Execute(The executable path should be correct):
   call ale#test#SetFilename('../eslint-test-files/react-app/subdir/testfile.js')
 
+  " eslint_d output with an older eslint version is used here.
   AssertEqual
   \ {
   \   'read_temporary_file': 1,
@@ -15,7 +17,7 @@ Execute(The executable path should be correct):
   \     . ' -c ' . ale#Escape(ale#path#Winify(g:dir . '/../eslint-test-files/react-app/.eslintrc.js'))
   \     . ' --fix %t',
   \ },
-  \ ale#fixers#eslint#Fix(bufnr(''))
+  \ ale#fixers#eslint#ApplyFixForVersion(bufnr(''), ['v4.4.1 (eslint_d v5.1.0)'])
 
 Execute(The lower priority configuration file in a nested directory should be preferred):
   call ale#test#SetFilename('../eslint-test-files/react-app/subdir-with-config/testfile.js')
@@ -28,7 +30,7 @@ Execute(The lower priority configuration file in a nested directory should be pr
   \     . ' -c ' . ale#Escape(ale#path#Winify(g:dir . '/../eslint-test-files/react-app/subdir-with-config/.eslintrc'))
   \     . ' --fix %t',
   \ },
-  \ ale#fixers#eslint#Fix(bufnr(''))
+  \ ale#fixers#eslint#ApplyFixForVersion(bufnr(''), [])
 
 Execute(package.json should be used as a last resort):
   call ale#test#SetFilename('../eslint-test-files/react-app/subdir-with-package-json/testfile.js')
@@ -41,7 +43,7 @@ Execute(package.json should be used as a last resort):
   \     . ' -c ' . ale#Escape(ale#path#Winify(g:dir . '/../eslint-test-files/react-app/.eslintrc.js'))
   \     . ' --fix %t',
   \ },
-  \ ale#fixers#eslint#Fix(bufnr(''))
+  \ ale#fixers#eslint#ApplyFixForVersion(bufnr(''), [])
 
   call ale#test#SetFilename('../eslint-test-files/package.json')
 
@@ -53,4 +55,82 @@ Execute(package.json should be used as a last resort):
   \     . ' -c ' . ale#Escape(ale#path#Winify(g:dir . '/../eslint-test-files/package.json'))
   \     . ' --fix %t',
   \ },
+  \ ale#fixers#eslint#ApplyFixForVersion(bufnr(''), [])
+
+Execute(The version check should be correct):
+  call ale#test#SetFilename('../eslint-test-files/react-app/subdir/testfile.js')
+
+  AssertEqual
+  \ {
+  \   'chain_with': 'ale#fixers#eslint#ApplyFixForVersion',
+  \   'command': (has('win32') ? 'node.exe ' : '')
+  \     . ale#Escape(ale#path#Winify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
+  \     . ' --version'
+  \ },
+  \ ale#fixers#eslint#Fix(bufnr(''))
+
+Execute(--fix-dry-run should be used for 4.9.0 and up):
+  call ale#test#SetFilename('../eslint-test-files/react-app/subdir/testfile.js')
+
+  AssertEqual
+  \ {
+  \   'command': (has('win32') ? 'node.exe ' : '')
+  \     . ale#Escape(ale#path#Winify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
+  \     . ' --stdin-filename %s --stdin --fix-dry-run',
+  \ },
+  \ ale#fixers#eslint#ApplyFixForVersion(bufnr(''), ['4.9.0'])
+
+Execute(--fix-to-stdout should be used for eslint_d):
+  call ale#test#SetFilename('../eslint-test-files/app-with-eslint-d/testfile.js')
+
+  AssertEqual
+  \ {
+  \   'read_temporary_file': 1,
+  \   'command': (has('win32') ? 'node.exe ' : '')
+  \     . ale#Escape(ale#path#Winify(g:dir . '/../eslint-test-files/app-with-eslint-d/node_modules/.bin/eslint_d'))
+  \     . ' -c ' . ale#Escape(ale#path#Winify(g:dir . '/../eslint-test-files/package.json'))
+  \     . ' --fix %t',
+  \ },
+  \ ale#fixers#eslint#ApplyFixForVersion(bufnr(''), [''])
+
+  " The option should be used when eslint_d is new enough.
+  " We look at the ESLint version instead of the eslint_d version.
+  AssertEqual
+  \ {
+  \   'command': (has('win32') ? 'node.exe ' : '')
+  \     . ale#Escape(ale#path#Winify(g:dir . '/../eslint-test-files/app-with-eslint-d/node_modules/.bin/eslint_d'))
+  \     . ' --stdin-filename %s --stdin --fix-to-stdout',
+  \ },
+  \ ale#fixers#eslint#ApplyFixForVersion(bufnr(''), ['v3.19.0 (eslint_d v4.2.0)'])
+
+  " The option should be used for new versions too.
+  AssertEqual
+  \ {
+  \   'command': (has('win32') ? 'node.exe ' : '')
+  \     . ale#Escape(ale#path#Winify(g:dir . '/../eslint-test-files/app-with-eslint-d/node_modules/.bin/eslint_d'))
+  \     . ' --stdin-filename %s --stdin --fix-to-stdout',
+  \ },
+  \ ale#fixers#eslint#ApplyFixForVersion(bufnr(''), ['4.9.0'])
+
+Execute(The version number should be cached):
+  call ale#test#SetFilename('../eslint-test-files/react-app/subdir-with-config/testfile.js')
+
+  " Call the second callback with the version output.
+  call ale#fixers#eslint#ApplyFixForVersion(bufnr(''), ['4.9.0'])
+
+  " The version command should be skipped.
+  AssertEqual
+  \ {
+  \   'chain_with': 'ale#fixers#eslint#ApplyFixForVersion',
+  \   'command': '',
+  \ },
   \ ale#fixers#eslint#Fix(bufnr(''))
+
+  " Call it again without the version output. We should use the newer command.
+  AssertEqual
+  \ {
+  \   'command': (has('win32') ? 'node.exe ' : '')
+  \     . ale#Escape(ale#path#Winify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
+  \     . ' --stdin-filename %s --stdin --fix-dry-run',
+  \ },
+  \ ale#fixers#eslint#ApplyFixForVersion(bufnr(''), [])