exported: correctly report deprecation-only comments (#1295)

Author: semihbkgr

rule/exported.go

@@ -182,7 +182,7 @@ func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) {
 		return
 	}
 
-	if !hasTextComment(fn.Doc) {
+	if !hasDocComment(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 !hasTextComment(doc) {
+	if !hasDocComment(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 !hasTextComment(vs.Doc) && !hasTextComment(gd.Doc) {
+	if !hasDocComment(vs.Doc) && !hasDocComment(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 hasTextComment(gd.Doc) && gd.Lparen.IsValid() {
+	if hasDocComment(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 hasTextComment(vs.Doc):
+	case hasDocComment(vs.Doc):
 		doc = vs.Doc
-	case hasTextComment(vs.Comment) && !hasTextComment(gd.Doc):
+	case hasDocComment(vs.Comment) && !hasDocComment(gd.Doc):
 		doc = vs.Comment
 	default:
 		doc = gd.Doc
@@ -359,17 +359,26 @@ func (w *lintExported) lintValueSpecDoc(vs *ast.ValueSpec, gd *ast.GenDecl, genD
 	}
 }
 
-// hasTextComment returns true if the comment contains a text comment
+// hasDocComment reports whether the comment group contains a documentation comment,
+// excluding directive comments and those consisting solely of a deprecation notice.
 // 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 {
+// e.g. // Deprecated: this is a deprecation comment
+func hasDocComment(comment *ast.CommentGroup) bool {
 	if comment == nil {
 		return false
 	}
 
 	// a comment could be directive and not a text comment
-	text := comment.Text()
-	return text != ""
+	text := comment.Text() // removes directives from the comment block
+	return text != "" && !isOnlyDeprecationComment(text)
+}
+
+// isOnlyDeprecationComment returns true if the comment starts with a standard deprecation notice.
+// It considers all paragraphs following the deprecation notice as part of the deprecation comment.
+// Assumes the comment is following the general ordering convention: (doc comment + deprecation)
+func isOnlyDeprecationComment(comment string) bool {
+	return strings.HasPrefix(comment, "Deprecated: ")
 }
 
 // normalizeText is a helper function that normalizes comment strings by:
@@ -401,7 +410,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 !hasTextComment(doc) {
+		if !hasDocComment(doc) {
 			doc = w.lastGen.Doc
 		}
 		w.lintTypeDoc(v, doc)
@@ -437,7 +446,7 @@ func (w *lintExported) lintInterfaceMethod(typeName string, m *ast.Field) {
 		return
 	}
 	name := m.Names[0].Name
-	if !hasTextComment(m.Doc) {
+	if !hasDocComment(m.Doc) {
 		w.onFailure(lint.Failure{
 			Node:       m,
 			Confidence: 1,

test/exported_test.go

@@ -43,3 +43,7 @@ func TestCheckDisablingOnDeclarationTypes(t *testing.T) {
 func TestCheckDirectiveComment(t *testing.T) {
 	testRule(t, "exported_issue_1202", &rule.ExportedRule{}, &lint.RuleConfig{})
 }
+
+func TestCheckDeprecationComment(t *testing.T) {
+	testRule(t, "exported_issue_1231", &rule.ExportedRule{}, &lint.RuleConfig{})
+}

testdata/exported_issue_1231.go

@@ -0,0 +1,53 @@
+package golint
+
+// Deprecated: this is deprecated, use math.PI instead
+const PI = 3.14 // MATCH /exported const PI should have comment or be unexported/
+
+// Deprecated: this is deprecated
+var Buffer []byte // MATCH /exported var Buffer should have comment or be unexported/
+
+// Eq returns true if a == b, false otherwise.
+// Deprecated: this is deprecated
+func Eq(a, b int) bool {
+	return a == b
+}
+
+// Deprecated: this is deprecated, use min instead
+// Min returns a if a <= b, b otherwise.
+func Min(a, b int) int { // MATCH /exported function Min should have comment or be unexported/
+	if a < b {
+		return a
+	}
+	return b
+}
+
+// Maximum returns a if a >= b, b otherwise.
+// Deprecated: this is deprecated, use max instead
+func Max(a, b int) int { // MATCH:24 /comment on exported function Max should be of the form "Max ..."/
+	if a > b {
+		return a
+	}
+	return b
+}
+
+// Deprecated: this is deprecated
+type Number float64 // MATCH /exported type Number should have comment or be unexported/
+
+// Name is a type that represents a name.
+type Name string
+
+// Greet returns a greeting for the name.
+func (n Name) Greet() string {
+	return "Hello, " + string(n)
+}
+
+// Deprecated: this is deprecated, use Name.ToString instead
+func (n Name) ToString() string { // MATCH /exported method Name.ToString should have comment or be unexported/
+	return string(n)
+}
+
+// String returns the string representation of the name.
+// Deprecated: this is deprecated, use Name.Greet instead
+func (n Name) String() string {
+	return string(n)
+}