lint: parse golangci-lint errors when g:go_metalinter_command is customized

[bhcleek]

Fixes #2707

[bhcleek]

@orlangure I'm trying to decide whether this PR is needed. Can you see if it's still relevant for you on master in light of #2715 ? You should be able to configure your golangci-lint config file as you want and then use let g:go_metalinter_enabled=[] and/or let g:go_metalinter_autosave_enabled = [] respectively.

[flw-cn]

There are still errors. Because I don't want to report some errors detected by golint, such as missing documents (OK, I admit I will supplement these damn documents when appropriate). So I customized g: go_metalinter_command and added --exclude-use-default=true to golangci-lint.

Then, vim-go does not recognize the errors output by golangci-lint.

Here is my config:

let g:go_metalinter_autosave = 1
let g:go_metalinter_autosave_enabled = []
let g:go_metalinter_enabled = []
let g:go_metalinter_command = "golangci-lint run --exclude-use-default=true"
let g:go_metalinter_deadline = "5s"

[bhcleek]

@flw-cn are you talking about on master? If so, then you'll need to set g:go_metalinter_command to golangci-lint, not a custom command.

[flw-cn]

Yes. I currently use the master branch. It works fine if I don't customize g:go_metalinter_command. But I want to add --exclude-use-default=true to golangci-lint because I want to suppress those errors.

$ git status && git show | head
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean
commit 10f8fea26e24856d809e2c651a2034a88a778254
Author: Billie Cleek <bhcleek@gmail.com>
Date:   Mon Feb 10 18:33:09 2020 -0800

    update CHANGELOG.md for #2639

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 16cabf4..c71607e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md

[bhcleek]

@flw-cn that will be a separate issue on master, and isn't directly relevant to the question I'm asking @orlangure . To do what you want on master, we'll need a new PR that moves https://github.com/fatih/vim-go/blob/master/autoload/go/lint.vim#L321 to within the if block at https://github.com/fatih/vim-go/blob/master/autoload/go/lint.vim#L323-L325.

[flw-cn]

Ok. I thought there was no plan to solve it. I will wait patiently.

[bhcleek]

@flw-cn feel free to create an issue. What you're asking for doesn't have one yet.

[flw-cn]

@bhcleek Thank you for your support. I noticed that there was already such an issue #2707. I have contracted it and will keep watching.

[orlangure]

You should be able to configure your golangci-lint config file as you want and then use let g:go_metalinter_enabled=[] and/or let g:go_metalinter_autosave_enabled = [] respectively.

@bhcleek my previous configuration doesn't work on master (when I use custom golangci-lint command). There are two missing things for me to start using configuration instead of a custom command:

1. Configs only allow to set enabled linters, but in my case I use --enable-all with 2 or 3 excluded linters
2. More importantly, if I understand correctly, by default golangci-lint runs only on current package, but I added ./... to my command to lint all child packages as well.

Maybe a change in my workflow can allow me to use configs with autosave (meaning I'll never need to lint child packages since every change is checked), but answering your original question, this PR is still relevant.

[flw-cn]

@orlangure If I understand correctly, the default GolangCI parameter is . / ... and not just the current package.

See also：
https://github.com/golangci/golangci-lint#quick-start

Quick Start

To run golangci-lint execute:

golangci-lint run

It's an equivalent of executing:

golangci-lint run ./...

[orlangure]

@flw-cn, thanks, I completely missed that.
@bhcleek, I changed my config to use whitelist of linters instead of a blacklist, all seems to work properly on master.

Thank you!

[flw-cn]

to @bhcleek:

This PR still seems to have what I want. But it now conflicts with the master branch. Will it continue to be merged?

I think the processing logic of this PR is correct: determine whether it is golangci-lint based on the first word of g:go_metalinter_command instead of the whole line.

Although moving https://github.com/fatih/vim-go/blob/master/autoload/go/lint.vim#L321 into the if block at https://github.com/fatih/vim-go/blob/master/autoload/go/lint.vim#L323-L325 can also bypass my problem, it seems that this PR is more reasonable.

Expect it to be merged eventually.

[bhcleek]

@flw-cn I do not intend to merge, because the ultimate goals of this PR can be accomplished on master without additional changes and trying to support an arbitrary command is difficult and error prone.

@orlangure

Configs only allow to set enabled linters, but in my case I use --enable-all with 2 or 3 excluded linters

golangci/golangci-lint#542 suggests that you can put enable-all: true in the golangci-lint, but the maintainer explicitly says not to, and that it i going to be removed; it looks like you're doing the right thing by whitelisting instead of blacklisting. Of course, that means that you could use g:go_metalinter_enabled and g:go_metalinter_autosave_enabled to set the linters to use instead of relying on the golangci-lint config file.

[flw-cn]

Golangci-lint's configuration file is definitely needed because GolangCI not only needs to run when developing locally but also needs to run in the workflow of GitHub Actions.

I don't think making vim-go support golangci-lint's configuration file is an excessive requirement, nor do I think parsing g:go_metalinter_command is a difficult task. (In fact, hasn't this already been done in this PR?)

I still want to be able to merge the contents of this PR into master, after all, this is achievable and not complicated.

[bhcleek]

I don't think making vim-go support golangci-lint's configuration file is an excessive requirement

Fully supporting golangci-lint config file is already done on master.

nor do I think parsing g:go_metalinter_command is a difficult task. (In fact, hasn't this already been done in this PR?)

I still want to be able to merge the contents of this PR into master, after all, this is achievable and not complicated.

It's more error prone than it appears on its face. With the change that was made to master to support the golangci-lint config file and @orlangure 's confirmation that the changes on master work for them, I don't see a reason to support an arbitrary command.