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

[FR]: lint Go code #208

Open
alexeagle opened this issue Apr 11, 2024 · 8 comments
Open

[FR]: lint Go code #208

alexeagle opened this issue Apr 11, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@alexeagle
Copy link
Member

What is the current behavior?

With #207 we no longer have any Go linters.

Describe the feature

#137 and #129 were bugs when golangci-lint was in this repo, those need to be solved for a roll-forward.

@alexeagle alexeagle added the enhancement New feature or request label Apr 11, 2024
@mrmeku
Copy link
Contributor

mrmeku commented Apr 11, 2024

In prior projects, I've used the github action provided by golang ci lint itself which supports its own only report only new issues implementation via only-new-issues: true. I wonder if that strategy can be applied here. Let the linter have access to all source files but cull the reported errors to just those affected by a git diff.

@alexeagle
Copy link
Member Author

Doesn't work with our design here that lints are just actions that visit the go_library graph and rely on Bazel's action caching (and remote execution).
Having git information available to the linter would make it non-hermetic and not follow that design.
But let's chat, maybe there's a middle path here I'm not seeing.

@ewhauser
Copy link
Contributor

I think it is worth trying to run a nogo binary as rules_lint compatible linter as opposed to trying to coerce golangci-lint to work in a Bazel way. You can run all the same analyzers with nogo that you can run under golangci-lint.

This is the approach that gVisor takes: https://github.com/google/gvisor/blob/master/tools/nogo/defs.bzl

@alexeagle
Copy link
Member Author

Ooh that example should be really helpful, thanks!

@aaomidi
Copy link

aaomidi commented Sep 6, 2024

This is one of our primary issues adopting bazel at the moment. We've kinda started wrapping around most of the golangci-lint linters (and, unfortunately, it's really hard to get the exact golangci-lint behavior by just doing this, since there's a lot of sensible defaults golangci-lint applies)

@ewhauser
Copy link
Contributor

ewhauser commented Sep 6, 2024

Have you tried https://github.com/sluongng/nogo-analyzer? It gets you very close to what golangci-lint does using the same analyzers.

@aaomidi
Copy link

aaomidi commented Sep 6, 2024

I have, unfortunately its not really what I'm looking for. For example: https://github.com/sluongng/nogo-analyzer/blob/master/goci-lint/errcheck/analyzer.go just brings in the errcheck analyzer, which isn't exactly what the golangci-lint project is doing.

Beyond that I think that's not a bzlmod yet, which makes consuming it for our team a bit more difficult.

Honestly, I also much prefer how aspect rules_lint works compare to how nogo does it. I feel like given enough linters, nogo is going to really slow down builds.

Also, no autofix support :(

@derekperkins
Copy link

derekperkins commented Sep 19, 2024

We'd love to move golangci-lint to bazel, but continue to run the GitHub Action in the meantime. 🤞

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

No branches or pull requests

5 participants