Repository: go-critic/go-critic (go)

Issue:
Check if deprecation comment is in a seperate paragraph.
The Go documentation states that a deprecated comment should be in a separate paragraph:

> To signal that an identifier should not be used, add a paragraph to its doc comment that begins with Deprecated: followed by some information about the deprecation, and a recommendation on what to use instead, if applicable. The paragraph does not have to be the last paragraph in the doc comment.

However, the go-critic's `deprecatedComment` checker is not detecting this issue, which prevents users from identifying old code that will be removed in the near future and preparing to make necessary changes. Therefore, I would like to request that you implement this check as well.

Pull request:
[checkers/deprecatedComment] Check that deprecated convention appears in a separate paragraph
Deprecation comments need to [be on their own paragraph](https://go.dev/wiki/Deprecated):

> To signal that an identifier should not be used, **add a paragraph** to its doc comment that begins with Deprecated: followed by some information about the deprecation, and a recommendation on what to use instead, if applicable.

Tools like the `go doc` generator and `staticcheck` explicitly [check for `Deprecated: ` only at the beginning of new paragraphs](https://github.com/dominikh/go-tools/blob/a2bad015c062e0d4ffabc65eb948e76acc395f0d/analysis/facts/deprecated/deprecated.go#L39), and it is a common mistake to write code like this (incorrect!):

```go
// Foo does something.
// Deprecated: use Bar instead.
func Foo() {}
```

Instead of the correct:

```go
// Foo does something.
//
// Deprecated: use Bar instead.
func Foo() {}
```

To check for this, this PR updates the `deprecatedComment` checker to error out if all of the following hold:
1. A comment line starts with 'Deprecated: ' (case sensitive),
2. This is not the first line of the comment group and
3. The previous line contains characters other than whitespace.

Note that some 'negative tests' were actually instances of this issue.

Fixes #1453


Patch:
diff --git a/checkers/deprecatedComment_checker.go b/checkers/deprecatedComment_checker.go
index c61d773..bb41d47 100644
--- a/checkers/deprecatedComment_checker.go
+++ b/checkers/deprecatedComment_checker.go
@@ -8,6 +8,8 @@ import (
 	"github.com/go-critic/go-critic/linter"
 )
 
+const deprecatedPrefix = "Deprecated: "
+
 func init() {
 	var info linter.CheckerInfo
 	info.Name = "deprecatedComment"
@@ -91,20 +93,25 @@ func (c *deprecatedCommentChecker) VisitDocComment(doc *ast.CommentGroup) {
 	//
 	// TODO(quasilyte): there are also multi-line deprecation comments.
 
+	// prev stores the previous line after it was trimmed.
+	// It's used to check whether the deprecation prefix is at the beginning of a new paragraph.
+	var prev string
+
 	for _, comment := range doc.List {
 		if strings.HasPrefix(comment.Text, "/*") {
 			// TODO(quasilyte): handle multi-line doc comments.
 			continue
 		}
-		l := comment.Text[len("//"):]
-		if len(l) < len("Deprecated: ") {
+		rawLine := strings.TrimPrefix(comment.Text, "//")
+		l := strings.TrimSpace(rawLine)
+		if len(rawLine) < len(deprecatedPrefix) {
+			prev = l
 			continue
 		}
-		l = strings.TrimSpace(l)
 
 		// Check whether someone messed up with a prefix casing.
 		upcase := strings.ToUpper(l)
-		if strings.HasPrefix(upcase, "DEPRECATED: ") && !strings.HasPrefix(l, "Deprecated: ") {
+		if strings.HasPrefix(upcase, "DEPRECATED: ") && !strings.HasPrefix(l, deprecatedPrefix) {
 			c.warnCasing(comment, l)
 			return
 		}
@@ -134,6 +141,12 @@ func (c *deprecatedCommentChecker) VisitDocComment(doc *ast.CommentGroup) {
 				return
 			}
 		}
+
+		if strings.HasPrefix(l, deprecatedPrefix) && prev != "" {
+			c.warnParagraph(comment)
+			return
+		}
+		prev = l
 	}
 }
 
@@ -154,3 +167,7 @@ func (c *deprecatedCommentChecker) warnTypo(cause ast.Node, line string) {
 	word := strings.Split(line, ":")[0]
 	c.ctx.Warn(cause, "typo in `%s`; should be `Deprecated`", word)
 }
+
+func (c *deprecatedCommentChecker) warnParagraph(cause ast.Node) {
+	c.ctx.Warn(cause, "`Deprecated: ` notices should be in a dedicated paragraph, separated from the rest")
+}


Test patch:
diff --git a/checkers/testdata/deprecatedComment/negative_tests.go b/checkers/testdata/deprecatedComment/negative_tests.go
index 4198922..7c7c364 100644
--- a/checkers/testdata/deprecatedComment/negative_tests.go
+++ b/checkers/testdata/deprecatedComment/negative_tests.go
@@ -35,6 +35,7 @@ var (
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
 // ComponentStatus (and ComponentStatus) holds the cluster validation info.
+//
 // Deprecated: This API is deprecated in v1.19+
 type ComponentStatus struct {
 	foo string
@@ -46,6 +47,7 @@ type ComponentStatus struct {
 }
 
 // ComponentStatusList represents the list of component statuses
+//
 // Deprecated: This API is deprecated in v1.19+
 type ComponentStatusList struct {
 	string
@@ -54,3 +56,15 @@ type ComponentStatusList struct {
 
 	Items []ComponentStatus
 }
+
+// SomethingOlder is old
+//
+// Deprecated: This API is deprecated in v1.19+
+//
+// and these are information that are in another paragraph, but not part of the deprecation one
+type SomethingOlder struct{}
+
+// Deprecated: This API is deprecated in v1.19+
+//
+// SomethingLegacy is also old
+type SomethingLegacy struct{}
diff --git a/checkers/testdata/deprecatedComment/positive_tests.go b/checkers/testdata/deprecatedComment/positive_tests.go
index 28d77ea..055ced3 100644
--- a/checkers/testdata/deprecatedComment/positive_tests.go
+++ b/checkers/testdata/deprecatedComment/positive_tests.go
@@ -206,3 +206,8 @@ type ComponentStatusListBad struct {
 
 	Items []int
 }
+
+// This is a sentence.
+/*! `Deprecated: ` notices should be in a dedicated paragraph, separated from the rest */
+// Deprecated: use something else.
+func InSameParagraphAsPreviousSentence() {}