golangci/golangci-lint

Add "nilaway" as a linter

Dentrax opened this issue ยท 12 comments

Your feature request related to a problem? Please describe.

NilAway is a static analysis tool that seeks to help developers avoid nil panics in production by catching them at compile time rather than runtime.

Describe the solution you'd like.

https://github.com/uber-go/nilaway

Describe alternatives you've considered.

-

Additional context.

IIUC, nilaway is more like a "tool" rather than a linter.

/cc @yuxincs @zzqatuber

ldez commented

Analyzers that just display potential problems are not considered as linters inside golangci-lint because of the large number of false positives.
Those kinds of analyzers are not allowed.

Similar to #3969

Ah, good to know. Maybe devs of that tool can optimize the code base to provide a linter interface.

Just curious to ask, why don't you provide a some kind of feature gate to allow "tools" and "false positives"? I totally understand your "golangci-lint's criteria". But maybe you can define "linter graduation policy": alpha > beta > stable, etc. So enabling experimental gates could allow non-stable and false-positive tools/linters.

I know, this would be a part of totally different proposal to discuss more about.

ldez commented

Maybe devs of that tool can optimize the code base to provide a linter interface.

This is not the problem: the problem is false positives, not the linter interface.

Just curious to ask, why don't you provide a some kind of feature gate to allow "tools" and "false positives"? I totally understand your "golangci-lint's criteria". But maybe you can define "linter graduation policy": alpha > beta > stable, etc. So enabling experimental gates could allow non-stable and false-positive tools/linters.

The goal of golangci-lint is to be accurate and relevant, i.e. no false positives, this is why we have default exclusions.
False positives are no relation with a linter "state" (alpha > beta > stable or experimental).
So, for now, there is no possibility of "false-positive linter" or "potential issues detector".

I don't fully understand this policy, when golangci-lint has style linters which are all very "potential issue detectors" and have lot of "false positives", but, it's up to you I guess

Why not just mention the problems in the documentation and allow devs the choice of whether to use it or not? Why opinionated over flexible?

dolmen commented

@karelbilek nilaway is still quite young and has really too many false positives. Just have a look at its issues queue.

Why can't it just be added outside of the default set -- something people would enable if they wanted?

I've created a custom plugin to be used locally and in GHA

(I have horrible experience with using plugins, because you need to keep tracking all the versions, and it produces inscrutable error messages when things go wrong. But, if it helps anyone... good)

yeah i can tell :D , but i've considered that in that plugin and the script with it should be able to detect the dependancies version and force it before building it

(I have horrible experience with using plugins, because you need to keep tracking all the versions, and it produces inscrutable error messages when things go wrong. But, if it helps anyone... good)

I agree. I have proposed an alternative way for integrating private linters from learnings of caddy the web server written in Go: #2505 (comment) which makes it easier to build a single custom binary (with the private linter code). Not sure if golangci-lint folks will like it ๐Ÿ˜ƒ

If we have to build a plugin, we're happy to add some logic (e.g., the New function) to make it easier to build NilAway as a plugin.

Somewhat related - golang/go#63290