go-tools
go-tools copied to clipboard
dominikh
quickfix: suggest replacing '%s' and others with %q
I'm tired of seeing '%s', "%s", `%s`, and so on. These should always be just %q.
Reasons:
- Simpler to write
- Consistent
- Actually quotes the contents - e.g.
'%s'can give you'don't do this'
This might fall under gosimple, but the last point makes this a bug I'd say. So perhaps one for staticcheck.
A conservative way to introduce this check would be to only complain on "%s", and think about the others later. That way, no correct output strings will change.
When we have analysis checkers that can suggest fixes, this would be very natural in your editor to see it as a suggestion you can just accept. The really nice thing would be to mark it as high confidence in the "%s" case and low confidence in the '%s' case so the former is auto fixed but latter would take a user action. Not sure it would ever belong in a tool that returns an exit code.
Agreed that this should be automatically fixable. I only filed it here because it doesn't seem frequent/important enough for vet, and I don't know where else to file it. For now, sed does an OK job :)
%q has the downside of making Windows file paths not copy/paste friendly because it turns all the back slashes into double back slashes.
I don't think it's always the case that "%s" is an error, which means it may not be a good candidate for auto-fixing. On the other hand, I should probably be using it more often.
I tend to write '%s', not because I wish to suggest character quoting semantics, but because I'm probably inside a double-quoted string, and that avoids escaping. Which is probably an argument against my opinions here. :P
@seebs which is why it'll only be part of the "quickfix" category, which should ideally map to the "Hint" severity in LSP. The staticcheck binary will not use quickfix, and editors should display them as very optional but possible changes.
I see people use "%s" quite a lot where they should just use %q, and it's because they don't know about %q. I can't remember ever seeing a legitimate use of "%s". So IMO staticcheck should check for this.
I don't really follow the discussion above about "quickfixes" (and in any case I probably wouldn't use such editor features). To be useful to me, a static analysis pass needs to be something I can put into our standard linting suite and run in CI easily. But I guess I'll just add this to our internal linting tool if you don't think it belongs in staticcheck.
It might make sense in the upcoming codereview category (see https://github.com/dominikh/go-tools/issues/1102 for details on that.)