Merge pull request #2875 from bhcleek/lint/golangci-lint-is-awful

lint: revert recent golangci-lint error handling changes

CHANGELOG.md

@@ -76,9 +76,6 @@ IMPROVEMENTS:
   the cursor.
   [[GH-2822]](https://github.com/fatih/vim-go/pull/2822)
   [[GH-2839]](https://github.com/fatih/vim-go/pull/2839)
-* Parse compiler errors that prevent golangci-lint linters from running more
-  usefully.
-  [[GH-2835]](https://github.com/fatih/vim-go/pull/2835)
 * Clarify documentation for terminal options.
   [GH-2843]](https://github.com/fatih/vim-go/pull/2843)
 

autoload/go/lint.vim

@@ -442,40 +442,14 @@ endfunction
 
 function! s:errorformat(metalinter) abort
   if a:metalinter == 'golangci-lint'
-    let l:efm = ''
-
-    " Golangci-lint can several formats, all of which seem to be undocumented.
-    " Based on trial and error, these error format strings seem to catch all
-    " the relevant combinations.
-
-    " When the actual locations and error message is wrapped in parentheses
-    " within brackets (there is usually duplicated location information in
-    " this form). This usually happens when the linter can't do its job
-    " because of compiler or AST problems.
-    let l:efm .= 'level=%tarning\ msg="%.%#(%f:%l:%c:\ %m)\ %.%#]"'
-    let l:efm .= ',level=%trror\ msg="%.%#(%f:%l:%c:\ %m)\ %.%#]"'
-
-    " When the actual locations and error message are not wrapped in
-    " in bracked without the inner parenthetical. This usually happens when
-    " the linter can't do its job because of compiler or AST problems.
-    let l:efm .= ',level=%tarning\ msg="%.%#:\ [%f:%l:%c:\ %m]"'
-    let l:efm .= ',level=%trror\ msg="%.%#:\ [%f:%l:%c:\ %m]"'
-
-    " When the file location is not provided. The usually happens when the
-    " linter can't do its job because of some other problem.
-    let l:efm .= ',level=%tarning\ msg="%m"'
-    let l:efm .= ',level=%trror\ msg="%m"'
-
-    " when the linter was able to compile and/or create the AST, and the
-    " linter found some problems.
-    let l:efm .= ',%f:%l:%c:\ %m'
-    let l:efm .= ',%f:%l\ %m'
+    " Golangci-lint can output the following:
+    "   <file>:<line>:<column>: <message> (<linter>)
+    " This can be defined by the following errorformat:
+    return 'level=%tarning\ msg="%m:\ [%f:%l:%c:\ %.%#]",level=%tarning\ msg="%m",level=%trror\ msg="%m:\ [%f:%l:%c:\ %.%#]",level=%trror\ msg="%m",%f:%l:%c:\ %m,%f:%l\ %m'
   elseif a:metalinter == 'gopls'
-    let l:efm = '%f:%l:%c:%t:\ %m'
-    let l:efm .= ',%f:%l:%c::\ %m'
+    return '%f:%l:%c:%t:\ %m,%f:%l:%c::\ %m'
   endif
 
-  return l:efm
 endfunction
 
 function! s:preserveerrors(autosave, listtype) abort

autoload/go/lint_test.vim

@@ -17,7 +17,7 @@ func! s:gometa(metalinter) abort
         \ ]
     if a:metalinter == 'golangci-lint'
       let expected = [
-            \ {'lnum': 5, 'bufnr': bufnr('%')+4, 'col': 1, 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'pattern': '', 'text': 'exported function `MissingFooDoc` should have comment or be unexported (golint)'}
+            \ {'lnum': 5, 'bufnr': bufnr('%')+5, 'col': 1, 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'pattern': '', 'text': 'exported function `MissingFooDoc` should have comment or be unexported (golint)'}
           \ ]
     endif
 
@@ -42,20 +42,21 @@ func! s:gometa(metalinter) abort
   endtry
 endfunc
 
-func! Test_GometaGolangciLint_problems() abort
-  call s:gometa_problems('golangci-lint')
+func! Test_GometaGolangciLint_shadow() abort
+  call s:gometa_shadow('golangci-lint')
 endfunc
 
-func! s:gometa_problems(metalinter) abort
+func! s:gometa_shadow(metalinter) abort
   let RestoreGOPATH = go#util#SetEnv('GOPATH', fnamemodify(getcwd(), ':p') . 'test-fixtures/lint')
-  silent exe 'e ' . $GOPATH . '/src/lint/golangci-lint/problems/problems.go'
+  silent exe 'e ' . $GOPATH . '/src/lint/golangci-lint/problems/shadow/problems.go'
 
   try
     let g:go_metalinter_command = a:metalinter
     let expected = [
-          \ {'lnum': 3, 'bufnr': bufnr('%'), 'col': 8, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': 'w', 'module': '', 'text': 'import \"/quux\": cannot import absolute path'},
-          \ {'lnum': 3, 'bufnr': bufnr('%'), 'col': 8, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': 'e', 'module': '', 'text': 'import \"/quux\": cannot import absolute path'},
+          \ {'lnum': 4, 'bufnr': bufnr('%'), 'col': 7, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': 'w', 'module': '', 'text': '[runner] Can''t run linter golint: golint: analysis skipped: errors in package'},
+          \ {'lnum': 4, 'bufnr': bufnr('%'), 'col': 7, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': 'e', 'module': '', 'text': 'Running error: golint: analysis skipped: errors in package'}
         \ ]
+
     " clear the quickfix lists
     call setqflist([], 'r')
 
@@ -129,19 +130,125 @@ func! s:gometaautosave(metalinter, withList) abort
   endtry
 endfunc
 
-func! Test_GometaAutoSaveGolangciLint_problems() abort
-  call s:gometaautosave_problems('golangci-lint')
+func! Test_GometaGolangciLint_importabs() abort
+  call s:gometa_importabs('golangci-lint')
+endfunc
+
+func! s:gometa_importabs(metalinter) abort
+  let RestoreGOPATH = go#util#SetEnv('GOPATH', fnamemodify(getcwd(), ':p') . 'test-fixtures/lint')
+  silent exe 'e ' . $GOPATH . '/src/lint/golangci-lint/problems/importabs/problems.go'
+
+  try
+    let g:go_metalinter_command = a:metalinter
+    let expected = [
+          \ {'lnum': 3, 'bufnr': bufnr('%'), 'col': 8, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': 'w', 'module': '', 'text': '[runner] Can''t run linter golint: golint: analysis skipped: errors in package'},
+          \ {'lnum': 3, 'bufnr': bufnr('%'), 'col': 8, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': 'e', 'module': '', 'text': 'Running error: golint: analysis skipped: errors in package'},
+        \ ]
+    " clear the quickfix lists
+    call setqflist([], 'r')
+
+    let g:go_metalinter_enabled = ['golint']
+
+    call go#lint#Gometa(0, 0)
+
+    let actual = getqflist()
+    let start = reltime()
+    while len(actual) == 0 && reltimefloat(reltime(start)) < 10
+      sleep 100m
+      let actual = getqflist()
+    endwhile
+
+    call gotest#assert_quickfix(actual, expected)
+  finally
+      call call(RestoreGOPATH, [])
+      unlet g:go_metalinter_enabled
+  endtry
+endfunc
+
+func! Test_GometaAutoSaveGolangciLint_importabs() abort
+  call s:gometaautosave_importabs('golangci-lint')
+endfunc
+
+func! s:gometaautosave_importabs(metalinter) abort
+  let RestoreGOPATH = go#util#SetEnv('GOPATH', fnameescape(fnamemodify(getcwd(), ':p')) . 'test-fixtures/lint')
+  silent exe 'e ' . $GOPATH . '/src/lint/golangci-lint/problems/importabs/ok.go'
+
+  try
+    let g:go_metalinter_command = a:metalinter
+    let expected = [
+          \ {'lnum': 3, 'bufnr': bufnr('%')+1, 'col': 8, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': 'w', 'module': '', 'text': '[runner] Can''t run linter golint: golint: analysis skipped: errors in package'},
+          \ {'lnum': 3, 'bufnr': bufnr('%')+1, 'col': 8, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': 'e', 'module': '', 'text': 'Running error: golint: analysis skipped: errors in package'}
+        \ ]
+
+    " clear the location lists
+    call setloclist(0, [], 'r')
+
+    let g:go_metalinter_autosave_enabled = ['golint']
+
+    call go#lint#Gometa(0, 1)
+
+    let actual = getloclist(0)
+    let start = reltime()
+    while len(actual) == 0 && reltimefloat(reltime(start)) < 10
+      sleep 100m
+      let actual = getloclist(0)
+    endwhile
+
+    call gotest#assert_quickfix(actual, expected)
+  finally
+    call call(RestoreGOPATH, [])
+    unlet g:go_metalinter_autosave_enabled
+  endtry
+endfunc
+
+func! Test_GometaGolangciLint_multiple() abort
+  call s:gometa_multiple('golangci-lint')
+endfunc
+
+func! s:gometa_multiple(metalinter) abort
+  let RestoreGOPATH = go#util#SetEnv('GOPATH', fnamemodify(getcwd(), ':p') . 'test-fixtures/lint')
+  silent exe 'e ' . $GOPATH . '/src/lint/golangci-lint/problems/multiple/problems.go'
+
+  try
+    let g:go_metalinter_command = a:metalinter
+    let expected = [
+          \ {'lnum': 8, 'bufnr': bufnr('%'), 'col': 7, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': 'w', 'module': '', 'text': '[runner] Can''t run linter golint: golint: analysis skipped: errors in package'},
+          \ {'lnum': 8, 'bufnr': bufnr('%'), 'col': 7, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': 'e', 'module': '', 'text': 'Running error: golint: analysis skipped: errors in package'},
+        \ ]
+    " clear the quickfix lists
+    call setqflist([], 'r')
+
+    let g:go_metalinter_enabled = ['golint']
+
+    call go#lint#Gometa(0, 0)
+
+    let actual = getqflist()
+    let start = reltime()
+    while len(actual) == 0 && reltimefloat(reltime(start)) < 10
+      sleep 100m
+      let actual = getqflist()
+    endwhile
+
+    call gotest#assert_quickfix(actual, expected)
+  finally
+      call call(RestoreGOPATH, [])
+      unlet g:go_metalinter_enabled
+  endtry
+endfunc
+
+func! Test_GometaAutoSaveGolangciLint_multiple() abort
+  call s:gometaautosave_multiple('golangci-lint')
 endfunc
 
-func! s:gometaautosave_problems(metalinter) abort
+func! s:gometaautosave_multiple(metalinter) abort
   let RestoreGOPATH = go#util#SetEnv('GOPATH', fnameescape(fnamemodify(getcwd(), ':p')) . 'test-fixtures/lint')
-  silent exe 'e ' . $GOPATH . '/src/lint/golangci-lint/problems/ok.go'
+  silent exe 'e ' . $GOPATH . '/src/lint/golangci-lint/problems/multiple/problems.go'
 
   try
     let g:go_metalinter_command = a:metalinter
     let expected = [
-          \ {'lnum': 3, 'bufnr': bufnr('%')+1, 'col': 8, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': 'w', 'module': '', 'text': 'import \"/quux\": cannot import absolute path'},
-          \ {'lnum': 3, 'bufnr': bufnr('%')+1, 'col': 8, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': 'e', 'module': '', 'text': 'import \"/quux\": cannot import absolute path'},
+          \ {'lnum': 8, 'bufnr': bufnr('%'), 'col': 7, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': 'w', 'module': '', 'text': '[runner] Can''t run linter golint: golint: analysis skipped: errors in package'},
+          \ {'lnum': 8, 'bufnr': bufnr('%'), 'col': 7, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': 'e', 'module': '', 'text': 'Running error: golint: analysis skipped: errors in package'},
         \ ]
 
     " clear the location lists
@@ -230,7 +337,7 @@ func! Test_Lint_GOPATH() abort
 
   let expected = [
           \ {'lnum': 5, 'bufnr': bufnr('%'), 'col': 1, 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'pattern': '', 'text': 'exported function MissingDoc should have comment or be unexported'},
-          \ {'lnum': 5, 'bufnr': bufnr('%')+5, 'col': 1, 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'pattern': '', 'text': 'exported function AlsoMissingDoc should have comment or be unexported'}
+          \ {'lnum': 5, 'bufnr': bufnr('%')+7, 'col': 1, 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'pattern': '', 'text': 'exported function AlsoMissingDoc should have comment or be unexported'}
       \ ]
 
   let winnr = winnr()
@@ -258,7 +365,7 @@ func! Test_Lint_NullModule() abort
 
   let expected = [
           \ {'lnum': 5, 'bufnr': bufnr('%'), 'col': 1, 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'pattern': '', 'text': 'exported function MissingDoc should have comment or be unexported'},
-          \ {'lnum': 5, 'bufnr': bufnr('%')+5, 'col': 1, 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'pattern': '', 'text': 'exported function AlsoMissingDoc should have comment or be unexported'}
+          \ {'lnum': 5, 'bufnr': bufnr('%')+7, 'col': 1, 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'pattern': '', 'text': 'exported function AlsoMissingDoc should have comment or be unexported'}
       \ ]
 
   let winnr = winnr()

autoload/go/test-fixtures/lint/src/lint/golangci-lint/problems/multiple/problems.go

@@ -0,0 +1,9 @@
+package problems
+
+import (
+	"time"
+)
+
+func mySleep(time int) {
+	time.Sleep(500 * time.Millisecond)
+}

autoload/go/test-fixtures/lint/src/lint/golangci-lint/problems/shadow/problems.go

@@ -0,0 +1,5 @@
+package problems
+
+func mySleep(time int) {
+	time.Sleep(500 * time.Millisecond)
+}

autoload/gotest.vim

@@ -122,26 +122,29 @@ endfun
 func! gotest#assert_quickfix(got, want) abort
   call assert_equal(len(a:want), len(a:got), "number of errors")
   if len(a:want) != len(a:got)
-    call assert_equal(a:want, a:got)
-    return
+    return assert_equal(a:want, a:got)
   endif
 
+  let l:retval = 0
   let i = 0
+
   while i < len(a:want)
     let want_item = a:want[i]
     let got_item = a:got[i]
     let i += 1
 
-    call assert_equal(want_item.bufnr, got_item.bufnr, "bufnr")
-    call assert_equal(want_item.lnum, got_item.lnum, "lnum")
-    call assert_equal(want_item.col, got_item.col, "col")
-    call assert_equal(want_item.vcol, got_item.vcol, "vcol")
-    call assert_equal(want_item.nr, got_item.nr, "nr")
-    call assert_equal(want_item.pattern, got_item.pattern, "pattern")
-    call assert_equal(want_item.text, got_item.text, "text")
-    call assert_equal(want_item.type, got_item.type, "type")
-    call assert_equal(want_item.valid, got_item.valid, "valid")
+    let l:retval = assert_equal(want_item.bufnr, got_item.bufnr, "bufnr") || l:retval
+    let l:retval = assert_equal(want_item.lnum, got_item.lnum, "lnum") || l:retval
+    let l:retval = assert_equal(want_item.col, got_item.col, "col") || l:retval
+    let l:retval = assert_equal(want_item.vcol, got_item.vcol, "vcol") || l:retval
+    let l:retval = assert_equal(want_item.nr, got_item.nr, "nr") || l:retval
+    let l:retval = assert_equal(want_item.pattern, got_item.pattern, "pattern") || l:retval
+    let l:retval = assert_equal(want_item.text, got_item.text, "text") || l:retval
+    let l:retval = assert_equal(want_item.type, got_item.type, "type") || l:retval
+    let l:retval = assert_equal(want_item.valid, got_item.valid, "valid") || l:retval
   endwhile
+
+  return l:retval
 endfunc
 
 " restore Vi compatibility settings