Repository: mgechev/revive (go)

Issue:
Improve "exported" rule
Right now, exported rule will report this

```go
//nolint:lll // foo
func Whatever() {}
```

>Blah blah commented should be form like ...

It also reports this

```go
//go:embed foo.json
var foo string
```

Or anything that is a Go directive

I spotted it by working on this https://github.com/gofr-dev/gofr/pull/1411 and especially this code

```go
		// code before
}

//nolint:exhaustive // We just want to take care of slice and struct in this case. 
func (c *Client) QueryWithCtx(ctx context.Context, dest any, stmt string, values ...any) error {

		// code after
```

https://github.com/gofr-dev/gofr/pull/1411/files#diff-5931c571fb913c21edcd5cd8b864b2fee691ebda14070081ad5765696b51646dR120-123

I had to add a comment for the method to make it works.

The problem occurs with golangci-lint by using EXC0014 only

It might be related to the fact exported already report a comment should be present for exported

But here, for me, suggesting "should be form" with a Go directive is a false positive

Pull request:
fix: invalid message reported for directive comment
They were invalid but not reported with the correct message.

So they were wrongly caught by golangci-lint that excluded them by default.

Fixes #1202

Patch:
diff --git a/rule/exported.go b/rule/exported.go
index 964c06e..7d59c4d 100644
--- a/rule/exported.go
+++ b/rule/exported.go
@@ -182,7 +182,7 @@ func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) {
 		return
 	}
 
-	if fn.Doc == nil {
+	if !hasTextComment(fn.Doc) {
 		w.onFailure(lint.Failure{
 			Node:       fn,
 			Confidence: 1,
@@ -247,7 +247,7 @@ func (w *lintExported) lintTypeDoc(t *ast.TypeSpec, doc *ast.CommentGroup) {
 		return
 	}
 
-	if doc == nil {
+	if !hasTextComment(doc) {
 		w.onFailure(lint.Failure{
 			Node:       t,
 			Confidence: 1,
@@ -314,7 +314,7 @@ func (w *lintExported) lintValueSpecDoc(vs *ast.ValueSpec, gd *ast.GenDecl, genD
 		return
 	}
 
-	if vs.Doc == nil && gd.Doc == nil {
+	if !hasTextComment(vs.Doc) && !hasTextComment(gd.Doc) {
 		if genDeclMissingComments[gd] {
 			return
 		}
@@ -332,16 +332,16 @@ func (w *lintExported) lintValueSpecDoc(vs *ast.ValueSpec, gd *ast.GenDecl, genD
 		return
 	}
 	// If this GenDecl has parens and a comment, we don't check its comment form.
-	if gd.Doc != nil && gd.Lparen.IsValid() {
+	if hasTextComment(gd.Doc) && gd.Lparen.IsValid() {
 		return
 	}
 	// The relevant text to check will be on either vs.Doc or gd.Doc.
 	// Use vs.Doc preferentially.
 	var doc *ast.CommentGroup
 	switch {
-	case vs.Doc != nil:
+	case hasTextComment(vs.Doc):
 		doc = vs.Doc
-	case vs.Comment != nil && gd.Doc == nil:
+	case hasTextComment(vs.Comment) && !hasTextComment(gd.Doc):
 		doc = vs.Comment
 	default:
 		doc = gd.Doc
@@ -359,6 +359,19 @@ func (w *lintExported) lintValueSpecDoc(vs *ast.ValueSpec, gd *ast.GenDecl, genD
 	}
 }
 
+// hasTextComment returns true if the comment contains a text comment
+// e.g. //go:embed foo.txt a directive comment, not a text comment
+// e.g. //nolint:whatever is a directive comment, not a text comment
+func hasTextComment(comment *ast.CommentGroup) bool {
+	if comment == nil {
+		return false
+	}
+
+	// a comment could be directive and not a text comment
+	text := comment.Text()
+	return text != ""
+}
+
 // normalizeText is a helper function that normalizes comment strings by:
 // * removing one leading space
 //
@@ -388,7 +401,7 @@ func (w *lintExported) Visit(n ast.Node) ast.Visitor {
 	case *ast.TypeSpec:
 		// inside a GenDecl, which usually has the doc
 		doc := v.Doc
-		if doc == nil {
+		if !hasTextComment(doc) {
 			doc = w.lastGen.Doc
 		}
 		w.lintTypeDoc(v, doc)
@@ -424,7 +437,7 @@ func (w *lintExported) lintInterfaceMethod(typeName string, m *ast.Field) {
 		return
 	}
 	name := m.Names[0].Name
-	if m.Doc == nil {
+	if !hasTextComment(m.Doc) {
 		w.onFailure(lint.Failure{
 			Node:       m,
 			Confidence: 1,


Test patch:
diff --git a/test/exported_test.go b/test/exported_test.go
index 6e32345..a69b90e 100644
--- a/test/exported_test.go
+++ b/test/exported_test.go
@@ -36,3 +36,7 @@ func TestCheckDisablingOnDeclarationTypes(t *testing.T) {
 
 	testRule(t, "exported_issue_1045", &rule.ExportedRule{}, &lint.RuleConfig{Arguments: args})
 }
+
+func TestCheckDirectiveComment(t *testing.T) {
+	testRule(t, "exported_issue_1202", &rule.ExportedRule{}, &lint.RuleConfig{})
+}
diff --git a/testdata/exported_issue_1202.go b/testdata/exported_issue_1202.go
new file mode 100644
index 0000000..aad298d
--- /dev/null
+++ b/testdata/exported_issue_1202.go
@@ -0,0 +1,27 @@
+package golint
+
+import (
+	_ "embed"
+)
+
+var A []byte // MATCH /exported var A should have comment or be unexported/
+
+var B string // MATCH /exported var B should have comment or be unexported/
+
+//go:embed foo.txt
+var C []byte // MATCH /exported var C should have comment or be unexported/
+
+//go:generate pwd
+var D string // MATCH /exported var D should have comment or be unexported/
+
+func E() string { // MATCH /exported function E should have comment or be unexported/
+	return "E"
+}
+
+//nolint:gochecknoglobals
+func F() string { // MATCH /exported function F should have comment or be unexported/
+	return "F"
+}
+
+//nolint:gochecknoglobals
+const G = "G" // MATCH /exported const G should have comment or be unexported/