Add go-metalinter support

[sijad]

ran into this
not sure what should I do

[msftclas]

Hi @sijad, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

[lukehoban]

Thanks @sijad.

I remember running into that same issue when I took a look at this awhile ago. In theory - we could filter the results to only those in the current file - but it would be hugely wasteful to compute all these lint results and just throw them away. I think that issue may be a blocker for getting gometalinter integrated into VSCode and similar tools.

[sijad]

@lukehoban I'm about to create a experimental version of gometalinter in TypeScript
I didn't go deep in gometalinter codes but I guess lot of thing should be changed to make it possible to run it for only single files, and if that TypeScript version goes well, what do you think about merge that with this extension? or should it be a separated extension?

[lukehoban]

@sijad I think it might be better to stick with using gometalinter if possible - I prefer to keep as much Go-specific logic in well-maintained Go ecosystem projects instead of moving it into the vscode-go extension.

Per the comment on alecthomas/gometalinter#119 it sounds like other tools that use gometalinter do just deal with just invoking linters at folder level. I'm open to trying the same in vscode-go, and if there are any real blocking perf issues with this, doing the work in gometalinter itself to enable this (which can also benefit other Go tools).

[sijad]

Sorry for delay, with some changes now gometalinter works but I need help in a few things.
lintTool config option now have an enum that only allow golint or gometalinter to be set. there is two reason:

1- gometalinter prints to stdout but golint prints to stderr, can we read both of them?
2- filename needs to pushed for golint arguments but it should not passed for gometalinter, if filename does not pass, golint will lint for whole directory (cwd) like gometalinter

I'll add a few test case for gometalinter soon

[lukehoban]

Thanks @sijad. This generally looks good.

Keeping golint as the default and letting users opt-in to gometalinter also means that we don't necessarily have to help with acquiring and configuring gometalinter, which keeps this simpler. I think that's probably a good thing for now.

I'm not sure whether there is sufficient test infrastructure currently to add a test for this, but if so, would be great to add that. It will likely require at least adding installation of gometalinter into the TravisCI configuration.

[sijad]

Ok some test cases also has been added, @lukehoban let me know if anything need to be changed
thanks

[sijad]

not sure why it fails in travis mac environment, any thoughts?

[johansja]

Wondering what is the status of this. I have played around with this a little bit as well. https://github.com/JohanSJA/vscode-go/tree/gometalinter

[atishpatel]

Is there a reason this hasn't been merged? I would really like this feature. Could I help in any way?

[lukehoban]

Finally got a chance to review and merge. I had to tweak the regexp a little to match some gometalinter warnings - specifically when there was no column number present. But after that, it looked good.

For others interested in this - note that it's still not the default - and if you want to use "go.lintTool": "gometalinter" you'll have to install it yourself since it is a multistep install (go get then gometalinter --install --update).

[lukehoban]

Thanks @sijad for the PR!

[ramya-rao-a]

gometalinter prints to stdout but golint prints to stderr

@sijad The latest golint prints to stdout as well

if you want to use "go.lintTool": "gometalinter" you'll have to install it yourself since it is a multistep install (go get then gometalinter --install --update).

@sijad @lukehoban Was there any reason, not to install gometalinter and then run gometalinter --install as part of the "install tool" process?

I have a PR out #735 to do the same, and was wondering if I am missing some reason why it shouldnt be done.

Would love your feedback on the PR as well.