new checker contains

Co-authored-by: ccoVeille <[email protected]>
Antonboom · Jun 25, 2024 · 09cdfc4 · 09cdfc4
1 parent cea8c9c
commit 09cdfc4
Show file tree

Hide file tree

Showing 10 changed files with 342 additions and 0 deletions.
diff --git a/README.md b/README.md
@@ -94,6 +94,7 @@ https://golangci-lint.run/usage/linters/#testifylint
 | [blank-import](#blank-import)                       | ✅                  | ❌       |
 | [bool-compare](#bool-compare)                       | ✅                  | ✅       |
 | [compares](#compares)                               | ✅                  | ✅       |
+| [contains](#contains)                               | ✅                  | ✅       |
 | [empty](#empty)                                     | ✅                  | ✅       |
 | [error-is-as](#error-is-as)                         | ✅                  | 🤏      |
 | [error-nil](#error-nil)                             | ✅                  | ✅       |
@@ -216,6 +217,28 @@ due to the inappropriate recursive nature of `assert.Equal` (based on
 
 ---
 
+### contains
+
+```go
+❌
+assert.True(t, strings.Contains(a, "abc123"))
+assert.False(t, strings.Contains(a, "456"))
+assert.True(t, strings.Contains(string(b), "abc123"))
+assert.False(t, strings.Contains(string(b), "456"))
+
+✅
+assert.Contains(t, a, "abc123")
+assert.NotContains(t, a, "456")
+assert.Contains(t, string(b), "abc123")
+assert.NotContains(t, string(b), "456")
+```
+
+**Autofix**: true. <br>
+**Enabled by default**: true. <br>
+**Reason**: More appropriate `testify` API with clearer failure message.
+
+---
+
 ### empty
 
 ```go

diff --git a/analyzer/checkers_factory_test.go b/analyzer/checkers_factory_test.go
@@ -21,6 +21,7 @@ func Test_newCheckers(t *testing.T) {
 		checkers.NewLen(),
 		checkers.NewNegativePositive(),
 		checkers.NewCompares(),
+		checkers.NewContains(),
 		checkers.NewErrorNil(),
 		checkers.NewNilCompare(),
 		checkers.NewErrorIsAs(),
@@ -38,6 +39,7 @@ func Test_newCheckers(t *testing.T) {
 		checkers.NewLen(),
 		checkers.NewNegativePositive(),
 		checkers.NewCompares(),
+		checkers.NewContains(),
 		checkers.NewErrorNil(),
 		checkers.NewNilCompare(),
 		checkers.NewErrorIsAs(),

diff --git a/analyzer/testdata/src/checkers-default/contains/contains_test.go b/analyzer/testdata/src/checkers-default/contains/contains_test.go
diff --git a/analyzer/testdata/src/checkers-default/contains/contains_test.go.golden b/analyzer/testdata/src/checkers-default/contains/contains_test.go.golden
@@ -0,0 +1,55 @@
+// Code generated by testifylint/internal/testgen. DO NOT EDIT.
+
+package contains
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestContainsChecker(t *testing.T) {
+	var (
+		a           = "abc123"
+		b           = []byte(a)
+		errSentinel = errors.New("user not found")
+	)
+
+	// Invalid.
+	{
+		assert.Contains(t, a, "abc123")                                           // want "contains: use assert\\.Contains"
+		assert.Containsf(t, a, "abc123", "msg with args %d %s", 42, "42")         // want "contains: use assert\\.Containsf"
+		assert.NotContains(t, a, "456")                                           // want "contains: use assert\\.NotContains"
+		assert.NotContainsf(t, a, "456", "msg with args %d %s", 42, "42")         // want "contains: use assert\\.NotContainsf"
+		assert.Contains(t, string(b), "abc123")                                   // want "contains: use assert\\.Contains"
+		assert.Containsf(t, string(b), "abc123", "msg with args %d %s", 42, "42") // want "contains: use assert\\.Containsf"
+		assert.NotContains(t, string(b), "456")                                   // want "contains: use assert\\.NotContains"
+		assert.NotContainsf(t, string(b), "456", "msg with args %d %s", 42, "42") // want "contains: use assert\\.NotContainsf"
+	}
+
+	// Valid.
+	{
+		assert.Contains(t, a, "abc123")
+		assert.Containsf(t, a, "abc123", "msg with args %d %s", 42, "42")
+		assert.NotContains(t, a, "456")
+		assert.NotContainsf(t, a, "456", "msg with args %d %s", 42, "42")
+		assert.Contains(t, string(b), "abc123")
+		assert.Containsf(t, string(b), "abc123", "msg with args %d %s", 42, "42")
+		assert.NotContains(t, string(b), "456")
+		assert.NotContainsf(t, string(b), "456", "msg with args %d %s", 42, "42")
+	}
+
+	// Ignored.
+	{
+		assert.Contains(t, errSentinel.Error(), "user")
+		assert.Containsf(t, errSentinel.Error(), "user", "msg with args %d %s", 42, "42")
+		assert.Equal(t, strings.Contains(a, "abc123"), true)
+		assert.Equalf(t, strings.Contains(a, "abc123"), true, "msg with args %d %s", 42, "42")
+		assert.False(t, !strings.Contains(a, "abc123"))
+		assert.Falsef(t, !strings.Contains(a, "abc123"), "msg with args %d %s", 42, "42")
+		assert.True(t, !strings.Contains(a, "456"))
+		assert.Truef(t, !strings.Contains(a, "456"), "msg with args %d %s", 42, "42")
+	}
+}
diff --git a/internal/checkers/checkers_registry.go b/internal/checkers/checkers_registry.go
@@ -13,6 +13,7 @@ var registry = checkersRegistry{
 	{factory: asCheckerFactory(NewLen), enabledByDefault: true},
 	{factory: asCheckerFactory(NewNegativePositive), enabledByDefault: true},
 	{factory: asCheckerFactory(NewCompares), enabledByDefault: true},
+	{factory: asCheckerFactory(NewContains), enabledByDefault: true},
 	{factory: asCheckerFactory(NewErrorNil), enabledByDefault: true},
 	{factory: asCheckerFactory(NewNilCompare), enabledByDefault: true},
 	{factory: asCheckerFactory(NewErrorIsAs), enabledByDefault: true},

diff --git a/internal/checkers/checkers_registry_test.go b/internal/checkers/checkers_registry_test.go
@@ -41,6 +41,7 @@ func TestAll(t *testing.T) {
 		"len",
 		"negative-positive",
 		"compares",
+		"contains",
 		"error-nil",
 		"nil-compare",
 		"error-is-as",
@@ -76,6 +77,7 @@ func TestEnabledByDefault(t *testing.T) {
 		"len",
 		"negative-positive",
 		"compares",
+		"contains",
 		"error-nil",
 		"nil-compare",
 		"error-is-as",

diff --git a/internal/checkers/contains.go b/internal/checkers/contains.go
@@ -0,0 +1,79 @@
+package checkers
+
+import (
+	"go/ast"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+// Contains detects situations like
+//
+//	assert.True(t, strings.Contains(a, "abc123"))
+//	assert.False(t, strings.Contains(a, "456"))
+//	assert.True(t, strings.Contains(string(b), "abc123"))
+//	assert.False(t, strings.Contains(string(b), "456"))
+//
+// and requires
+//
+//	assert.Contains(t, a, "abc123")
+//	assert.NotContains(t, a, "456")
+//	assert.Contains(t, string(b), "abc123")
+//	assert.NotContains(t, string(b), "456")
+type Contains struct{}
+
+// NewContains constructs Contains checker.
+func NewContains() Contains   { return Contains{} }
+func (Contains) Name() string { return "contains" }
+
+func (checker Contains) Check(pass *analysis.Pass, call *CallMeta) *analysis.Diagnostic {
+	switch call.Fn.NameFTrimmed {
+	case "True":
+		if len(call.Args) < 1 {
+			return nil
+		}
+
+		ce, ok := call.Args[0].(*ast.CallExpr)
+		if !ok {
+			return nil
+		}
+		if len(ce.Args) != 2 {
+			return nil
+		}
+
+		if isStringsContainsCall(pass, ce) {
+			const proposed = "Contains"
+			return newUseFunctionDiagnostic(checker.Name(), call, proposed,
+				newSuggestedFuncReplacement(call, proposed, analysis.TextEdit{
+					Pos:     ce.Pos(),
+					End:     ce.End(),
+					NewText: formatAsCallArgs(pass, ce.Args[0], ce.Args[1]),
+				}),
+			)
+		}
+
+	case "False":
+		if len(call.Args) < 1 {
+			return nil
+		}
+
+		ce, ok := call.Args[0].(*ast.CallExpr)
+		if !ok {
+			return nil
+		}
+		if len(ce.Args) != 2 {
+			return nil
+		}
+
+		if isStringsContainsCall(pass, ce) {
+			const proposed = "NotContains"
+			return newUseFunctionDiagnostic(checker.Name(), call, proposed,
+				newSuggestedFuncReplacement(call, proposed, analysis.TextEdit{
+					Pos:     ce.Pos(),
+					End:     ce.End(),
+					NewText: formatAsCallArgs(pass, ce.Args[0], ce.Args[1]),
+				}),
+			)
+		}
+	}
+	return nil
+}
diff --git a/internal/checkers/helpers_pkg_func.go b/internal/checkers/helpers_pkg_func.go
@@ -12,6 +12,10 @@ func isRegexpMustCompileCall(pass *analysis.Pass, ce *ast.CallExpr) bool {
 	return isPkgFnCall(pass, ce, "regexp", "MustCompile")
 }
 
+func isStringsContainsCall(pass *analysis.Pass, ce *ast.CallExpr) bool {
+	return isPkgFnCall(pass, ce, "strings", "Contains")
+}
+
 func isPkgFnCall(pass *analysis.Pass, ce *ast.CallExpr, pkg, fn string) bool {
 	se, ok := ce.Fun.(*ast.SelectorExpr)
 	if !ok {