Go: x/tools/cmd/stringer: generated code does not pass golint

[MStoykov]

on go version go1.6 linux/amd64 (and previous as far as I know) the generated code by stringer has some:

• const/vars with '_' in their name
• uses i -= 1 instead of i--

both of which are things golint will (rightfully) complain about.

Currently this means that I would not use stringer (luckily my usecases are small enough to not be much of code), because I would have to hack my way around the automatic golinting of all (non vendor) code in our project.

[robpike]

Not an issue. Generated code does not need to follow style guidelines and in fact, as i this case, can deliberately avoid them to minimize collisions. You don't use _ in an identifier, so when stringer does it reduces the chance of a name collision.

Your problem is treating golint as gospel. It is heuristic and unreliable. Helpful yes, but often wrong.

[meling]

Sorry to resurface this. Editors are integrating golint and it would be more helpful and convenient to reduce the warning noise by avoiding the use of underscore in this particular case.

Please note that golint does in fact accept the first/prefix underscore that you already have in there to minimize collisions. (not sure if this is a feature or a bug in golint). So there is really not much need for the second underscore that causes the golint warning.

Moreover, I'm sure there are other unlikely names that could be used to further minimize collisions, for example ...StringerName and ...StringerIndex.

[ianlancetaylor]

@meling Don't run golint on generated code. I don't understand the argument about the editor here; there shouldn't be any reason to use an editor to modify the code generated by stringer.

This issue is closed so this is not the best place for a discussion. I recommend raising this on the golang-dev mailnig list.

[robpike]

I could imagine the linter deliberately ignoring generated code, which is usually easy to identify in good practice with the banner across the top.

But I don't think that's the right answer. The right answer is not to require golint passing every line of code before you can build.

[freeformz]

Here is where this comes up for me.

1. I tell people to use golint (and a few similar tools) and to generally
   address the issues it presents as a general rule of thumb.
2. People do so and usually integrate it with their editors and/or CI.
3. At some point they generate code (or integrate a package that does) and
   golint (and/or the other tools they may have also hooked up) start to
   complain. They see these complaints every time golint runs, which could be
   as often as every save.
4. People roll their eyes at Go and golint.
5. People stop using golint (and possibly other tools).

So, while I agree that golint should not be gospel or even run on generated
code, this is pretty common in my experience. It makes go look like a clown
if it's own linter complains about code that an official package generates.

On Tue, Jul 26, 2016 at 11:24 PM Rob Pike [email protected] wrote:

I could imagine the linter deliberately ignoring generated code, which is
usually easy to identify in good practice with the banner across the top.

But I don't think that's the right answer. The right answer is not to
require golint passing every line of code before you can build.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/golang/go/issues/15346#issuecomment-235497705, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAAZ_94Zdj2CTDsPnFvf6t0Xk3uGDCbks5qZvmfgaJpZM4IJfXg
.

[robpike]

I understand the problem, I am just saying that generated code should not be expected to pass all the style heuristics tools can throw at it. In other words, that stringer is not the right place to fix this.

[brtzsnr]

@robpike stringer can be fixed to pass gplint given that's a tool semi-officially supported by the Golang team. Alternatively, go lint needs to learn to ignore generated code unless explicitly asked. Right now golint . checks the whole directory and I have to filter out stringer results.

[robpike]

See https://github.com/golang/go/issues/13560. Its resolution might lead, eventually, to a happier result for you.

I still assert that it is not required for the generated code to pass golint and that in fact the generated code is using unidiomatic style (underscores in names) deliberately and to its advantage.