Fix #491 - Only set -x for shellcheck for versions which support the option

2 files changed, 109 insertions, 10 deletions

diff --git a/ale_linters/sh/shellcheck.vim b/ale_linters/sh/shellcheck.vim
index 32b47e22..1f6bdf81 100644
--- a/ale_linters/sh/shellcheck.vim
+++ b/ale_linters/sh/shellcheck.vim
@@ -15,6 +15,8 @@ let g:ale_sh_shellcheck_executable =
 let g:ale_sh_shellcheck_options =
 \   get(g:, 'ale_sh_shellcheck_options', '')
 
+let s:version_cache = {}
+
 function! ale_linters#sh#shellcheck#GetExecutable(buffer) abort
     return ale#Var(a:buffer, 'sh_shellcheck_executable')
 endfunction
@@ -43,22 +45,58 @@ function! ale_linters#sh#shellcheck#GetDialectArgument(buffer) abort
     return ''
 endfunction
 
-function! ale_linters#sh#shellcheck#GetCommand(buffer) abort
+function! ale_linters#sh#shellcheck#VersionCheck(buffer) abort
+    let l:executable = ale_linters#sh#shellcheck#GetExecutable(a:buffer)
+
+    " Don't check the version again if we've already cached it.
+    if has_key(s:version_cache, l:executable)
+        return ''
+    endif
+
+    return ale#Escape(l:executable) . ' --version'
+endfunction
+
+" Get the shellcheck version from the cache, or parse it and cache it.
+function! s:GetVersion(executable, output) abort
+    let l:version = get(s:version_cache, a:executable, [])
+
+    for l:match in ale#util#GetMatches(a:output, '\v\d+\.\d+\.\d+')
+        let l:version = ale#semver#Parse(l:match[0])
+        let s:version_cache[a:executable] = l:version
+    endfor
+
+    return l:version
+endfunction
+
+function! s:CanUseExternalOption(version) abort
+    return !empty(a:version)
+    \   && ale#semver#GreaterOrEqual(a:version, [0, 4, 0])
+endfunction
+
+function! ale_linters#sh#shellcheck#GetCommand(buffer, version_output) abort
+    let l:executable = ale_linters#sh#shellcheck#GetExecutable(a:buffer)
+    let l:version = s:GetVersion(l:executable, a:version_output)
+
     let l:options = ale#Var(a:buffer, 'sh_shellcheck_options')
     let l:exclude_option = ale#Var(a:buffer, 'sh_shellcheck_exclusions')
     let l:dialect = ale_linters#sh#shellcheck#GetDialectArgument(a:buffer)
+    let l:external_option = s:CanUseExternalOption(l:version) ? ' -x' : ''
 
     return ale#path#BufferCdString(a:buffer)
-    \   . ale#Escape(ale_linters#sh#shellcheck#GetExecutable(a:buffer))
+    \   . ale#Escape(l:executable)
     \   . (!empty(l:dialect) ? ' -s ' . l:dialect : '')
     \   . (!empty(l:options) ? ' ' . l:options : '')
     \   . (!empty(l:exclude_option) ? ' -e ' . l:exclude_option : '')
-    \   . ' -x -f gcc -'
+    \   . l:external_option
+    \   . ' -f gcc -'
 endfunction
 
 call ale#linter#Define('sh', {
 \   'name': 'shellcheck',
 \   'executable_callback': 'ale_linters#sh#shellcheck#GetExecutable',
-\   'command_callback': 'ale_linters#sh#shellcheck#GetCommand',
+\   'command_chain': [
+\       {'callback': 'ale_linters#sh#shellcheck#VersionCheck'},
+\       {'callback': 'ale_linters#sh#shellcheck#GetCommand'},
+\   ],
 \   'callback': 'ale#handlers#gcc#HandleGCCFormat',
 \})
diff --git a/test/command_callback/test_shellcheck_command_callback.vader b/test/command_callback/test_shellcheck_command_callback.vader
index 13e9a2c1..bf422b21 100644
--- a/test/command_callback/test_shellcheck_command_callback.vader
+++ b/test/command_callback/test_shellcheck_command_callback.vader
@@ -13,7 +13,7 @@ Before:
   call ale#test#SetFilename('test.sh')
 
   let b:prefix = 'cd ' . ale#Escape(ale#path#Winify(g:dir)) . ' && '
-  let b:suffix = ' -x -f gcc -'
+  let b:suffix = ' -f gcc -'
 
 After:
   Restore
@@ -31,14 +31,14 @@ After:
 Execute(The default shellcheck command should be correct):
   AssertEqual
   \ b:prefix . ale#Escape('shellcheck') . b:suffix,
-  \ ale_linters#sh#shellcheck#GetCommand(bufnr(''))
+  \ ale_linters#sh#shellcheck#GetCommand(bufnr(''), [])
 
 Execute(The shellcheck command should accept options):
   let b:ale_sh_shellcheck_options = '--foobar'
 
   AssertEqual
   \ b:prefix . ale#Escape('shellcheck') . ' --foobar' . b:suffix,
-  \ ale_linters#sh#shellcheck#GetCommand(bufnr(''))
+  \ ale_linters#sh#shellcheck#GetCommand(bufnr(''), [])
 
 Execute(The shellcheck command should accept options and exclusions):
   let b:ale_sh_shellcheck_options = '--foobar'
@@ -46,14 +46,14 @@ Execute(The shellcheck command should accept options and exclusions):
 
   AssertEqual
   \ b:prefix . ale#Escape('shellcheck') . ' --foobar -e foo,bar' . b:suffix,
-  \ ale_linters#sh#shellcheck#GetCommand(bufnr(''))
+  \ ale_linters#sh#shellcheck#GetCommand(bufnr(''), [])
 
 Execute(The shellcheck command should include the dialect):
   let b:is_bash = 1
 
   AssertEqual
   \ b:prefix . ale#Escape('shellcheck') . ' -s bash' . b:suffix,
-  \ ale_linters#sh#shellcheck#GetCommand(bufnr(''))
+  \ ale_linters#sh#shellcheck#GetCommand(bufnr(''), [])
 
 Execute(The shellcheck command should include the dialect before options and exclusions):
   let b:is_bash = 1
@@ -65,4 +65,65 @@ Execute(The shellcheck command should include the dialect before options and exc
   \   . ale#Escape('shellcheck')
   \   . ' -s bash --foobar -e foo,bar'
   \   . b:suffix,
-  \ ale_linters#sh#shellcheck#GetCommand(bufnr(''))
+  \ ale_linters#sh#shellcheck#GetCommand(bufnr(''), [])
+
+Execute(The VersionCheck function should return the --version command):
+  AssertEqual
+  \ ale#Escape('shellcheck') . ' --version',
+  \ ale_linters#sh#shellcheck#VersionCheck(bufnr(''))
+
+  let g:ale_sh_shellcheck_executable = 'foobar'
+
+  AssertEqual
+  \ ale#Escape('foobar') . ' --version',
+  \ ale_linters#sh#shellcheck#VersionCheck(bufnr(''))
+
+Execute(The -x option should be added when the version is new enough):
+  AssertEqual
+  \ b:prefix . ale#Escape('shellcheck') . ' -x' . b:suffix,
+  \ ale_linters#sh#shellcheck#GetCommand(bufnr(''), [
+  \   'ShellCheck - shell script analysis tool',
+  \   'version: 0.4.4',
+  \   'license: GNU General Public License, version 3',
+  \   'website: http://www.shellcheck.net',
+  \ ])
+
+  " We should cache the version check
+  AssertEqual
+  \ b:prefix . ale#Escape('shellcheck') . ' -x' . b:suffix,
+  \ ale_linters#sh#shellcheck#GetCommand(bufnr(''), [])
+
+  AssertEqual '', ale_linters#sh#shellcheck#VersionCheck(bufnr(''))
+
+Execute(The version check shouldn't be run again for new versions):
+  call ale_linters#sh#shellcheck#GetCommand(bufnr(''), [
+  \ 'ShellCheck - shell script analysis tool',
+  \ 'version: 0.4.4',
+  \ 'license: GNU General Public License, version 3',
+  \ 'website: http://www.shellcheck.net',
+  \])
+
+Execute(The -x option should not be added when the version is too old):
+  AssertEqual
+  \ b:prefix . ale#Escape('shellcheck') . b:suffix,
+  \ ale_linters#sh#shellcheck#GetCommand(bufnr(''), [
+  \   'ShellCheck - shell script analysis tool',
+  \   'version: 0.3.9',
+  \   'license: GNU General Public License, version 3',
+  \   'website: http://www.shellcheck.net',
+  \ ])
+
+  " We should cache the version check
+  AssertEqual
+  \ b:prefix . ale#Escape('shellcheck') . b:suffix,
+  \ ale_linters#sh#shellcheck#GetCommand(bufnr(''), [])
+
+Execute(The version check shouldn't be run again for old versions):
+  call ale_linters#sh#shellcheck#GetCommand(bufnr(''), [
+  \ 'ShellCheck - shell script analysis tool',
+  \ 'version: 0.3.9',
+  \ 'license: GNU General Public License, version 3',
+  \ 'website: http://www.shellcheck.net',
+  \])
+
+  AssertEqual '', ale_linters#sh#shellcheck#VersionCheck(bufnr(''))