Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimize deprecatedComment_checker: delete regexp usage #1188

Merged
merged 3 commits into from Jan 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 16 additions & 9 deletions checkers/deprecatedComment_checker.go
Expand Up @@ -2,7 +2,6 @@ package checkers

import (
"go/ast"
"regexp"
"strings"

"github.com/go-critic/go-critic/checkers/internal/astwalk"
Expand All @@ -24,12 +23,15 @@ func FuncOld() int`
collection.AddChecker(&info, func(ctx *linter.CheckerContext) (linter.FileWalker, error) {
c := &deprecatedCommentChecker{ctx: ctx}

c.commonPatterns = []*regexp.Regexp{
regexp.MustCompile(`(?i)this (?:function|type) is deprecated`),
regexp.MustCompile(`(?i)deprecated[.!]? use \S* instead`),
regexp.MustCompile(`(?i)\[\[deprecated\]\].*`),
regexp.MustCompile(`(?i)note: deprecated\b.*`),
regexp.MustCompile(`(?i)deprecated in.*`),
c.commonPatterns = []string{
"this type is deprecated",
"this function is deprecated",
"[[deprecated]]",
"note: deprecated",
"deprecated in",
"deprecated. use",
"deprecated! use",
"deprecated use",
// TODO(quasilyte): more of these?
}

Expand All @@ -41,6 +43,7 @@ func FuncOld() int`
"Dprecated: ",
"Derecated: ",
"Depecated: ",
"Depekated: ",
"Deprcated: ",
"Depreated: ",
"Deprected: ",
Expand All @@ -63,7 +66,7 @@ type deprecatedCommentChecker struct {
astwalk.WalkHandler
ctx *linter.CheckerContext

commonPatterns []*regexp.Regexp
commonPatterns []string
commonTypos []string
}

Expand Down Expand Up @@ -114,7 +117,11 @@ func (c *deprecatedCommentChecker) VisitDocComment(doc *ast.CommentGroup) {

// Check for other commonly used patterns.
for _, pat := range c.commonPatterns {
if pat.MatchString(l) {
if len(l) < len(pat) {
continue
}

if strings.EqualFold(l[:len(pat)], pat) {
c.warnPattern(comment)
return
}
Expand Down
14 changes: 14 additions & 0 deletions checkers/testdata/deprecatedComment/negative_tests.go
Expand Up @@ -6,6 +6,9 @@ func ProperDeprecationComment() {}
// This is not a Deprecated: comment at all.
func FalsePositive1() {}

// This is not a Deprecated?
func FalsePositive2() {}

// Deprecated is a function name.
func Deprecated() {}

Expand All @@ -17,3 +20,14 @@ type Derpecated struct{}

// Note that this one is not deprecated.
func f() {}

var (
// Dep: ...
_ = 0

// deprec
_ = 0

// dePreca
_ = 0
)
34 changes: 34 additions & 0 deletions checkers/testdata/deprecatedComment/positive_tests.go
Expand Up @@ -62,6 +62,16 @@ const BadFormat8 = 10
// [[deprecated]]
const BadFormat9 = 10

// BadFormat10 is an example, too.
//
/*! the proper format is `Deprecated: <text>` */
// ThIs TyPe iS DepRecateD, use foo instead.
type BadFormat10 int

/*! use `Deprecated: ` (note the casing) instead of `DEPRECATED: ` */
// DEPRECATED: part of the old API; use API v2
func BadFormat11() {}

type badNestedDoc struct {
/*! use `Deprecated: ` (note the casing) instead of `deprecated: ` */
// deprecated: ha-ha
Expand Down Expand Up @@ -121,6 +131,26 @@ var (
// Deprecate: ...
_ = 0

/*! typo in `DeprEcate`; should be `Deprecated` */
// DeprEcate: ...
_ = 0

/*! typo in `deprecate`; should be `Deprecated` */
// deprecate: ...
_ = 0

/*! typo in `dePrecate`; should be `Deprecated` */
// dePrecate: ...
_ = 0

/*! typo in `Depekated`; should be `Deprecated` */
// Depekated: ...
_ = 0

/*! typo in `DepeKated`; should be `Deprecated` */
// DepeKated: ...
_ = 0

/*! typo in `Derpecate`; should be `Deprecated` */
// Derpecate: ...
_ = 0
Expand All @@ -147,3 +177,7 @@ func foo2() {
/*! the proper format is `Deprecated: <text>` */
// deprecated in 1.8: use bar instead.
type foo3 string

/*! the proper format is `Deprecated: <text>` */
// deprecated in 1.11: use f instead.
type foo4 string