Enabling golangci-lint makes flycheck useless for syntax errors

[tko]

Description

Enabling golangci-lint makes flycheck useless for syntax errors. Error list shows only a small subset of errors, jumping to next/previous error doesn't do anything, focusing and moving in flycheck-buffer fails to open the correct file.

flychecker-verify-setup

First checker to run:

  golangci-lint
    - may enable: yes
    - executable: Found at /usr/local/bin/golangci-lint

Checkers that are compatible with this mode, but will not run until properly configured:

  go-gofmt (disabled)
    - may enable:    Automatically disabled!
    - executable:    Found at /usr/local/bin/gofmt
    - next checkers: go-golint, go-vet, go-build, go-test, go-errcheck, go-unconvert, go-staticcheck

  go-golint (disabled)
    - may enable:    Automatically disabled!
    - executable:    Found at /Users/tkomulainen/Projects/go/bin/golint
    - next checkers: go-vet, go-build, go-test, go-errcheck, go-unconvert

  go-vet (disabled)
    - may enable:    Automatically disabled!
    - executable:    Found at /usr/local/bin/go
    - go tool vet:   present
    - next checkers: go-build, go-test, go-errcheck, go-unconvert, go-staticcheck

  go-build (disabled)
    - may enable:    Automatically disabled!
    - predicate:     t
    - executable:    Found at /usr/local/bin/go
    - next checkers: go-errcheck, go-unconvert, go-staticcheck

  go-test (disabled)
    - may enable:    Automatically disabled!
    - predicate:     nil
    - executable:    Found at /usr/local/bin/go
    - next checkers: go-errcheck, go-unconvert, go-staticcheck

  go-errcheck (disabled)
    - may enable:    Automatically disabled!
    - predicate:     t
    - executable:    Not found
    - next checkers: go-unconvert, go-staticcheck

  go-unconvert (disabled)
    - may enable: Automatically disabled!
    - predicate:  t
    - executable: Not found

  go-staticcheck (disabled)
    - may enable: Automatically disabled!
    - executable: Found at /Users/tkomulainen/Projects/go/bin/staticcheck

Reproduction guide 🪲

• Enable layer (go :variables go-use-golangci-lint t)
• Start Emacs
• Edit /tmp/x/x.go:

package x

type X struct{}

func lint() {
	x := X{NoSuchField: 42}
}

Observed behaviour: 👀 💔
The flycheck buffer shows only a very small subset of errors and SPC e n and SPC e p fails to jump to next/previous error as it normally would.

Focusing and moving in the flycheck buffer tries to jump to a file, but opens an empty buffer instead and complains in minibuffer with

flycheck-buffer: :working-directory /tmp/x/level=error msg="Running error: bodyclose: failed prerequisites: [buildssa@_/tmp/x: analysis skipped: errors in package: [/tmp/x/ of syntax checker golangci-lint does not exist

Expected behaviour: ❤️ 😄
Flycheck buffer shows all errors, SPC e n and SPC e p jumps to next/previous error.

System Info 💻

• OS: darwin
• Emacs: 26.3
• Spacemacs: 0.300.0
• Spacemacs branch: develop (rev. 2d7480f)
• Graphic display: t
• Distribution: spacemacs
• Editing style: vim
• Completion: helm
• Layers:

(helm emacs-lisp multiple-cursors treemacs
      (auto-completion :variables auto-completion-enable-help-tooltip t)
      colors csv dash docker git
      (go :variables go-backend 'go-mode go-format-before-save t go-use-golangci-lint t gofmt-command "goimports")
      helpful html imenu-list
      (javascript :variables javascript-backend 'lsp)
      lsp markdown nginx
      (org :variables org-enable-reveal-js-support t org-enable-sticky-header t org-enable-verb-support t)
      (osx :variables osx-command-as 'hyper osx-right-command-as 'super osx-option-as 'none osx-right-option-as 'meta)
      puppet python react restclient semantic shell-scripts syntax-checking typescript version-control yaml)

• System configuration features: NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS MODULES THREADS LCMS2

[tko]

Reading through the default flycheck/go checkers I think go-build should run before golangci-lint; approximately go-build :next-checkers (warning . golangci-lint) but I've no idea how to reconfigure flycheck that way.

[smile13241324]

This looks like a problem with flycheck-golangci-lint.el this application is responsible for running the external linter and parsing the results. However this is not properly done, i.e. I am getting the following message in the flycheck error list:

unknown field NoSuchField in struct literal ... test.go:14:2 x declared but not used]

This is actually two error messages which should have been separated for flycheck. This seems not have happened, in addition the error messages contain information where it happened, also this information seems to be lost/not parsed causing flycheck to report all issues at the top of the file.

I think this is an upstream issue.

[tko]

Yes, the linter isn't reporting syntax errors nicely, which is relatively understandable as it is not the compiler and expects syntactically valid code probably. The "standard" flycheck checkers seem to avoid this issue with :next-checkers chaining. However instead adding flycheck-golangci-lint into the mix the spacemacs config disables the standard linters completely

I believe golangci-lint does replace go-gofmt, go-golint, go-vet, go-errcheck but not go-build or go-test

[smile13241324]

This would mean we have actually two issues here:

• we are disabling too much checkers --> I can fix this, just need to double check the golangci-lint docs.
• golangci-lint not formatting the results properly, here I don't think that this is caused by a corrupted file. I mean the issues reported were classic linting issues, not used variable and wrong property in struct, and should have been reported in a way that flycheck could show them in the line they have happened. As it is right now it would still be relatively hard to read, I mean having all your issues at one line in the file. --> This is still either a missing configuration on our side, or an upstream issue. When I double check the docs I can see if we are missing some mandatory config files.

I will try to have a look into this in the next view days. Hope that helps.

[smile13241324]

The issue comes from error messages reported by the linter which cannot be understood by the flycheck plugin. I have found this issue which is the root of our issue. Basically the devs hope to have it solved in the latest version and request to receive issues for these kind of errors. However I did run 1.27.0 and the issue persists with your test case.

I have now changed the code to first run go-build, then golangci-lint via checker chaining. This makes sure that though golangci-lint fails you still see the errors from go-build. The expectation is that these kind of errors are less likely to occur when go-build can run successfully.

Here is my command output, note that the lines should only contain the file + line number:

golangci-lint run --print-issued-lines=false --out-format=line-number

WARN [runner] Can't run linter goanalysis_metalinter: assign: failed prerequisites: inspect@example 
WARN [runner] Can't run linter unused: buildssa: analysis skipped: errors in package: [/home/smile13241324/goWorkspace/src/example/example.go:13:9: unknown field NoSuchField in struct literal /home/smile13241324/goWorkspace/src/example/example.go:13:2: x declared but not used] 
ERRO Running error: buildssa: analysis skipped: errors in package: [/home/smile13241324/goWorkspace/src/example/example.go:13:9: unknown field NoSuchField in struct literal /home/smile13241324/goWorkspace/src/example/example.go:13:2: x declared but not used]

I have also fixed the missing config for the respective flycheck plugin. Here we disabled the other checkers, however we did not activate all the others from golangci-lint. I have now changed the config to use all available linters and to also run the linter for test files, I assume this is what go-test would have done.

[smile13241324]

@tko with these changes I think the support for golangci-lint should be fixed. When it fails you still see the errors from go-build. When these are fixed golangci-lint normally takes over as usual.

In addition golangci-lint will now run all the previously disabled checkers, which it did not before as the linter was not properly configured. For example go-fmt did not run by default.

Would you mind opening an upstream bug with your test case and the two warnings from the linter? Hopefully this can be fixed in the next releases.

[tko]

@smile13241324 thanks, I'll see how things work.

I am noticing when editing something_test.go there's File mode specification error: (user-error Can’t use syntax checker go-build in this buffer) -- I think this has to do with go-build being valid only for non-test files but it's forcefully enabled, and conversely go-test is valid only for test files though it's not forced on.

[tko]

I'm finding (setq flycheck-golangci-lint-enable-all t) to be relatively harmful as it doesn't work at all if you have .golangi.yml with disable-all in it, the two options can't be used at the same time.

While enable-all may be a relatively good default in the "all features turned on", setting it here makes it relatively difficult to turn off again as, for example (go :variables flycheck-golangci-lint-enable-all t) has no effect. It also seems enable-all is considered deprecated though I'm not entirely sure if it also covers command line params:

# please, do not use enable-all: it's deprecated and will be removed soon.

https://github.com/golangci/golangci-lint/blob/3c46e160deb6b1261979e78866427cb3f726ee1e/.golangci.yml#L64

[smile13241324]

@tko thanks for the feedback, I had hopped I could keep this relatively simple, but as it looks like I have made the problem worse ... let's see if I can remove the hassle here.

• Failing activation of go build for test files --> lets check the buffer name and select go-build only if the file ends with .go, if the file ends with test.go select go-test. If non match just activate golangci-lint.
• As to the config variables, you are right, lets remove them again. However I will add a section to the readme how to set these. Also mention that the prefered way is to have a config file in your project.

I will work on this the next days.

[smile13241324]

I think I am finished. In my tests I was able to successfully open an example.go file as well as a example_test.go file. For the first go-build was activated for the second go-test. Both run golangci-lint afterwards.

I have also removed the static flycheck-golangci-lint variables. Instead I have added more information to the README to make sure that these variables require some kind of config before the linter can be properly used.

Last but not least I have tried to explain the current situation as to avoid confusing too much users seeing these error messages at the top of their files when golangci-lint is used and basic errors are included in the file.

@tko would mind having another look at this feature?

[tko]

@smile13241324 looking much better.

On syntax errors I'm seeing the same error message in triplicate or so, one from go-build (or go-test, depending) and others from golangci-lint. I could get rid of it with slightly different next checker invocation:

(flycheck-add-next-checker 'go-build '(warning . golangci-lint) t)
(flycheck-add-next-checker 'go-test '(warning . golangci-lint) t)

Also I think the cond could be written to take advantage of the conditions built into the go-build/go-test checkers themselves instead of duplicating it partially:

(cond ((flycheck-may-use-checker 'go-test) (flycheck-select-checker 'go-test))
      ((flycheck-may-use-checker 'go-build) (flycheck-select-checker 'go-build)))

[smile13241324]

Nice 👍 , I am really learning something here :).
I have done the suggested changes, would you mind having another go @tko?

[tko]

@smile13241324 It's working quite alright, thanks. I think this issue can be closed unless you want to tag it with fixed-in-develop instead.

I've noticed that when creating new files (SPC f f whatever.go), either normal.go or _test.go, the go-build / go-test linter isn't getting enabled (shows up as "may run: nil" in flycheck-verify-setup) ... I don't know if it's specific golangci-lint and I'm too lazy to try find out, either way golangci-lint is mostly working and other issues can be dealt with separately.

[smile13241324]

@tko great, I will close it then.