Ignore blank import of embed if embed is actually used in the file (#501)

* Ignore blank import of embed

rule/blank-imports.go

@@ -2,6 +2,7 @@ package rule
 
 import (
 	"go/ast"
+	"strings"
 
 	"github.com/mgechev/revive/lint"
 )
@@ -9,66 +10,66 @@ import (
 // BlankImportsRule lints given else constructs.
 type BlankImportsRule struct{}
 
-// Apply applies the rule to given file.
-func (r *BlankImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
-	var failures []lint.Failure
-
-	fileAst := file.AST
-	walker := lintBlankImports{
-		file:    file,
-		fileAst: fileAst,
-		onFailure: func(failure lint.Failure) {
-			failures = append(failures, failure)
-		},
-	}
-
-	ast.Walk(walker, fileAst)
-
-	return failures
-}
-
 // Name returns the rule name.
 func (r *BlankImportsRule) Name() string {
 	return "blank-imports"
 }
 
-type lintBlankImports struct {
+// Apply applies the rule to given file.
-	fileAst   *ast.File
+func (r *BlankImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
-	file      *lint.File
+	if file.Pkg.IsMain() || file.IsTest() {
-	onFailure func(lint.Failure)
-}
-
-func (w lintBlankImports) Visit(_ ast.Node) ast.Visitor {
-	// In package main and in tests, we don't complain about blank imports.
-	if w.file.Pkg.IsMain() || w.file.IsTest() {
 		return nil
 	}
 
+	const (
+		message         = "a blank import should be only in a main or test package, or have a comment justifying it"
+		category        = "imports"
+
+		embedImportPath = `"embed"`
+	)
+
+	var failures []lint.Failure
+
 	// The first element of each contiguous group of blank imports should have
 	// an explanatory comment of some kind.
-	for i, imp := range w.fileAst.Imports {
+	for i, imp := range file.AST.Imports {
-		pos := w.file.ToPosition(imp.Pos())
+		pos := file.ToPosition(imp.Pos())
 
 		if !isBlank(imp.Name) {
 			continue // Ignore non-blank imports.
 		}
+
 		if i > 0 {
-			prev := w.fileAst.Imports[i-1]
+			prev := file.AST.Imports[i-1]
-			prevPos := w.file.ToPosition(prev.Pos())
+			prevPos := file.ToPosition(prev.Pos())
-			if isBlank(prev.Name) && prevPos.Line+1 == pos.Line {
+
-				continue // A subsequent blank in a group.
+			isSubsequentBlancInAGroup := isBlank(prev.Name) && prevPos.Line+1 == pos.Line && prev.Path.Value != embedImportPath
+			if isSubsequentBlancInAGroup {
+				continue
 			}
 		}
 
+		if imp.Path.Value == embedImportPath && r.fileHasValidEmbedComment(file.AST) {
+			continue
+		}
+
 		// This is the first blank import of a group.
 		if imp.Doc == nil && imp.Comment == nil {
-			w.onFailure(lint.Failure{
+			failures = append(failures, lint.Failure{Failure: message, Category: category, Node: imp, Confidence: 1})
-				Node:       imp,
-				Failure:    "a blank import should be only in a main or test package, or have a comment justifying it",
-				Confidence: 1,
-				Category:   "imports",
-			})
 		}
 	}
-	return nil
+
+	return failures
+}
+
+func (r *BlankImportsRule) fileHasValidEmbedComment(fileAst *ast.File) bool {
+	for _, commentGroup := range fileAst.Comments {
+		for _, comment := range commentGroup.List {
+			if strings.HasPrefix(comment.Text, "//go:embed ") {
+				return true
+			}
+		}
+	}
+
+	return false
 }

testdata/golint/blank-import-lib.go

@@ -38,6 +38,11 @@ import (
 	_ "go/token"
 )
 
+import (
+	_ "embed"
+	/* MATCH:42 /a blank import should be only in a main or test package, or have a comment justifying it/ */
+)
+
 var (
 	_ fmt.Stringer // for "fmt"
 	_ ast.Node     // for "go/ast"

testdata/golint/blank-import-with-embed.go

@@ -0,0 +1,14 @@
+// Test that blank import of "embed" is allowed
+
+// Package foo ...
+package foo
+
+import (
+	_ "embed"
+	_ "fmt"
+	/* MATCH:8 /a blank import should be only in a main or test package, or have a comment justifying it/ */
+)
+
+//go:embed .gitignore
+var _ string
+