search

atlassian / bazel-tools

Reusable bits for Bazel
Apache License 2.0
113 stars 36 forks source link

roadmap for nogo, go modules, and generated code #118

Closed derekperkins closed 4 years ago

derekperkins commented 4 years ago

The documentation for these various tools are very clear about how go modules and generated code are not supported, with a suggestion to look at nogo. Is there any plan to build those nogo integrations here or elsewhere?

derekperkins commented 4 years ago

cc @whizard

ash2k commented 4 years ago

Thanks for the question. I personally don't have any plans to work on such integrations and I'm not aware of any plans someone else might have.

nogo supports plugging in Analyzers so the path forward is to see how the desired linters can be plugged into it.

Looking at golang-ci-lint, for instance, I found that at least some linters (I've checked only two - see https://github.com/golangci/golangci-lint/tree/master/pkg/golinters) are using the golang.org/x/tools/go/analysis package, which is what nogo expects. E.g. this is how one of the linters is constructed https://github.com/tdakkota/asciicheck/blob/master/asciicheck.go. So to plug it into nogo one would need to create adapter packages (one per linter) with the desired linters exposed as package-level variables so that nogo can use them. This seems straightforward.

I personally use golant-ci-lint at the moment because it's more convenient to configure linters to ignore certain lines of code using a comment. I think such granularity is not available via nogo, unless I'm just not seeing it in the docs.

Another issue with nogo is that it analyzes all Go files in the build. I don't want it to analyze my dependencies, for example. This is a waste of CPU as I cannot do much about any found issues anyway. It's good to run it once/periodically to see the biggest problems in third-party libraries, but not all the time.

FYI @jayconrod.

derekperkins commented 4 years ago

Thanks for the feedback.

We don't accept not go/analysis linters. https://golangci-lint.run/contributing/new-linters/

It looks like most, if not all, linters in golangci-lint require the use of the golang.org/x/tools/go/analysis, which is great news for possible porting.

Another issue with nogo is that it analyzes all Go files in the build. I don't want it to analyze my dependencies, for example.

That's a valid concern for sure. Maybe it has changed since you last looked at it, but now It seems like it should be pretty simple to setup an only_files filter.

image https://github.com/bazelbuild/rules_go/blob/master/go/nogo.rst#configuring-analyzers

I personally use golant-ci-lint at the moment because it's more convenient to configure linters to ignore certain lines of code using a comment. I think such granularity is not available via nogo, unless I'm just not seeing it in the docs

This is the piece that I'm not sure about, and like you, I haven't been able to see anything indicating that level of granularity.

I've gone ahead and opened an upstream issue in golangci-lint to see if there is interest in adding Bazel support upstream.

ash2k commented 4 years ago

Thanks for trying to move this forward! I'll be watching those issues. I'm closing this one as there is nothing to do here in this project.