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

maybe stop relying on "golint" #1705

Closed
kevinburke opened this issue Nov 28, 2017 · 9 comments
Closed

maybe stop relying on "golint" #1705

kevinburke opened this issue Nov 28, 2017 · 9 comments

Comments

@kevinburke
Copy link
Contributor

There are a lot of problems with it and situations where it flags things that are not actually problems. This is confusing and discouraging. It also doesn't have a maintainer.

Just to name a few from the issues list.

golang/lint#354
golang/lint#350
golang/lint#286

Along with others I've run into, for example on #1698:

error: ExampleLoad refers to unknown identifier: Load (vet)

This is confusing, because there is an identifier named Load in that package, and godoc handles it with no problems; it looks like there's an error in the script generating the report card recommendation.

@kirillDanshin
Copy link
Contributor

error: ExampleLoad refers to unknown identifier: Load (vet)

it has nothing to do with golint. it's the go tool vet output. if you don't get it via go tool vet --all, you may want to enable more rules or update the Go to latest stable.
there is nacl.Load(), but you're trying to write an example in nacl_test, not the nacl where Load() is defined. so that's not a vet's bug, it's yours. to omit this error you typically use something like Example_Load.

golint edge cases can be fixed, but I even didn't seen before real-life use of that like piece of sh code, that can trigger that bugs.
even more, I think golang/lint#286 can be ignored 'cause that's not the way you should do that.

thanks for feedback, we appreciate it, but this time it's not relevant.

@kevinburke
Copy link
Contributor Author

Apologies, but there’s a zero percent chance it’s my error. If it was, my test suite would fail and go doc would fail to render the example. Neither of those things happen.

I’ll find the root cause and submit a patch today to make that clear.

@kirillDanshin
Copy link
Contributor

basically, no, in your case it will work with tests and go doc, but go vet will indicate it as an error and will be right

@kevinburke
Copy link
Contributor Author

It's this issue: gojp/goreportcard#187

Also reported here: gojp/goreportcard#113

and here: https://goreportcard.com/report/github.com/arsham/expvastic

also here: gojp/goreportcard#113 (comment)

here's a discussion of an error in the handling of this situation inside "go vet": golang/go#22530. The result of the discussion is you should be using "go vet" directly, not "go tool vet". Which is another error, since goreportcard indicates it's using "go vet". Furthermore, I can't reproduce the error locally using "go tool vet" either, though this may be because I'm running tip.

So, again, it's a problem with the tool, not a problem in my code. I only wish I'd bet you money on that outcome first.

@kevinburke
Copy link
Contributor Author

I'm not 100% sure what this is referring to:

golint edge cases can be fixed, but I even didn't seen before real-life use of that like piece of sh code, that can trigger that bugs.

I ran into one of the "piece of shit" edge cases while trying to update the x/crypto/ssh package: https://go-review.googlesource.com/c/crypto/+/80145#message-1d05ae3ca1b90b6041ab9d5e6edfc88e6e89f38c

Others have as well, multiple times:

I'd also note the Code of Conduct expects you to exercise collaboration and respect in your speech, and marks 'demeaning' speech, such as (incorrectly) labeling code a "piece of shit", as unacceptable. https://github.com/avelino/awesome-go/blob/master/CODE_OF_CONDUCT.md

@kirillDanshin
Copy link
Contributor

note, that you just guessed the continuation of word from a list of options. also, this part of message was a joke and was not meant to be insulting to any Gopher at all.

anyway, it's not changing the subject of this issue

@kevinburke
Copy link
Contributor Author

kevinburke commented Nov 29, 2017 via email

@kevinburke
Copy link
Contributor Author

Would also love to point you at the "Purpose" disclaimer for golint:

The suggestions made by golint are exactly that: suggestions. Golint is not perfect, and has both false positives and false negatives. Do not treat its output as a gold standard. We will not be adding pragmas or other knobs to suppress specific warnings, so do not expect or require code to be completely "lint-free". In short, this tool is not, and will never be, trustworthy enough for its suggestions to be enforced automatically

Which doesn't seem to match up with my interactions with contributors to this repository.

@kirillDanshin
Copy link
Contributor

@kevinburke I need to also point out that when I use golint/govet output to analyze repository, I assume that they can determine only most generic errors and I checking out every error if I can.
so, basically, we can ignore golint/govet results if we see that there are nothing developer can fix and improve overall code quality.
golint/govet will never replace human analyzing, code review of each file in the repo etc.
it just a tool we use and love for simplifying issue and possible bugs search

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

No branches or pull requests

2 participants