Skip to content

tolowerequalfold precision: strings.ToLower/ToUpper matched by identifier name "strings" — shadowing FP + alias-import FN in a C [Content truncated due to length] #40580

Description

@github-actions

Summary

pkg/linters/tolowerequalfold is a CI-enforced linter, but its package-identity check matches the strings package by the syntactic identifier name rather than by type information. This is the same syntactic_stdlib_match antipattern already migrated to astutil.IsPkgSelector in sortslice (#38029), rawloginlib (#39981), and regexpcompileinfunction (#39733), and currently tracked for the CI-enforced trio in #40243 (osexitinlibrary / fprintlnsprintf / errstringmatch).

tolowerequalfold was missed by the #40243 sweep — it is a distinct, additional CI-enforced holdout.

Location

pkg/linters/tolowerequalfold/tolowerequalfold.go:255 (in caseConvArg):

ident, ok := sel.X.(*ast.Ident)
if !ok {
    return nil, false
}
if ident.Name != "strings" {   // <-- syntactic name match, no TypesInfo
    return nil, false
}
if sel.Sel.Name != "ToLower" && sel.Sel.Name != "ToUpper" {
    return nil, false
}

Both detection paths funnel through this function: isCaseConvCall (line 212) delegates directly to caseConvArg, so line 255 is the single chokepoint for the whole analyzer.

Why this is wrong

  • False positive (shadowing): a local variable, parameter, or field named strings whose type has ToLower/ToUpper methods is falsely flagged as the stdlib strings package.
  • False negative (alias import): import str "strings"str.ToLower(x) escapes detection entirely.

The linter already has full type information available and uses it correctly elsewhere — sameOperand at line 271 calls pass.TypesInfo.ObjectOf. Only the package-identity check was left syntactic.

Recommendation

Replace the manual *ast.Ident extraction + ident.Name != "strings" check (lines 251–257) with the canonical helper, which is already imported (pkg/linters/internal/astutil, line 15):

sel, ok := call.Fun.(*ast.SelectorExpr)
if !ok {
    return nil, false
}
if !astutil.IsPkgSelector(pass, sel, "strings") {
    return nil, false
}
if sel.Sel.Name != "ToLower" && sel.Sel.Name != "ToUpper" {
    return nil, false
}

Impact / current state

Latent: a repo-wide grep shows no aliased strings import and no shadowing strings variable in production today, so there is no live miss right now. The fix is a consistency / future-proofing change that aligns the last CI-enforced holdout with the team's active migration, preventing a silent build pass/fail divergence the moment such code is introduced.

Validation checklist

  • Add a testdata case with import str "strings"; str.ToLower(a) == str.ToLower(b) → expect diagnostic (FN fix).
  • Add a testdata case with a shadowing local strings value type exposing ToLower → expect no diagnostic (FP fix).
  • Existing testdata still passes (go test ./pkg/linters/tolowerequalfold/...).
  • Re-grep pkg/linters for Name (==|!=) "strings" → only intended sites remain.

Effort

Small — single-file, ~6 lines, helper already imported, existing test scaffold.

Not a duplicate of #40243 (which enumerates osexitinlibrary, fprintlnsprintf, errstringmatch) or #40435 (fileclosenotdeferred, contextcancelnotdeferred). tolowerequalfold is a separate CI-enforced analyzer.

Generated by 🤖 Sergo - Serena Go Expert · 276.1 AIC · ⌖ 11 AIC · ⊞ 5.8K ·

  • expires on Jun 27, 2026, 9:30 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions