Make package comment more confident (#694)

Author: okhowang

lint/package.go

@@ -36,6 +36,11 @@ var (
 	notSet     = 3
 )
 
+// Files return package's files.
+func (p *Package) Files() map[string]*File {
+	return p.files
+}
+
 // IsMain returns if that's the main package.
 func (p *Package) IsMain() bool {
 	p.Lock()

rule/package-comments.go

@@ -5,6 +5,7 @@ import (
 	"go/ast"
 	"go/token"
 	"strings"
+	"sync"
 
 	"github.com/mgechev/revive/lint"
 )
@@ -14,10 +15,12 @@ import (
 // This has a notable false positive in that a package comment
 // could rightfully appear in a different file of the same package,
 // but that's not easy to fix since this linter is file-oriented.
-type PackageCommentsRule struct{}
+type PackageCommentsRule struct {
+	checkPackageCommentCache sync.Map
+}
 
 // Apply applies the rule to given file.
-func (*PackageCommentsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+func (r *PackageCommentsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 
 	if file.IsTest() {
@@ -29,7 +32,7 @@ func (*PackageCommentsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fail
 	}
 
 	fileAst := file.AST
-	w := &lintPackageComments{fileAst, file, onFailure}
+	w := &lintPackageComments{fileAst, file, onFailure, r}
 	ast.Walk(w, fileAst)
 	return failures
 }
@@ -43,6 +46,49 @@ type lintPackageComments struct {
 	fileAst   *ast.File
 	file      *lint.File
 	onFailure func(lint.Failure)
+	rule      *PackageCommentsRule
+}
+
+func (l *lintPackageComments) checkPackageComment() []lint.Failure {
+	// deduplicate warnings in package
+	if _, exists := l.rule.checkPackageCommentCache.LoadOrStore(l.file.Pkg, struct{}{}); exists {
+		return nil
+	}
+	var docFile *ast.File     // which name is doc.go
+	var packageFile *ast.File // which name is $package.go
+	var firstFile *ast.File
+	var firstFileName string
+	for name, file := range l.file.Pkg.Files() {
+		if file.AST.Doc != nil {
+			return nil
+		}
+		if name == "doc.go" {
+			docFile = file.AST
+		}
+		if name == file.AST.Name.String()+".go" {
+			packageFile = file.AST
+		}
+		if firstFileName == "" || firstFileName > name {
+			firstFile = file.AST
+			firstFileName = name
+		}
+	}
+	// prefer warning on doc.go, $package.go over first file
+	if docFile == nil {
+		docFile = packageFile
+	}
+	if docFile == nil {
+		docFile = firstFile
+	}
+	if docFile != nil {
+		return []lint.Failure{{
+			Category:   "comments",
+			Node:       docFile,
+			Confidence: 1,
+			Failure:    "should have a package comment",
+		}}
+	}
+	return nil
 }
 
 func (l *lintPackageComments) Visit(_ ast.Node) ast.Visitor {
@@ -89,12 +135,9 @@ func (l *lintPackageComments) Visit(_ ast.Node) ast.Visitor {
 	}
 
 	if l.fileAst.Doc == nil {
-		l.onFailure(lint.Failure{
-			Category:   "comments",
-			Node:       l.fileAst,
-			Confidence: 0.2,
-			Failure:    "should have a package comment, unless it's in another file for this package",
-		})
+		for _, failure := range l.checkPackageComment() {
+			l.onFailure(failure)
+		}
 		return nil
 	}
 	s := l.fileAst.Doc.Text()

testdata/golint/package-doc1.go

@@ -1,3 +1,3 @@
 // Test of missing package comment.
 
-package foo // MATCH /should have a package comment, unless it's in another file for this package/
+package foo // MATCH /should have a package comment/