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

lint: make staticcheck the default metalinter #3126

Merged
merged 1 commit into from
Jan 3, 2021

Conversation

bhcleek
Copy link
Collaborator

@bhcleek bhcleek commented Jan 3, 2021

No description provided.

@bhcleek bhcleek added this to the vim-go 1.25 milestone Jan 3, 2021
Copy link

@mdlayher mdlayher left a comment

Choose a reason for hiding this comment

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

LGTM!

@bhcleek bhcleek merged commit 706c73b into fatih:master Jan 3, 2021
@bhcleek bhcleek deleted the lint/default/staticcheck branch January 3, 2021 20:51
@leighmcculloch
Copy link

👋🏻 What was the motivation for changing the default meta-linter? I don't have an issue with the decision but I was hoping to learn why because I'm a golangci-lint user and want to know why folks are switching for if I should too. The PR description doesn't say why or link to related discussion.

@bhcleek
Copy link
Collaborator Author

bhcleek commented Jan 4, 2021

@leighmcculloch Thanks for asking.

Support for golangci-lint was added to vim-go a couple of years ago. At the time gometalinter was deprecated and had been scheduled to be archived. Staticcheck didn't seem like a good fit for vim-go at the time. So at that point, golangci-lint seemed like the only path forward for vim-go's :GoMetaLinter command.

Golangci-lint was problematic pretty much right away. Its messages are inconsistently structured, it behaves differently depending on where its working directory is and whether it's given absolute or relative paths, and it's generally been difficult to work with. In some cases, it's nearly impossible to handle its errors correctly, because its error message formats are so terribly inconsistent depending on which linters' messages are provided.

Staticcheck, on the other hand, structures its errors messages consistently which makes it very easy to integrate with Vim, it has straightforward cli options, and is easily customizable using config files. After reevaluating staticcheck due to frustrations with golangci-lint, I became convinced that it was the right path forward, so I added support for staticcheck a few months ago.

I'm sure not everyone will agree with this decision. You are free to use golangci-lint for the foreseeable future, though I would like to consider fully removing support for it 2 or 3 vim-go releases from now.

@leighmcculloch
Copy link

Thanks for the thorough knowledge share, it was very insightful, and makes a lot of sense.

@reedobrien
Copy link

@bhcleek

First, thanks for maintaining this plugin!

AFAICT this change causes whatever s:default_metalinter is to always be called regardless of what is set in let g:go_metlinter_command = "golangci-lint"

That is if I change s:default_metalinter that is the linter that is run. When I set it to gopls it reports metalinter dispatched. If I set it to golangci-lint or staticcheck it reports the correct one dispatched. Again regardless of what is set in .vimrc for let g:go_metlinter_command = "...".

Also with staticcheck it seems to work correctly when I run :GoMetalinter but when it runs on save -- via let g:go_metalinter_autosave = 1 it fails as it appears to call staticcheck with a single file rather than with the package as the target.
I can reproduce this by calling staticcheck on a file vs on the directory or package. This worked with staticcheck as an optional linter in let g:go_metalinter_enabled = [..., 'staticcheck', '...'].

These may be separate issues. Let me know if you would like to create an issue or two.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants