Skip to content
This repository has been archived by the owner on May 9, 2021. It is now read-only.

Proposal: blank imports of the embed package should not be lint errors #511

Closed
zevdg opened this issue Mar 4, 2021 · 3 comments
Closed

Comments

@zevdg
Copy link

zevdg commented Mar 4, 2021

https://github.com/golang/go/wiki/CodeReviewComments#import-blank says

Packages that are imported only for their side effects (using the syntax import _ "pkg") should only be imported in the main package of a program, or in tests that require them.

however https://golang.org/pkg/embed/#hdr-Strings_and_Bytes says

The //go:embed directive requires importing "embed", even when using a string or []byte. In source files that don't refer to embed.FS, use a blank import (import _ "embed").

If we follow the current letter of the law, that arbitrarily limits embedding of strings and []bytes (but not embed.FSs) to main packages and tests. This doesn't seem intentional. I propose we special case the embed package here and allow blank imports of embed from any package.

Given https://github.com/golang/lint#scope, it sounds like we should update CodeReviewComments#import-blank to note this exception as well. There is precedent for that doc having rules with clear exceptions. For example:

Don't add a Context member to a struct type; instead add a ctx parameter to each method on that type that needs to pass it along. The one exception is for methods whose signature must match an interface in the standard library or in a third party library.

I think a simple update that explains the what and not the why would be sufficient. One only needs to read the documentation of the embed package to see why this exception makes sense. Something like:

Packages that are imported only for their side effects (using the syntax import _ "pkg") should only be imported in the main package of a program, or in tests that require them. The embed package is an exception to this rule.

@zevdg
Copy link
Author

zevdg commented Mar 4, 2021

Oh, I just found golang/go#38968, so nvm. Leaving this here for reference.

@Antonboom
Copy link

+1

@mvdan
Copy link
Member

mvdan commented May 8, 2021

Thank you for submitting this issue! As per golang/go#38968, we are freezing and deprecating golint. There's no drop-in replacement to golint per se, but you should find that Staticcheck works well in encouraging good Go code, much like golint did in the past, since it also includes style checks. There's always gofmt and go vet too, of course.

If you would like to contribute further, I'd encourage you to engage Staticcheck's issue tracker or look at vet's open issues, as they are both actively maintained. If you have an idea that doesn't fit into either of those tools, you could look at other Go linters, or write your own - these days it's fairly straightforward with go/analysis.

To help avoid confusion, I'm closing all issues before we freeze the repository. If you have any feedback, you can leave a comment on the proposal thread where it was decided to deprecate golint - though note that the proposal has been accepted for nearly a year. Thanks!

@mvdan mvdan closed this as completed May 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants