diff --git a/rule/exported.go b/rule/exported.go index 503e25a..8585f40 100644 --- a/rule/exported.go +++ b/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, diff --git a/test/exported_test.go b/test/exported_test.go index 6b35cbc..e517fc6 100644 --- a/test/exported_test.go +++ b/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{}) +} diff --git a/testdata/exported_issue_1231.go b/testdata/exported_issue_1231.go new file mode 100644 index 0000000..ce7ccab --- /dev/null +++ b/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) +}