diff --git a/lint/package.go b/lint/package.go index 52505f3..47e5b6f 100644 --- a/lint/package.go +++ b/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() diff --git a/rule/package-comments.go b/rule/package-comments.go index 072a523..33963ab 100644 --- a/rule/package-comments.go +++ b/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() diff --git a/testdata/golint/package-doc1.go b/testdata/golint/package-doc1.go index 1ab0a7c..4c649c2 100644 --- a/testdata/golint/package-doc1.go +++ b/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/