feat(linters): add stringscountcontains linter

cmd/linters/main.go

@@ -53,6 +53,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/ssljson"
 	"github.com/github/gh-aw/pkg/linters/strconvparseignorederror"
 	"github.com/github/gh-aw/pkg/linters/stringreplaceminusone"
+	"github.com/github/gh-aw/pkg/linters/stringscountcontains"
 	"github.com/github/gh-aw/pkg/linters/stringsindexcontains"
 	"github.com/github/gh-aw/pkg/linters/timeafterleak"
 	"github.com/github/gh-aw/pkg/linters/timesleepnocontext"
@@ -99,6 +100,7 @@ func main() {
 		strconvparseignorederror.Analyzer,
 		stringreplaceminusone.Analyzer,
 		stringsindexcontains.Analyzer,
+		stringscountcontains.Analyzer,
 		jsonmarshalignoredeerror.Analyzer,
 		lenstringzero.Analyzer,
 		lenstringsplit.Analyzer,

pkg/linters/README.md

@@ -43,6 +43,7 @@ This package currently provides custom Go analyzers in the following subpackages
 - `ssljson` — validates `ssl.json` skill artifacts found in `.github/skills/` against the SSL spec (enum membership, graph integrity, transition targets, entry pointer validity).
 - `strconvparseignorederror` — reports `strconv` parsing calls (`Atoi`, `ParseInt`, etc.) where the error return is discarded with `_`.
 - `stringreplaceminusone` — reports `strings.Replace` calls whose `n` argument is `-1`, which should use the more readable `strings.ReplaceAll`.
+- `stringscountcontains` — reports `strings.Count(s, sub)` comparisons with `0` or `1` (e.g. `> 0`, `>= 1`, `== 0`, `!= 0`, `< 1`, `<= 0`) and their yoda-order variants that should use `strings.Contains(s, sub)` or `!strings.Contains(s, sub)` instead.
 - `stringsindexcontains` — reports `strings.Index(s, substr)` comparisons with `-1` or `0` (e.g. `!= -1`, `>= 0`, `> -1`, `== -1`, `< 0`, `<= -1`) and their yoda-order variants that should use `strings.Contains(s, substr)` or `!strings.Contains(s, substr)` instead.
 - `timeafterleak` — reports `time.After` calls used as the channel-receive expression in a `select` case inside a `for` or `range` loop that leak a timer channel on each iteration when another case fires first.
 - `timesleepnocontext` — reports `time.Sleep` calls inside functions that already receive a `context.Context`, where a context-aware `select` should be used instead.
@@ -95,6 +96,7 @@ This package currently provides custom Go analyzers in the following subpackages
 | `ssljson` | Custom `go/analysis` analyzer that validates SSL JSON skill artifacts in `.github/skills/` |
 | `strconvparseignorederror` | Custom `go/analysis` analyzer that flags `strconv` parsing calls where the error return is discarded with `_` |
 | `stringreplaceminusone` | Custom `go/analysis` analyzer that flags `strings.Replace` calls with `n=-1` that should use `strings.ReplaceAll` |
+| `stringscountcontains` | Custom `go/analysis` analyzer that flags `strings.Count(s, sub)` comparisons with `0` or `1` that should use `strings.Contains` or `!strings.Contains` |
 | `stringsindexcontains` | Custom `go/analysis` analyzer that flags `strings.Index(s, substr)` comparisons with `-1` or `0` that should use `strings.Contains` or `!strings.Contains` |
 | `timeafterleak` | Custom `go/analysis` analyzer that flags `time.After` in `select` cases inside loops that leak a timer channel on each iteration when another case fires first |
 | `timesleepnocontext` | Custom `go/analysis` analyzer that flags `time.Sleep` calls in context-aware functions |
@@ -207,6 +209,7 @@ _ = timesleepnocontext.Analyzer
 - `github.com/github/gh-aw/pkg/linters/ssljson` — ssl-json analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/strconvparseignorederror` — strconv-parse-ignored-error analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/stringreplaceminusone` — string-replace-minus-one analyzer subpackage
+- `github.com/github/gh-aw/pkg/linters/stringscountcontains` — strings-count-contains analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/stringsindexcontains` — strings-index-contains analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/timeafterleak` — time-after-leak analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/timesleepnocontext` — time-sleep-no-context analyzer subpackage

pkg/linters/spec_test.go

@@ -43,6 +43,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/ssljson"
 	"github.com/github/gh-aw/pkg/linters/strconvparseignorederror"
 	"github.com/github/gh-aw/pkg/linters/stringreplaceminusone"
+	"github.com/github/gh-aw/pkg/linters/stringscountcontains"
 	"github.com/github/gh-aw/pkg/linters/stringsindexcontains"
 	"github.com/github/gh-aw/pkg/linters/timeafterleak"
 	"github.com/github/gh-aw/pkg/linters/timesleepnocontext"
@@ -65,7 +66,7 @@ type docAnalyzer struct {
 }
 
 // documentedAnalyzers returns the analyzer subpackages documented in the README
-// "Public API > Subpackages" table. The README documents 39 analyzers
+// "Public API > Subpackages" table. The README documents 40 analyzers
 // subpackages (the non-analyzer `internal` helper subpackage is excluded because
 // it exposes no Analyzer).
 //
@@ -76,7 +77,7 @@ type docAnalyzer struct {
 //	hardcodedfilepath, httpnoctx, jsonmarshalignoredeerror, largefunc, lenstringsplit, lenstringzero,
 //	manualmutexunlock, osexitinlibrary, ossetenvlibrary, panic-in-library-code, rawloginlib,
 //	regexpcompileinfunction, seenmapbool, sortslice, sprintferrdot, sprintferrorsnew, ssljson,
-//	strconvparseignorederror, stringreplaceminusone, stringsindexcontains, timeafterleak, timesleepnocontext,
+//	strconvparseignorederror, stringreplaceminusone, stringscountcontains, stringsindexcontains, timeafterleak, timesleepnocontext,
 //	tolowerequalfold, uncheckedtypeassertion, wgdonenotdeferred, writebytestring
 func documentedAnalyzers() []docAnalyzer {
 	return []docAnalyzer{
@@ -112,6 +113,7 @@ func documentedAnalyzers() []docAnalyzer {
 		{"ssljson", ssljson.Analyzer},
 		{"strconvparseignorederror", strconvparseignorederror.Analyzer},
 		{"stringreplaceminusone", stringreplaceminusone.Analyzer},
+		{"stringscountcontains", stringscountcontains.Analyzer},
 		{"stringsindexcontains", stringsindexcontains.Analyzer},
 		{"timeafterleak", timeafterleak.Analyzer},
 		{"timesleepnocontext", timesleepnocontext.Analyzer},

pkg/linters/stringscountcontains/stringscountcontains.go

@@ -0,0 +1,220 @@
+// Package stringscountcontains implements a Go analysis linter that flags
+// strings.Count(s, sub) comparisons with 0 or 1 (e.g. > 0, >= 1, == 0,
+// != 0, < 1, <= 0) and their yoda-order variants that should use the more
+// readable strings.Contains(s, sub) or !strings.Contains(s, sub) instead.
+package stringscountcontains
+
+import (
+	"fmt"
+	"go/ast"
+	"go/constant"
+	"go/token"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+
+	"github.com/github/gh-aw/pkg/linters/internal/astutil"
+	"github.com/github/gh-aw/pkg/linters/internal/filecheck"
+	"github.com/github/gh-aw/pkg/linters/internal/nolint"
+)
+
+// Analyzer is the strings-count-contains analysis pass.
+var Analyzer = &analysis.Analyzer{
+	Name:     "stringscountcontains",
+	Doc:      "reports strings.Count(s, sub) comparisons with 0 or 1 (e.g. > 0, >= 1, == 0, != 0, < 1, <= 0) and their yoda-order variants that should use strings.Contains(s, sub) or !strings.Contains(s, sub)",
+	URL:      "https://github.com/github/gh-aw/tree/main/pkg/linters/stringscountcontains",
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+}
+
+func run(pass *analysis.Pass) (any, error) {
+	insp, err := astutil.Inspector(pass)
+	if err != nil {
+		return nil, err
+	}
+	noLintLinesByFile := nolint.BuildLineIndex(pass, "stringscountcontains")
+
+	nodeFilter := []ast.Node{(*ast.BinaryExpr)(nil)}
+
+	insp.Preorder(nodeFilter, func(n ast.Node) {
+		expr, ok := n.(*ast.BinaryExpr)
+		if !ok {
+			return
+		}
+
+		pos := pass.Fset.PositionFor(expr.Pos(), false)
+		if filecheck.IsTestFile(pos.Filename) {
+			return
+		}
+		if nolint.HasDirective(pos, noLintLinesByFile) {
+			return
+		}
+
+		countCall, negated, matched := matchCountComparison(pass, expr)
+		if !matched {
+			return
+		}
+
+		if len(countCall.Args) != 2 {
+			return
+		}
+
+		sText := astutil.NodeText(pass.Fset, countCall.Args[0])
+		subText := astutil.NodeText(pass.Fset, countCall.Args[1])
+		pkgText := countPkgText(pass, countCall)
+		if sText == "" || subText == "" || pkgText == "" {
+			return
+		}
+
+		var msg string
+		if negated {
+			msg = fmt.Sprintf("use !strings.Contains(%s, %s) instead of strings.Count comparison", sText, subText)
+		} else {
+			msg = fmt.Sprintf("use strings.Contains(%s, %s) instead of strings.Count comparison", sText, subText)
+		}
+
+		pass.Report(analysis.Diagnostic{
+			Pos:            expr.Pos(),
+			End:            expr.End(),
+			Message:        msg,
+			SuggestedFixes: buildContainsFix(pass, expr, pkgText, sText, subText, negated),
+		})
+	})
+
+	return nil, nil
+}
+
+// matchCountComparison reports whether expr is a strings.Count comparison with
+// 0 or 1 that can be replaced with strings.Contains.
+//
+// Matched patterns (contains → negated=false):
+//   - strings.Count(s, sub) > 0
+//   - strings.Count(s, sub) >= 1
+//   - strings.Count(s, sub) != 0
+//   - 0 < strings.Count(s, sub)
+//   - 1 <= strings.Count(s, sub)
+//   - 0 != strings.Count(s, sub)
+//
+// Matched patterns (not-contains → negated=true):
+//   - strings.Count(s, sub) == 0
+//   - strings.Count(s, sub) < 1
+//   - strings.Count(s, sub) <= 0
+//   - 0 == strings.Count(s, sub)
+//   - 1 > strings.Count(s, sub)
+func matchCountComparison(pass *analysis.Pass, expr *ast.BinaryExpr) (call *ast.CallExpr, negated bool, matched bool) {
+	// Normalize so the strings.Count call is on the left side.
+	left, right, flipped := normalizeOperands(pass, expr)
+
+	countCall, ok := asStringsCountCall(pass, left)
+	if !ok {
+		return nil, false, false
+	}
+
+	op := expr.Op
+	if flipped {
+		op = astutil.FlipComparisonOp(op)
+	}
+
+	litVal, ok := constIntValue(pass, right)
+	if !ok {
+		return nil, false, false
+	}
+
+	switch op {
+	case token.GTR:
+		// strings.Count(...) > 0  →  contains
+		if litVal == 0 {
+			return countCall, false, true
+		}
+	case token.GEQ:
+		// strings.Count(...) >= 1  →  contains
+		if litVal == 1 {
+			return countCall, false, true
+		}
+	case token.NEQ:
+		// strings.Count(...) != 0  →  contains
+		if litVal == 0 {
+			return countCall, false, true
+		}
+	case token.EQL:
+		// strings.Count(...) == 0  →  !contains
+		if litVal == 0 {
+			return countCall, true, true
+		}
+	case token.LSS:
+		// strings.Count(...) < 1  →  !contains
+		if litVal == 1 {
+			return countCall, true, true
+		}
+	case token.LEQ:
+		// strings.Count(...) <= 0  →  !contains
+		if litVal == 0 {
+			return countCall, true, true
+		}
+	}
+
+	return nil, false, false
+}
+
+// normalizeOperands returns (left, right) such that if the strings.Count call
+// is on the right side, the operands are swapped and flipped=true.
+func normalizeOperands(pass *analysis.Pass, expr *ast.BinaryExpr) (left, right ast.Expr, flipped bool) {
+	if _, ok := asStringsCountCall(pass, expr.X); ok {
+		return expr.X, expr.Y, false
+	}
+	return expr.Y, expr.X, true
+}
+
+// asStringsCountCall returns the *ast.CallExpr if expr is a call to strings.Count.
+func asStringsCountCall(pass *analysis.Pass, expr ast.Expr) (*ast.CallExpr, bool) {
+	call, ok := expr.(*ast.CallExpr)
+	if !ok {
+		return nil, false
+	}
+	sel, ok := call.Fun.(*ast.SelectorExpr)
+	if !ok || sel.Sel.Name != "Count" {
+		return nil, false
+	}
+	if !astutil.IsPkgSelector(pass, sel, "strings") {
+		return nil, false
+	}
+	return call, true
+}
+
+// constIntValue returns the integer constant value of expr, if it is a constant integer.
+func constIntValue(pass *analysis.Pass, expr ast.Expr) (int64, bool) {
+	tv, ok := pass.TypesInfo.Types[expr]
+	if !ok || tv.Value == nil || tv.Value.Kind() != constant.Int {
+		return 0, false
+	}
+	v, exact := constant.Int64Val(tv.Value)
+	return v, exact
+}
+
+// countPkgText returns the package selector text (e.g., "strings") from a strings.Count call.
+func countPkgText(pass *analysis.Pass, call *ast.CallExpr) string {
+	sel, ok := call.Fun.(*ast.SelectorExpr)
+	if !ok {
+		return ""
+	}
+	return astutil.NodeText(pass.Fset, sel.X)
+}
+
+// buildContainsFix builds the suggested fix rewriting the comparison to strings.Contains.
+func buildContainsFix(pass *analysis.Pass, expr *ast.BinaryExpr, pkgText, sText, subText string, negated bool) []analysis.SuggestedFix {
+	var replacement string
+	if negated {
+		replacement = "!" + pkgText + ".Contains(" + sText + ", " + subText + ")"
+	} else {
+		replacement = pkgText + ".Contains(" + sText + ", " + subText + ")"
+	}
+
+	return []analysis.SuggestedFix{{
+		Message: "Replace strings.Count comparison with strings.Contains",
+		TextEdits: []analysis.TextEdit{{
+			Pos:     expr.Pos(),
+			End:     expr.End(),
+			NewText: []byte(replacement),
+		}},
+	}}
+}

pkg/linters/stringscountcontains/stringscountcontains_test.go

@@ -0,0 +1,16 @@
+//go:build !integration
+
+package stringscountcontains_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+
+	"github.com/github/gh-aw/pkg/linters/stringscountcontains"
+)
+
+func TestAnalyzer(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.RunWithSuggestedFixes(t, testdata, stringscountcontains.Analyzer, "stringscountcontains")
+}

pkg/linters/stringscountcontains/testdata/src/stringscountcontains/stringscountcontains.go

@@ -0,0 +1,56 @@
+package stringscountcontains
+
+import "strings"
+
+func badCountGTR(s, sub string) bool {
+	return strings.Count(s, sub) > 0 // want `use strings\.Contains\(s, sub\) instead of strings\.Count comparison`
+}
+
+func badCountGEQ(s, sub string) bool {
+	return strings.Count(s, sub) >= 1 // want `use strings\.Contains\(s, sub\) instead of strings\.Count comparison`
+}
+
+func badCountNEQ(s, sub string) bool {
+	return strings.Count(s, sub) != 0 // want `use strings\.Contains\(s, sub\) instead of strings\.Count comparison`
+}
+
+func badCountEQL(s, sub string) bool {
+	return strings.Count(s, sub) == 0 // want `use !strings\.Contains\(s, sub\) instead of strings\.Count comparison`
+}
+
+func badCountLSS(s, sub string) bool {
+	return strings.Count(s, sub) < 1 // want `use !strings\.Contains\(s, sub\) instead of strings\.Count comparison`
+}
+
+func badCountLEQ(s, sub string) bool {
+	return strings.Count(s, sub) <= 0 // want `use !strings\.Contains\(s, sub\) instead of strings\.Count comparison`
+}
+
+func badYodaGTR(s, sub string) bool {
+	return 0 < strings.Count(s, sub) // want `use strings\.Contains\(s, sub\) instead of strings\.Count comparison`
+}
+
+func badYodaGEQ(s, sub string) bool {
+	return 1 <= strings.Count(s, sub) // want `use strings\.Contains\(s, sub\) instead of strings\.Count comparison`
+}
+
+func badYodaNEQ(s, sub string) bool {
+	return 0 != strings.Count(s, sub) // want `use strings\.Contains\(s, sub\) instead of strings\.Count comparison`
+}
+
+func badYodaEQL(s, sub string) bool {
+	return 0 == strings.Count(s, sub) // want `use !strings\.Contains\(s, sub\) instead of strings\.Count comparison`
+}
+
+func badYodaLSS(s, sub string) bool {
+	return 1 > strings.Count(s, sub) // want `use !strings\.Contains\(s, sub\) instead of strings\.Count comparison`
+}
+
+func goodCountActualCount(s, sub string) bool {
+	// Comparing against a value other than 0/1 (as a containment sentinel) is fine.
+	return strings.Count(s, sub) > 2
+}
+
+func goodContainsAlready(s, sub string) bool {
+	return strings.Contains(s, sub)
+}