tolowerequalfold linter: false negative when ToLower/ToUpper result is stored in a local variable before comparison

[github-actions]

Overview

The tolowerequalfold linter was enforced in CI this week (.github/workflows/cgo.yml:1078, the 12th LINTER_FLAGS entry) after its 13 direct-call violation sites were converted to strings.EqualFold. However, the linter only matches comparisons where an operand is a direct strings.ToLower(...) / strings.ToUpper(...) call. When the lowercased result is first stored in a local variable, the subsequent == / != comparison is not detected — so the same case-insensitive idiom the linter targets can pass CI unflagged.

This is a completeness/precision gap in an already-enforced linter, demonstrated by inconsistent idioms left in-tree because the linter could not see them.

Root cause

In pkg/linters/tolowerequalfold/tolowerequalfold.go, run() inspects only BinaryExpr nodes and relies on caseConvArg (lines 78-101), which returns true only for a *ast.CallExpr to strings.ToLower/ToUpper. An *ast.Ident holding the result of such a call is never recognized, so lower == "x" is invisible.

Evidence (4 files, 5 sites)

File:line	Comparison	Source of the var
pkg/parser/yaml_import.go:67	lower == "copilot-setup-steps.yml" || lower == "copilot-setup-steps.yaml"	lower := strings.ToLower(base) (:66)
pkg/workflow/features.go:27	if flagLower == "inline-agents"	flagLower := strings.ToLower(strings.TrimSpace(...)) (:19)
pkg/workflow/runs_on_validation.go:52	lower == "macos"	lower := strings.ToLower(label) (:51)
pkg/workflow/error_recovery.go:221	lowerField == "engine"	lowerField := strings.ToLower(field) (:217)

Smoking-gun: same-file inconsistency

• yaml_import.go was edited to convert the direct-call comparison at line 39 to strings.EqualFold(base, "action.yml"), but left the var-based comparison at line 67 as lower == "..." — the linter could not flag it.
• features.go already uses strings.EqualFold at lines 69 and 105, yet retains flagLower == "inline-agents" at line 27 in the same file.

These are functionally equivalent ASCII-keyword case-insensitive compares; converting them to EqualFold is semantics-preserving.

Recommendation

Extend the linter with a conservative one-level alias check (single-file, no cross-package dataflow):

1. Within each function body, collect local objects whose only definition is a single v := strings.ToLower/ToUpper(<expr>) assignment.
2. In the BinaryExpr check, treat an operand *ast.Ident resolving to such an object the same as a direct call.
3. To keep false-positive risk near zero, scope the new rule to comparisons where the other operand is a string literal (*ast.BasicLit). All 5 evidence sites match this shape, and a literal operand cannot trigger the self-compare case the existing sameOperand guard (lines 103-113) protects against.

Validation checklist

• okExamples testdata: self-compare v := strings.ToLower(x); _ = v == x still NOT flagged.
• //want testdata: v := strings.ToLower(x); _ = v == "lit" IS flagged.
• Convert the 5 evidence sites to strings.EqualFold(...) so enforced CI stays green.
• make golint-custom LINTER_FLAGS="-tolowerequalfold" reports the 5 sites before fix, zero after.

Effort

Small–medium: single-file change in tolowerequalfold.go + 5 trivial call-site conversions + testdata. Matches the proven single-file linter-precision pattern (sg24a1 R25, sg27a1 R28).

References: §27083452959

Generated by 🤖 Sergo - Serena Go Expert · 437.4 AIC · ⌖ 14.9 AIC · ⊞ 6.5K · ◷

• expires on Jun 14, 2026, 5:27 AM UTC