Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add gometalinter #312

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 27 additions & 22 deletions ale_linters/go/gobuild.vim
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
" inspired by work from dzhou121 <dzhou121@gmail.com>

function! ale_linters#go#gobuild#GoEnv(buffer) abort
if exists('s:go_env')
if exists('g:ale_linters#go#gobuild#go_env')
return ''
endif

Expand All @@ -15,15 +15,15 @@ let s:SplitChar = has('unix') ? ':' : ':'

" get a list of all source directories from $GOPATH and $GOROOT
function! s:SrcDirs() abort
let l:paths = split(s:go_env.GOPATH, s:SplitChar)
call add(l:paths, s:go_env.GOROOT)
let l:paths = split(g:ale_linters#go#gobuild#go_env.GOPATH, s:SplitChar)
call add(l:paths, g:ale_linters#go#gobuild#go_env.GOROOT)

return l:paths
endfunction

" figure out from a directory like `/home/user/go/src/some/package` that the
" import for that path is simply `some/package`
function! s:PackageImportPath(buffer) abort
function! ale_linters#go#gobuild#PackageImportPath(buffer) abort
let l:bufname = resolve(bufname(a:buffer))
let l:pkgdir = fnamemodify(l:bufname, ':p:h')

Expand All @@ -41,13 +41,13 @@ endfunction
" get the package info data structure using `go list`
function! ale_linters#go#gobuild#GoList(buffer, goenv_output) abort
if !empty(a:goenv_output)
let s:go_env = {
let g:ale_linters#go#gobuild#go_env = {
\ 'GOPATH': a:goenv_output[0],
\ 'GOROOT': a:goenv_output[1],
\}
endif

return 'go list -json ' . shellescape(s:PackageImportPath(a:buffer))
return 'go list -json ' . shellescape(ale_linters#go#gobuild#PackageImportPath(a:buffer))
endfunction

let s:filekeys = [
Expand All @@ -68,7 +68,7 @@ let s:filekeys = [

" get the go and test go files from the package
" will return empty list if the package has any cgo or other invalid files
function! s:PkgFiles(pkginfo) abort
function! ale_linters#go#gobuild#PkgFiles(pkginfo) abort
let l:files = []

for l:key in s:filekeys
Expand All @@ -83,7 +83,7 @@ endfunction

function! ale_linters#go#gobuild#CopyFiles(buffer, golist_output) abort
let l:tempdir = tempname()
let l:temppkgdir = l:tempdir . '/src/' . s:PackageImportPath(a:buffer)
let l:temppkgdir = l:tempdir . '/src/' . ale_linters#go#gobuild#PackageImportPath(a:buffer)
call mkdir(l:temppkgdir, 'p', 0700)

if empty(a:golist_output)
Expand All @@ -94,15 +94,15 @@ function! ale_linters#go#gobuild#CopyFiles(buffer, golist_output) abort
let l:pkginfo = json_decode(join(a:golist_output, "\n"))

" get all files for the package
let l:files = s:PkgFiles(l:pkginfo)
let l:files = ale_linters#go#gobuild#PkgFiles(l:pkginfo)

" copy the files to a temp directory with $GOPATH structure
return 'cp ' . join(l:files, ' ') . ' ' . shellescape(l:temppkgdir) . ' && echo ' . shellescape(l:tempdir)
endfunction

function! ale_linters#go#gobuild#GetCommand(buffer, copy_output) abort
function! ale_linters#go#gobuild#WriteBuffers(buffer, copy_output) abort
let l:tempdir = a:copy_output[0]
let l:importpath = s:PackageImportPath(a:buffer)
let l:importpath = ale_linters#go#gobuild#PackageImportPath(a:buffer)

" write the a:buffer and any modified buffers from the package to the tempdir
for l:bufnum in range(1, bufnr('$'))
Expand All @@ -119,7 +119,7 @@ function! ale_linters#go#gobuild#GetCommand(buffer, copy_output) abort
" only consider buffers other than a:buffer if they have the same import
" path as a:buffer and are modified
if l:bufnum != a:buffer
if s:PackageImportPath(l:bufnum) !=# l:importpath
if ale_linters#go#gobuild#PackageImportPath(l:bufnum) !=# l:importpath
continue
endif

Expand All @@ -128,30 +128,36 @@ function! ale_linters#go#gobuild#GetCommand(buffer, copy_output) abort
endif
endif

call writefile(getbufline(l:bufnum, 1, '$'), l:tempdir . '/src/' . s:PkgFile(l:bufnum))
call writefile(getbufline(l:bufnum, 1, '$'), l:tempdir . '/src/' . ale_linters#go#gobuild#PkgFile(l:bufnum))
endfor

return 'echo ' . shellescape(l:tempdir)
endfunction

function! ale_linters#go#gobuild#GetCommand(buffer, copy_output) abort
let l:tempdir = a:copy_output[0]
let l:importpath = ale_linters#go#gobuild#PackageImportPath(a:buffer)
let l:gopaths = [ l:tempdir ]
call extend(l:gopaths, split(s:go_env.GOPATH, s:SplitChar))
call extend(l:gopaths, split(g:ale_linters#go#gobuild#go_env.GOPATH, s:SplitChar))

return 'GOPATH=' . shellescape(join(l:gopaths, s:SplitChar)) . ' go test -c -o /dev/null ' . shellescape(l:importpath)
endfunction

function! s:PkgFile(buffer) abort
function! ale_linters#go#gobuild#PkgFile(buffer) abort
let l:bufname = resolve(bufname(a:buffer))
let l:importpath = s:PackageImportPath(a:buffer)
let l:importpath = ale_linters#go#gobuild#PackageImportPath(a:buffer)
let l:fname = fnamemodify(l:bufname, ':t')

return l:importpath . '/' . l:fname
endfunction

function! s:FindBuffer(file) abort
function! ale_linters#go#gobuild#FindBuffer(file) abort
for l:buffer in range(1, bufnr('$'))
if !buflisted(l:buffer)
continue
endif

let l:pkgfile = s:PkgFile(l:buffer)
let l:pkgfile = ale_linters#go#gobuild#PkgFile(l:buffer)

if a:file =~ '/' . l:pkgfile . '$'
return l:buffer
Expand All @@ -164,8 +170,6 @@ endfunction
let s:path_pattern = '[a-zA-Z]\?\\\?:\?[[:alnum:]/\.\-_]\+'
let s:handler_pattern = '^\(' . s:path_pattern . '\):\(\d\+\):\?\(\d\+\)\?: \(.\+\)$'

let s:multibuffer = 0

function! ale_linters#go#gobuild#Handler(buffer, lines) abort
let l:output = []

Expand All @@ -176,13 +180,13 @@ function! ale_linters#go#gobuild#Handler(buffer, lines) abort
continue
endif

let l:buffer = s:FindBuffer(l:match[1])
let l:buffer = ale_linters#go#gobuild#FindBuffer(l:match[1])

if l:buffer == -1
continue
endif

if !s:multibuffer && l:buffer != a:buffer
if !get(g:, 'ale_experimental_multibuffer', 0) && l:buffer != a:buffer
" strip lines from other buffers
continue
endif
Expand All @@ -208,6 +212,7 @@ call ale#linter#Define('go', {
\ {'callback': 'ale_linters#go#gobuild#GoEnv', 'output_stream': 'stdout'},
\ {'callback': 'ale_linters#go#gobuild#GoList', 'output_stream': 'stdout'},
\ {'callback': 'ale_linters#go#gobuild#CopyFiles', 'output_stream': 'stdout'},
\ {'callback': 'ale_linters#go#gobuild#WriteBuffers', 'output_stream': 'stdout'},
\ {'callback': 'ale_linters#go#gobuild#GetCommand', 'output_stream': 'stderr'},
\ ],
\ 'callback': 'ale_linters#go#gobuild#Handler',
Expand Down
70 changes: 70 additions & 0 deletions ale_linters/go/gometalinter.vim
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
let s:SplitChar = has('unix') ? ':' : ':'

let g:ale_linters#go#gometalinter#args = get(g:, 'ale_linters#go#gometalinter#args', [
\ '--fast',
\ '--tests',
\ ])

function! s:Args() abort
return join(map(copy(g:ale_linters#go#gometalinter#args), 'shellescape(v:val)'), ' ')
endfunction

function! ale_linters#go#gometalinter#GetCommand(buffer, copy_output) abort
let l:tempdir = a:copy_output[0]
let l:importpath = ale_linters#go#gobuild#PackageImportPath(a:buffer)
let l:gopaths = [ l:tempdir ]
call extend(l:gopaths, split(g:ale_linters#go#gobuild#go_env.GOPATH, s:SplitChar))

return 'GOPATH=' . shellescape(join(l:gopaths, s:SplitChar)) . ' gometalinter ' . s:Args() . ' ' . shellescape(l:tempdir . '/src/' . l:importpath)
endfunction

let s:path_pattern = '[a-zA-Z]\?\\\?:\?[[:alnum:]/\.\-_]\+'
let s:handler_pattern = '^\(' . s:path_pattern . '\):\(\d\+\):\?\(\d\+\)\?:\?\([a-zA-Z]\+\): \(.\+\)$'

function! ale_linters#go#gometalinter#Handler(buffer, lines) abort
let l:output = []

for l:line in a:lines
let l:match = matchlist(l:line, s:handler_pattern)

if len(l:match) == 0
continue
endif

let l:buffer = ale_linters#go#gobuild#FindBuffer(l:match[1])

if l:buffer == -1
continue
endif

if !get(g:, 'ale_experimental_multibuffer', 0) && l:buffer != a:buffer
" strip lines from other buffers
continue
endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could perhaps start implementing this flag in the function which fixes problems with the loclist items. If the flag isn't on, you could automatically remove items for other buffers in that function.


call add(l:output, {
\ 'bufnr': l:buffer,
\ 'lnum': l:match[2] + 0,
\ 'vcol': 0,
\ 'col': l:match[3] + 0,
\ 'text': l:match[5],
\ 'type': toupper(l:match[4][0]),
\ 'nr': -1,
\})
endfor

return l:output
endfunction

call ale#linter#Define('go', {
\ 'name': 'gometalinter',
\ 'executable': 'gometalinter',
\ 'command_chain': [
\ {'callback': 'ale_linters#go#gobuild#GoEnv', 'output_stream': 'stdout'},
\ {'callback': 'ale_linters#go#gobuild#GoList', 'output_stream': 'stdout'},
\ {'callback': 'ale_linters#go#gobuild#CopyFiles', 'output_stream': 'stdout'},
\ {'callback': 'ale_linters#go#gobuild#WriteBuffers', 'output_stream': 'stdout'},
\ {'callback': 'ale_linters#go#gometalinter#GetCommand', 'output_stream': 'stdout'},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these output_stream options are also the default value. I'll document this eventually, if I haven't already.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be an idea to move functions shared between two linter definitions into the autoload/ale/handlers directory. At the moment, ALE loads the defnitions for a given filetype like so:

execute 'silent! runtime! ale_linters/' . a:filetype . '/*.vim'

If anyone ever changes the way linters are loaded so only the list of enabled linters are loaded, which has tried before, then this code will break when only this linter is enabled for go.

\ ],
\ 'callback': 'ale_linters#go#gometalinter#Handler',
\})
11 changes: 11 additions & 0 deletions autoload/ale/util.vim
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,17 @@ function! ale#util#LocItemCompare(left, right) abort
return 1
endif

" put warnings after errors (for the same line) since the text that shows
" when the cursor is moved will show only the first entry

if a:left['type'] < a:right['type']
return -1
endif

if a:left['type'] > a:right['type']
return 1
endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change will sort the items in the wrong order, which is important for the movement commands, etc. Instead of getting items in column order 1, 2, 3, 4, 5, you can see items in order E3, E4, W1, W2, W5, etc.

You should move these lines down below the column sorting, so it's the third key items are sorted on. Then E will take priority over W if there are both warnings and errors for the same column, if that's the problem. The way the items are sorted, the cursor is supposed to show information for a problem near the cursor. It doesn't matter what the severity is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're going to sort on the error type, you should take issue #149 into account. Eventually, I would like to support error types in some way beyond "warning" and "error". 'E' seems to be the first single character which could appear in lexicographic order, but 'I' comes before 'W' in lexicographic order, and 'W' is more severe than 'I', so that wouldn't be right.


if a:left['col'] < a:right['col']
return -1
endif
Expand Down
18 changes: 10 additions & 8 deletions test/test_loclist_sorting.vader
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
Before:
let g:loclist = [
\ {'lnum': 5, 'col': 5},
\ {'lnum': 5, 'col': 4},
\ {'lnum': 2, 'col': 10},
\ {'lnum': 3, 'col': 2},
\ {'lnum': 5, 'col': 5, 'type': 'W'},
\ {'lnum': 5, 'col': 4, 'type': 'W'},
\ {'lnum': 2, 'col': 10, 'type': 'W'},
\ {'lnum': 3, 'col': 2, 'type': 'W'},
\ {'lnum': 3, 'col': 1, 'type': 'E'},
\]

Execute (Sort loclist with comparison function):
call sort(g:loclist, 'ale#util#LocItemCompare')

Then (loclist item should be sorted):
AssertEqual [
\ {'lnum': 2, 'col': 10},
\ {'lnum': 3, 'col': 2},
\ {'lnum': 5, 'col': 4},
\ {'lnum': 5, 'col': 5},
\ {'lnum': 2, 'col': 10, 'type': 'W'},
\ {'lnum': 3, 'col': 1, 'type': 'E'},
\ {'lnum': 3, 'col': 2, 'type': 'W'},
\ {'lnum': 5, 'col': 4, 'type': 'W'},
\ {'lnum': 5, 'col': 5, 'type': 'W'},
\], g:loclist

After:
Expand Down