Summary
Two strings.Contains-simplification linters are type-resolved, RWSF-verified, and carry full nolint + test-skip parity, but neither is in the CI custom-linter batch (.github/workflows/cgo.yml:1208). One is enforce-ready today; the other becomes ready after a single one-line production fix.
This mirrors previously-accepted enforce-readiness issues (#42644 sprintfint, #42416, #39324, #39016) that landed once zero-violation status was established.
Findings
| Linter |
Prod violations (default build) |
RWSF golden |
IsPkgSelector |
nolint+test-skip |
Verdict |
stringsindexcontains |
0 |
yes (_test.go:15) |
yes (:184) |
yes |
enforce now |
stringscountcontains |
1 (pkg/cli/compile_update_check.go:302) |
yes (_test.go:15) |
yes (:178) |
yes |
fix 1 site, then enforce |
Audit detail — both linters are correctness-clean
Both use astutil.IsPkgSelector (no syntactic stdlib-name match — avoids the #39981/#40580 shadow/alias class), gate the literal via pass.TypesInfo constant.Int (constIntValue), reuse the actual selector qualifier (indexPkgText/countPkgText, so no alias/import-robustness bug of the #45037 class — the rewrite stays same-package strings.Contains and adds no import), and skip test files + honor nolint. The suggested-fix bodies replace the whole *ast.BinaryExpr with a call/!call (precedence-safe). Empty-substring semantics are consistent in both directions (strings.Index(s,"")==0↔Contains true; strings.Count(s,"")>=1↔Contains true), so no false-positive/false-negative edge. Pattern sets are complete for their {-1,0} / {0,1} threshold families and correctly exclude non-presence forms (Index>0, Count>1). No open sergo issue targets either linter.
The single blocking violation for stringscountcontains
// pkg/cli/compile_update_check.go:302
func hasExplicitMinorComponent(version string) bool {
core := strings.TrimPrefix(version, "v")
if idx := strings.IndexAny(core, "-+"); idx >= 0 {
core = core[:idx]
}
return strings.Count(core, ".") >= 1 // linter flags this: Count(...) >= 1
}The linter is working as designed here — strings.Count(core, ".") >= 1 is exactly the "long-way presence check" it targets. The behavior-preserving rewrite:
return strings.Contains(core, ".")
(Count(x) >= 1 ⟺ at least one occurrence ⟺ Contains(x); same for the empty-substring case since Count(s,"") >= 1 is always true and Contains(s,"") is always true.)
Recommendation
- Add
-stringsindexcontains to LINTER_FLAGS in .github/workflows/cgo.yml (both the default and the GOOS=js GOARCH=wasm invocations, lines 1208 & 1211) — it has zero violations and full suppression parity.
- Apply the one-line rewrite at
compile_update_check.go:302, then add -stringscountcontains to the same LINTER_FLAGS.
Validation checklist
Effort: small (1-line prod change + 2 CI flag additions).
Generated by 🤖 Sergo - Serena Go Expert · 369.4 AIC · ⌖ 14 AIC · ⊞ 5.8K · ◷
Summary
Two
strings.Contains-simplification linters are type-resolved, RWSF-verified, and carry fullnolint+ test-skip parity, but neither is in the CI custom-linter batch (.github/workflows/cgo.yml:1208). One is enforce-ready today; the other becomes ready after a single one-line production fix.This mirrors previously-accepted enforce-readiness issues (#42644 sprintfint, #42416, #39324, #39016) that landed once zero-violation status was established.
Findings
IsPkgSelectornolint+test-skipstringsindexcontains_test.go:15):184)stringscountcontainspkg/cli/compile_update_check.go:302)_test.go:15):178)Audit detail — both linters are correctness-clean
Both use
astutil.IsPkgSelector(no syntactic stdlib-name match — avoids the #39981/#40580 shadow/alias class), gate the literal viapass.TypesInfoconstant.Int(constIntValue), reuse the actual selector qualifier (indexPkgText/countPkgText, so no alias/import-robustness bug of the #45037 class — the rewrite stays same-packagestrings.Containsand adds no import), and skip test files + honornolint. The suggested-fix bodies replace the whole*ast.BinaryExprwith a call/!call(precedence-safe). Empty-substring semantics are consistent in both directions (strings.Index(s,"")==0↔Contains true;strings.Count(s,"")>=1↔Contains true), so no false-positive/false-negative edge. Pattern sets are complete for their{-1,0}/{0,1}threshold families and correctly exclude non-presence forms (Index>0,Count>1). No open sergo issue targets either linter.The single blocking violation for stringscountcontains
The linter is working as designed here —
strings.Count(core, ".") >= 1is exactly the "long-way presence check" it targets. The behavior-preserving rewrite:(
Count(x) >= 1⟺ at least one occurrence ⟺Contains(x); same for the empty-substring case sinceCount(s,"") >= 1is always true andContains(s,"")is always true.)Recommendation
-stringsindexcontainstoLINTER_FLAGSin.github/workflows/cgo.yml(both the default and theGOOS=js GOARCH=wasminvocations, lines 1208 & 1211) — it has zero violations and full suppression parity.compile_update_check.go:302, then add-stringscountcontainsto the sameLINTER_FLAGS.Validation checklist
make golint-custom LINTER_FLAGS="-stringsindexcontains -test=false"reports 0 findings on prod packages.compile_update_check.go:302rewritten tostrings.Contains(core, ".");TestHasExplicitMinorComponent(or equivalent) still passes.make golint-custom LINTER_FLAGS="-stringscountcontains -test=false"reports 0 findings after the fix.cgo.ymlruns.Effort: small (1-line prod change + 2 CI flag additions).