diff --git a/rule/package_comments.go b/rule/package_comments.go index 74af62679..6849d04d7 100644 --- a/rule/package_comments.go +++ b/rule/package_comments.go @@ -118,16 +118,16 @@ func (l *lintPackageComments) Visit(_ ast.Node) ast.Visitor { lastCG = cg } if lastCG != nil && strings.HasPrefix(lastCG.Text(), prefix) { - endPos := l.file.ToPosition(lastCG.End()) + endLine := commentGroupEndLine(l.file, lastCG) pkgPos := l.file.ToPosition(l.fileAst.Package) - if endPos.Line+1 < pkgPos.Line { + if endLine+1 < pkgPos.Line { // There isn't a great place to anchor this error; // the start of the blank lines between the doc and the package statement // is at least pointing at the location of the problem. pos := token.Position{ - Filename: endPos.Filename, + Filename: pkgPos.Filename, // Offset not set; it is non-trivial, and doesn't appear to be needed. - Line: endPos.Line + 1, + Line: endLine + 1, Column: 1, } l.onFailure(lint.Failure{ @@ -163,6 +163,16 @@ func (l *lintPackageComments) Visit(_ ast.Node) ast.Visitor { return nil } +// commentGroupEndLine returns the line of the last character of the given comment group. +// +// It does not rely on [ast.CommentGroup.End] because that position is derived from the +// comment text, from which the scanner strips carriage returns. On CRLF sources End() +// therefore points before the actual end of a block comment (see https://go.dev/issue/41197). +func commentGroupEndLine(file *lint.File, cg *ast.CommentGroup) int { + lastComment := cg.List[len(cg.List)-1] + return file.ToPosition(lastComment.Slash).Line + strings.Count(lastComment.Text, "\n") +} + func isEmptyDoc(commentGroup *ast.CommentGroup) bool { return commentGroup == nil || commentGroup.Text() == "" } diff --git a/test/package_comments_test.go b/test/package_comments_test.go new file mode 100644 index 000000000..d5da9a33e --- /dev/null +++ b/test/package_comments_test.go @@ -0,0 +1,13 @@ +package test_test + +import ( + "testing" + + "github.com/mgechev/revive/rule" +) + +func TestPackageComments(t *testing.T) { + testRule(t, "package_comments/issue607_not_match", &rule.PackageCommentsRule{}) + testRule(t, "package_comments/issue607_match", &rule.PackageCommentsRule{}) + testRule(t, "package_comments/issue607_drift_not_match", &rule.PackageCommentsRule{}) +} diff --git a/testdata/package_comments/.gitattributes b/testdata/package_comments/.gitattributes new file mode 100644 index 000000000..d4ec2a579 --- /dev/null +++ b/testdata/package_comments/.gitattributes @@ -0,0 +1,2 @@ +# Fixtures in this directory must keep Windows line endings. +*.go text eol=crlf diff --git a/testdata/package_comments/issue607_drift_not_match.go b/testdata/package_comments/issue607_drift_not_match.go new file mode 100644 index 000000000..e11012672 --- /dev/null +++ b/testdata/package_comments/issue607_drift_not_match.go @@ -0,0 +1,9 @@ +/* +Package fixtures has a multi-line comment. + + + + + +*/ +package fixtures diff --git a/testdata/package_comments/issue607_match.go b/testdata/package_comments/issue607_match.go new file mode 100644 index 000000000..88097a72c --- /dev/null +++ b/testdata/package_comments/issue607_match.go @@ -0,0 +1,8 @@ +/* +Package fixtures has a multi-line comment. +Its line endings have a carriage return. +*/ + +package fixtures + +// MATCH:5 /package comment is detached; there should be no blank lines between it and the package statement/ diff --git a/testdata/package_comments/issue607_not_match.go b/testdata/package_comments/issue607_not_match.go new file mode 100644 index 000000000..d21dd52db --- /dev/null +++ b/testdata/package_comments/issue607_not_match.go @@ -0,0 +1,5 @@ +/* +Package fixtures has a multi-line comment. +Its line endings have a carriage return. +*/ +package fixtures