Add end_col and end_lnum to ShellCheck (#4692)

* Add end_col and end_lnum to ShellCheck

ShellCheck supports a JSON format mode which includes an 'endLine' and
'endColumn' field.

We must use the newer 'json1' format as it properly treats tabs as a
single character. 'json1' was not supported until v0.7.0 in 2019[1], so
we maintain support for the older GCC based format.

[1] https://github.com/koalaman/shellcheck/blob/v0.9.0/CHANGELOG.md?plain=1#L121

* Add wiki link to ShellCheck json output

2 files changed, 187 insertions, 7 deletions

diff --git a/autoload/ale/handlers/shellcheck.vim b/autoload/ale/handlers/shellcheck.vim
index fd540e8e..c2514d59 100644
--- a/autoload/ale/handlers/shellcheck.vim
+++ b/autoload/ale/handlers/shellcheck.vim
@@ -49,6 +49,7 @@ function! ale#handlers#shellcheck#GetCommand(buffer, version) abort
     let l:exclude_option = ale#Var(a:buffer, 'sh_shellcheck_exclusions')
     let l:dialect = ale#Var(a:buffer, 'sh_shellcheck_dialect')
     let l:external_option = ale#semver#GTE(a:version, [0, 4, 0]) ? ' -x' : ''
+    let l:format = ale#semver#GTE(a:version, [0, 7, 0]) ? 'json1' : 'gcc'
 
     if l:dialect is# 'auto'
         let l:dialect = ale#handlers#shellcheck#GetDialectArgument(a:buffer)
@@ -59,10 +60,69 @@ function! ale#handlers#shellcheck#GetCommand(buffer, version) abort
     \   . (!empty(l:options) ? ' ' . l:options : '')
     \   . (!empty(l:exclude_option) ? ' -e ' . l:exclude_option : '')
     \   . l:external_option
-    \   . ' -f gcc -'
+    \   . ' -f ' . l:format . ' -'
 endfunction
 
-function! ale#handlers#shellcheck#Handle(buffer, lines) abort
+function! s:HandleShellcheckJSON(buffer, lines) abort
+    try
+        let l:errors = json_decode(a:lines[0])
+    catch
+        return []
+    endtry
+
+    if !has_key(l:errors, 'comments')
+        return []
+    endif
+
+    let l:output = []
+
+    for l:error in l:errors['comments']
+        if l:error['level'] is# 'error'
+            let l:type = 'E'
+        elseif l:error['level'] is# 'info'
+            let l:type = 'I'
+        elseif l:error['level'] is# 'style'
+            let l:type = 'I'
+        else
+            let l:type = 'W'
+        endif
+
+        let l:item = {
+        \   'lnum': l:error['line'],
+        \   'type': l:type,
+        \   'text': l:error['message'],
+        \   'code': 'SC' . l:error['code'],
+        \   'detail': l:error['message'] . "\n\nFor more information:\n  https://www.shellcheck.net/wiki/SC" . l:error['code'],
+        \}
+
+        if has_key(l:error, 'column')
+            let l:item.col = l:error['column']
+        endif
+
+        if has_key(l:error, 'endColumn')
+            let l:item.end_col = l:error['endColumn'] - 1
+        endif
+
+        if has_key(l:error, 'endLine')
+            let l:item.end_lnum = l:error['endLine']
+        endif
+
+
+        " If the filename is something like <stdin>, <nofile> or -, then
+        " this is an error for the file we checked.
+        if has_key(l:error, 'file')
+            if l:error['file'] isnot# '-' && l:error['file'][0] isnot# '<'
+                let l:item['filename'] = l:error['file']
+            endif
+        endif
+
+        call add(l:output, l:item)
+    endfor
+
+    return l:output
+endfunction
+
+function! s:HandleShellcheckGCC(buffer, lines) abort
     let l:pattern = '\v^([a-zA-Z]?:?[^:]+):(\d+):(\d+)?:? ([^:]+): (.+) \[([^\]]+)\]$'
     let l:output = []
 
@@ -98,6 +158,12 @@ function! ale#handlers#shellcheck#Handle(buffer, lines) abort
     return l:output
 endfunction
 
+function! ale#handlers#shellcheck#Handle(buffer, version, lines) abort
+    return ale#semver#GTE(a:version, [0, 7, 0])
+    \   ? s:HandleShellcheckJSON(a:buffer, a:lines)
+    \   : s:HandleShellcheckGCC(a:buffer, a:lines)
+endfunction
+
 function! ale#handlers#shellcheck#DefineLinter(filetype) abort
     " This global variable can be set with a string of comma-separated error
     " codes to exclude from shellcheck. For example:
@@ -118,6 +184,14 @@ function! ale#handlers#shellcheck#DefineLinter(filetype) abort
     \       '%e --version',
     \       function('ale#handlers#shellcheck#GetCommand'),
     \   )},
-    \   'callback': 'ale#handlers#shellcheck#Handle',
+    \   'callback': {buffer, lines -> ale#semver#RunWithVersionCheck(
+    \       buffer,
+    \       ale#Var(buffer, 'sh_shellcheck_executable'),
+    \       '%e --version',
+    \       {buffer, version -> ale#handlers#shellcheck#Handle(
+    \           buffer,
+    \           l:version,
+    \           lines)},
+    \   )},
     \})
 endfunction
diff --git a/test/handler/test_shellcheck_handler.vader b/test/handler/test_shellcheck_handler.vader
index 33f12989..7d9e620b 100644
--- a/test/handler/test_shellcheck_handler.vader
+++ b/test/handler/test_shellcheck_handler.vader
@@ -4,7 +4,7 @@ Before:
 After:
   call ale#linter#Reset()
 
-Execute(The shellcheck handler should handle basic errors or warnings):
+Execute(The shellcheck handler should handle basic errors or warnings <0.7.0):
   AssertEqual
   \ [
   \   {
@@ -22,12 +22,12 @@ Execute(The shellcheck handler should handle basic errors or warnings):
   \     'code': 'SC1068',
   \   },
   \ ],
-  \ ale#handlers#shellcheck#Handle(bufnr(''), [
+  \ ale#handlers#shellcheck#Handle(bufnr(''), [0, 6, 0], [
   \   '-:2:1: warning: In POSIX sh, ''let'' is not supported. [SC2039]',
   \   '-:2:3: error: Don''t put spaces around the = in assignments. [SC1068]',
   \ ])
 
-Execute(The shellcheck handler should handle notes):
+Execute(The shellcheck handler should handle notes <0.7.0):
   AssertEqual
   \ [
   \   {
@@ -38,6 +38,112 @@ Execute(The shellcheck handler should handle notes):
   \     'code': 'SC2086',
   \   },
   \ ],
-  \ ale#handlers#shellcheck#Handle(bufnr(''), [
+  \ ale#handlers#shellcheck#Handle(bufnr(''), [0, 6, 0], [
   \   '-:3:3: note: Double quote to prevent globbing and word splitting. [SC2086]',
   \ ])
+
+Execute(The shellcheck handler should handle basic errors or warnings >=0.7.0):
+  AssertEqual
+  \ [
+  \   {
+  \     'lnum': 2,
+  \     'end_lnum': 3,
+  \     'col': 1,
+  \     'end_col': 1,
+  \     'type': 'W',
+  \     'text': 'In POSIX sh, ''let'' is not supported.',
+  \     'code': 'SC2039',
+  \     'detail': 'In POSIX sh, ''let'' is not supported.' . "\n\nFor more information:\n  https://www.shellcheck.net/wiki/" . 'SC2039',
+  \   },
+  \   {
+  \     'lnum': 2,
+  \     'end_lnum': 3,
+  \     'col': 3,
+  \     'end_col': 3,
+  \     'type': 'E',
+  \     'text': 'Don''t put spaces around the = in assignments.',
+  \     'code': 'SC1068',
+  \     'detail': 'Don''t put spaces around the = in assignments.' . "\n\nFor more information:\n  https://www.shellcheck.net/wiki/" . 'SC1068',
+  \   },
+  \ ],
+  \ ale#handlers#shellcheck#Handle(bufnr(''), [0, 7, 0], [
+  \   '{
+  \     "comments": [
+  \       {
+  \         "file":"-",
+  \         "line":2,
+  \         "endLine":3,
+  \         "column":1,
+  \         "endColumn":2,
+  \         "level":"warning",
+  \         "code":2039,
+  \         "message":"In POSIX sh, ''let'' is not supported.",
+  \         "fix": null
+  \       },
+  \       {
+  \         "file":"-",
+  \         "line":2,
+  \         "endLine":3,
+  \         "column":3,
+  \         "endColumn":4,
+  \         "level":"error",
+  \         "code":1068,
+  \         "message":"Don''t put spaces around the = in assignments.",
+  \         "fix": null
+  \       }
+  \     ]
+  \   }'
+  \ ])
+
+Execute(The shellcheck handler should handle info and style >=0.7.0):
+  AssertEqual
+  \ [
+  \   {
+  \     'lnum': 3,
+  \     'end_lnum': 5,
+  \     'col': 3,
+  \     'end_col': 4,
+  \     'type': 'I',
+  \     'text': 'Double quote to prevent globbing and word splitting.',
+  \     'code': 'SC2086',
+  \     'detail': 'Double quote to prevent globbing and word splitting.' . "\n\nFor more information:\n  https://www.shellcheck.net/wiki/" . 'SC2086',
+  \   },
+  \   {
+  \     'lnum': 13,
+  \     'end_lnum': 13,
+  \     'col': 17,
+  \     'end_col': 27,
+  \     'type': 'I',
+  \     'text': '$/${} is unnecessary on arithmetic variables.',
+  \     'code': 'SC2004',
+  \     'detail': '$/${} is unnecessary on arithmetic variables.' . "\n\nFor more information:\n  https://www.shellcheck.net/wiki/" . 'SC2004',
+  \   }
+  \ ],
+  \ ale#handlers#shellcheck#Handle(bufnr(''), [0, 7, 0], [
+  \   '{
+  \     "comments": [
+  \       {
+  \         "file": "-",
+  \         "line": 3,
+  \         "endLine": 5,
+  \         "column": 3,
+  \         "endColumn": 5,
+  \         "level": "info",
+  \         "code": 2086,
+  \         "message": "Double quote to prevent globbing and word splitting.",
+  \         "fix": null
+  \       },
+  \       {
+  \         "file": "-",
+  \         "line": 13,
+  \         "endLine": 13,
+  \         "column": 17,
+  \         "endColumn": 28,
+  \         "level": "style",
+  \         "code": 2004,
+  \         "message": "$/${} is unnecessary on arithmetic variables.",
+  \         "fix": null
+  \       }
+  \     ]
+  \   }'
+  \ ])