Add a lint warning for blank imports in library packages.
This adds a check to the golint tool for blank imports in library packages, if
those imports do not come with a documentation comment of some kind. Only the
presence of a comment is checked, not its content. Package main is exempt from
this check.
Signed-off-by: David Symonds <dsymonds@golang.org>
diff --git a/lint.go b/lint.go
index d679f2f..67cb0a4 100644
--- a/lint.go
+++ b/lint.go
@@ -65,6 +65,7 @@
f.lintPackageComment()
f.lintImports()
+ f.lintBlankImports()
f.lintExported()
f.lintNames()
f.lintVarDecls()
@@ -141,6 +142,37 @@
}
}
+// lintBlankImports complains if a non-main package has blank imports that are
+// not documented.
+func (f *file) lintBlankImports() {
+ // In package main and in tests, we don't complain about blank imports.
+ if f.f.Name.Name == "main" || f.isTest() {
+ return
+ }
+
+ // The first element of each contiguous group of blank imports should have
+ // an explanatory comment of some kind.
+ for i, imp := range f.f.Imports {
+ pos := f.fset.Position(imp.Pos())
+
+ if !isBlank(imp.Name) {
+ continue // Ignore non-blank imports.
+ }
+ if i > 0 {
+ prev := f.f.Imports[i-1]
+ prevPos := f.fset.Position(prev.Pos())
+ if isBlank(prev.Name) && prevPos.Line+1 == pos.Line {
+ continue // A subsequent blank in a group.
+ }
+ }
+
+ // This is the first blank import of a group.
+ if imp.Doc == nil && imp.Comment == nil {
+ f.errorf(imp, 1, "blank imports in libraries should have a comment")
+ }
+ }
+}
+
// lintImports examines import blocks.
func (f *file) lintImports() {
@@ -666,6 +698,10 @@
return ok && id.Name == ident
}
+// isBlank returns whether id is the blank identifier "_".
+// If id == nil, the answer is false.
+func isBlank(id *ast.Ident) bool { return id != nil && id.Name == "_" }
+
func isPkgDot(expr ast.Expr, pkg, name string) bool {
sel, ok := expr.(*ast.SelectorExpr)
return ok && isIdent(sel.X, pkg) && isIdent(sel.Sel, name)
diff --git a/testdata/blank-import-lib.go b/testdata/blank-import-lib.go
new file mode 100644
index 0000000..e9a2cd5
--- /dev/null
+++ b/testdata/blank-import-lib.go
@@ -0,0 +1,33 @@
+// Test that blank imports in library packages are flagged.
+
+// Package foo ...
+package foo
+
+// The instructions need to go before the imports below so they will not be
+// mistaken for documentation.
+
+/* MATCH /blank imports/ */ import _ "encoding/json"
+
+import (
+ "fmt"
+ /* MATCH /blank imports/ */ _ "os"
+
+ /* MATCH /blank imports/ */ _ "net/http"
+ _ "path"
+)
+
+import _ "encoding/base64" // Don't gripe about this
+
+import (
+ // Don't gripe about these next two lines.
+ _ "compress/zlib"
+ _ "syscall"
+
+ /* MATCH /blank imports/ */ _ "path/filepath"
+)
+
+import (
+ "go/ast"
+ _ "go/scanner" // Don't gripe about this or the following line.
+ _ "go/token"
+)
diff --git a/testdata/blank-import-lib_test.go b/testdata/blank-import-lib_test.go
new file mode 100644
index 0000000..0307985
--- /dev/null
+++ b/testdata/blank-import-lib_test.go
@@ -0,0 +1,20 @@
+// Test that blank imports in test packages are not flagged.
+// OK
+
+// Package foo ...
+package foo
+
+// These are essentially the same imports as in the "library" package, but
+// these should not trigger the warning because this is a test.
+
+import _ "encoding/json"
+
+import (
+ "fmt"
+ "testing"
+
+ _ "os"
+
+ _ "net/http"
+ _ "path"
+)
diff --git a/testdata/blank-import-main.go b/testdata/blank-import-main.go
new file mode 100644
index 0000000..9b72b1c
--- /dev/null
+++ b/testdata/blank-import-main.go
@@ -0,0 +1,12 @@
+// Test that blank imports in package main are not flagged.
+// OK
+
+// Binary foo ...
+package main
+
+import _ "fmt"
+
+import (
+ "os"
+ _ "path"
+)
