Linter precision: `tolowerequalfold` false-positives on the `strings.ToLower(x) == x` case-detection idiom

[github-actions]

Summary

The newly-registered (21st) custom linter tolowerequalfold (pkg/linters/tolowerequalfold/tolowerequalfold.go, registered at cmd/linters/main.go:38,63) reports a diagnostic whenever either operand of an ==/!= comparison is a strings.ToLower(...)/strings.ToUpper(...) call, recommending strings.EqualFold. It does not check whether the other operand is the same expression being lowered. As a result it flags the common case-detection idiom strings.ToLower(x) != x ("does x contain any uppercase?"), which must not be rewritten to strings.EqualFold — EqualFold(x, x) is unconditionally true.

Applying the linter's own suggestion to one of these sites would silently break a validation guard, so this is a correctness-affecting precision gap, not just noise.

• Severity: High (linter emits a fix that introduces a bug)
• Type: Linter precision / self-consistency (single-file fix)
• Enforcement: tolowerequalfold is currently unenforced (not in cgo.yml:1040 LINTER_FLAGS); it has no //nolint support (no nolint.BuildLineIndex), so suppression is not available — the only paths are a code change or a linter fix.

False-positive sites (self-comparison idiom)

All three lower the operand and compare it to the same identifier — a "is this already lowercase?" check, not a case-insensitive comparison:

File:Line	Code	Intent
pkg/workflow/tools_validation_github.go:191	if strings.ToLower(pattern) != pattern {	reject pattern unless already lowercase
pkg/cli/logs_report_tools.go:56	hasCapital := strings.ToLower(name) != name	detect an uppercase letter
pkg/cli/codemod_discussion_trigger_categories.go:68	if typeString != strings.ToLower(typeString) {	detect non-lowercase

Why it is dangerous — at tools_validation_github.go:191 the linter's suggested fix yields strings.EqualFold(pattern, pattern), which is always true, inverting the guard and allowing non-lowercase repository patterns to pass validation.

Verification of the gap

The linter reports when lowerLeft || lowerRight regardless of the opposite operand (tolowerequalfold.go:50-56). The testdata okExamples (testdata/src/tolowerequalfold/tolowerequalfold.go:17-26) covers EqualFold, plain ==, and standalone ToLower, but not the ToLower(x) == x self-comparison — so the case is genuinely unhandled, not intentionally accepted.

Recommendation

Add a self-comparison filter to the linter: when one operand is strings.ToLower/ToUpper(E), skip the diagnostic if the other operand is structurally equal to the inner argument E (the common case is both being the same *ast.Ident / same types.Object).

Before (tolowerequalfold.go):

lowerLeft := isCaseConvCall(expr.X)
lowerRight := isCaseConvCall(expr.Y)

if lowerLeft || lowerRight {
    pass.ReportRangef(expr, "use strings.EqualFold ...")
}

After (sketch — return the inner arg and compare):

if arg, ok := caseConvArg(expr.X); ok && sameOperand(pass, arg, expr.Y) {
    return // ToLower(x) == x : case-detection idiom, not a fold comparison
}
if arg, ok := caseConvArg(expr.Y); ok && sameOperand(pass, expr.X, arg) {
    return
}
if isCaseConvCall(expr.X) || isCaseConvCall(expr.Y) {
    pass.ReportRangef(expr, "use strings.EqualFold ...")
}

where caseConvArg returns the single argument of a strings.ToLower/ToUpper call, and sameOperand compares two *ast.Idents via pass.TypesInfo.ObjectOf (covering the three sites above). A future enhancement could extend structural equality to selector/index expressions.

Validation

• Add okExamples fixtures for strings.ToLower(x) == x and strings.ToLower(x) != x (both should NOT diagnose)
• Keep existing flaggedExamples passing (genuine ToLower(x) == "literal" still diagnosed)
• Re-run the linter over pkg/ + cmd/; confirm the 3 sites above no longer flag while the ~15 genuine EqualFold opportunities (e.g. pkg/parser/yaml_import.go:39, pkg/workflow/resolve.go:173, pkg/cli/codespace.go:17) remain flagged
• make test for the linters package

Notes for enforcement

This is a precision fix only — after it lands, ~15 genuine true-positive sites remain, so tolowerequalfold is not near-zero and is not yet ready for a clean cgo.yml:1040 append. Converting those 15 sites to strings.EqualFold (plus the linter's missing //nolint support) is a separate follow-up.

Estimated effort: Small (single-file linter change + testdata).

References: §26932367410

Generated by 🤖 Sergo - Serena Go Expert · opus48 10.2M · ◷

• expires on Jun 11, 2026, 5:26 AM UTC