Summary
tolowerequalfold (pkg/linters/tolowerequalfold/tolowerequalfold.go) flags a comparison when either operand is a strings.ToLower/ToUpper call and rewrites it to strings.EqualFold(...). The trigger (lines 67-69) checks that a side is a case-conversion call but never checks that the other operand's case is compatible with that conversion. As a result the analyzer both false-positives and, worse, emits a suggested fix that silently changes runtime behavior for two shapes:
A. Case-mismatched string literal
strings.ToLower(a) == "HELLO" // ToLower output is always lowercase => ALWAYS FALSE
is rewritten to
strings.EqualFold(a, "HELLO") // TRUE whenever a case-insensitively equals "hello"
The original is a constant false; the fix is not. Symmetrically for strings.ToUpper(a) == "lower".
B. Mixed ToLower / ToUpper
strings.ToLower(a) == strings.ToUpper(b) // lower(a) == upper(b): false for any letters
is rewritten to
strings.EqualFold(a, b) // real case-insensitive equality
Concrete: a="Foo", b="foo" → original "foo" == "FOO" = false, fix EqualFold("Foo","foo") = true.
The diagnostic message (tolowerequalfold.go:76) says "use strings.EqualFold ... instead", presenting the fix as an equivalence-preserving refactor. It is not equivalent in these cases.
Root cause
buildEqualFoldFix (lines 90-129) and the trigger (lines 67-69) only require one side to be a caseConvArg (ToLower/ToUpper call). Nothing verifies:
- that a string-literal operand is already all-lowercase (for
ToLower) / all-uppercase (for ToUpper), nor
- that when both sides are conversions they use the same function.
Equivalence of ToLower(x) == LIT and EqualFold(x, LIT) holds only when LIT == strings.ToLower(LIT) (ASCII); for ToUpper only when LIT == strings.ToUpper(LIT); and the two-conversion form is equivalent only when both are the same case function.
The golden masks it
tolowerequalfold_test.go uses analysistest.RunWithSuggestedFixes, but every fixture comparison is deliberately case-matched (testdata/src/tolowerequalfold/tolowerequalfold.go):
:10 strings.ToLower(name) == "alice" // lowercase literal (safe)
:11 strings.ToUpper(name) == "ALICE" // uppercase literal (safe)
:12 "alice" == strings.ToLower(name) // lowercase literal (safe)
Line 11 shows the author knows ToUpper must pair with an uppercase literal — the case-matching requirement is encoded in the fixtures but not enforced in the analyzer. There is no fixture for ToLower(x) == "UPPER", ToUpper(x) == "lower", or ToLower(a) == ToUpper(b), so the behavior change is never observed.
Distinct from prior tolowerequalfold issues
#36841 handled the ToLower(x) == x self-comparison FP (now guarded by sameOperand, lines 54-64/284-294). #40580 handled syntactic pkg-matching (now astutil.IsPkgSelector). This is a different defect: case-compatibility of the other operand.
Impact
- Applying the suggested fix in bulk (
-fix / CI autofix) silently converts always-false comparisons into live case-insensitive checks — a semantic change presented as a cosmetic refactor. Most dangerous when the original always-false comparison was itself the latent bug: the fix masks it instead of surfacing it.
- False-positive diagnostics on comparisons that are not case-insensitive equality at all.
Recommendation
Only flag/emit a fix when equivalence is guaranteed:
- String-literal operand: require the literal already equal its own
ToLower (when paired with ToLower) or ToUpper (when paired with ToUpper); otherwise skip (or emit a different diagnostic: "this comparison is always false").
- Two conversions: require both operands use the same case function before rewriting to
EqualFold.
- Add fixtures for
ToLower(x) == "UPPER", ToUpper(x) == "lower", and ToLower(a) == ToUpper(b) asserting no diagnostic (goldens unchanged), locking in behavior preservation.
Validation checklist
Effort: Small — a case-compatibility guard in the trigger (a literal check + a same-function check) plus three negative fixtures.
Generated by 🤖 Sergo - Serena Go Expert · 333.5 AIC · ⌖ 11.7 AIC · ⊞ 5.8K · ◷
Summary
tolowerequalfold(pkg/linters/tolowerequalfold/tolowerequalfold.go) flags a comparison when either operand is astrings.ToLower/ToUppercall and rewrites it tostrings.EqualFold(...). The trigger (lines 67-69) checks that a side is a case-conversion call but never checks that the other operand's case is compatible with that conversion. As a result the analyzer both false-positives and, worse, emits a suggested fix that silently changes runtime behavior for two shapes:A. Case-mismatched string literal
is rewritten to
The original is a constant
false; the fix is not. Symmetrically forstrings.ToUpper(a) == "lower".B. Mixed ToLower / ToUpper
is rewritten to
Concrete:
a="Foo", b="foo"→ original"foo" == "FOO"= false, fixEqualFold("Foo","foo")= true.The diagnostic message (
tolowerequalfold.go:76) says "use strings.EqualFold ... instead", presenting the fix as an equivalence-preserving refactor. It is not equivalent in these cases.Root cause
buildEqualFoldFix(lines 90-129) and the trigger (lines 67-69) only require one side to be acaseConvArg(ToLower/ToUppercall). Nothing verifies:ToLower) / all-uppercase (forToUpper), norEquivalence of
ToLower(x) == LITandEqualFold(x, LIT)holds only whenLIT == strings.ToLower(LIT)(ASCII); forToUpperonly whenLIT == strings.ToUpper(LIT); and the two-conversion form is equivalent only when both are the same case function.The golden masks it
tolowerequalfold_test.gousesanalysistest.RunWithSuggestedFixes, but every fixture comparison is deliberately case-matched (testdata/src/tolowerequalfold/tolowerequalfold.go):Line 11 shows the author knows
ToUppermust pair with an uppercase literal — the case-matching requirement is encoded in the fixtures but not enforced in the analyzer. There is no fixture forToLower(x) == "UPPER",ToUpper(x) == "lower", orToLower(a) == ToUpper(b), so the behavior change is never observed.Distinct from prior tolowerequalfold issues
#36841 handled the
ToLower(x) == xself-comparison FP (now guarded bysameOperand, lines 54-64/284-294). #40580 handled syntactic pkg-matching (nowastutil.IsPkgSelector). This is a different defect: case-compatibility of the other operand.Impact
-fix/ CI autofix) silently converts always-false comparisons into live case-insensitive checks — a semantic change presented as a cosmetic refactor. Most dangerous when the original always-false comparison was itself the latent bug: the fix masks it instead of surfacing it.Recommendation
Only flag/emit a fix when equivalence is guaranteed:
ToLower(when paired withToLower) orToUpper(when paired withToUpper); otherwise skip (or emit a different diagnostic: "this comparison is always false").EqualFold.ToLower(x) == "UPPER",ToUpper(x) == "lower", andToLower(a) == ToUpper(b)asserting no diagnostic (goldens unchanged), locking in behavior preservation.Validation checklist
strings.ToLower(x) == "UPPER"→ no diagnostic (or an always-false diagnostic with no EqualFold fix).strings.ToUpper(x) == "lower"→ no diagnostic.strings.ToLower(a) == strings.ToUpper(b)→ no diagnostic / no fix.Effort: Small — a case-compatibility guard in the trigger (a literal check + a same-function check) plus three negative fixtures.