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

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

Closed
mstoykov opened this issue Apr 18, 2016 · 8 comments
Closed

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

mstoykov opened this issue Apr 18, 2016 · 8 comments

Comments

@mstoykov
Copy link

mstoykov commented Apr 18, 2016

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.

@bradfitz bradfitz added this to the Unreleased milestone Apr 18, 2016
@bradfitz bradfitz changed the title x/tools/cmd/stringer generated code does not pass golint x/tools/cmd/stringer: generated code does not pass golint Apr 18, 2016
@robpike
Copy link
Contributor

robpike commented Apr 18, 2016

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.

@robpike robpike closed this as completed Apr 18, 2016
@meling
Copy link

meling commented Jul 26, 2016

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
Copy link
Contributor

@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
Copy link
Contributor

robpike commented Jul 27, 2016

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
Copy link

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 notifications@github.com 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
#15346 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAAZ_94Zdj2CTDsPnFvf6t0Xk3uGDCbks5qZvmfgaJpZM4IJfXg
.

@robpike
Copy link
Contributor

robpike commented Jul 28, 2016

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
Copy link
Contributor

brtzsnr commented Jan 28, 2017

@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
Copy link
Contributor

robpike commented Jan 28, 2017

See #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.

@golang golang locked and limited conversation to collaborators Jan 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants