Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions docs/adr/44888-tolowerequalfold-case-compatibility-guard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# ADR-44888: Guard tolowerequalfold Linter Against Case-Incompatible Rewrites

**Date**: 2026-07-11
**Status**: Accepted
**Deciders**: gh-aw maintainers

---

### Context

The `tolowerequalfold` custom linter flags comparisons of the form `strings.ToLower(x) == y` and rewrites them to `strings.EqualFold(x, y)`. The original trigger condition was purely structural: it fired whenever one operand was a `strings.ToLower` or `strings.ToUpper` call (or a local variable aliasing such a call) and the other was any expression. This caused two classes of incorrect rewrites:

1. **Case-mismatched literals**: `strings.ToLower(name) == "ALICE"` is always false (the output of `ToLower` can never equal an uppercase string), so it is dead code — not a case-insensitive equality check. Rewriting it to `strings.EqualFold(name, "ALICE")` silently converts dead code into a live check.
2. **Mixed conversion functions**: `strings.ToLower(a) == strings.ToUpper(b)` is always false for any string containing letters, for the same reason. Rewriting to `EqualFold` introduces a behavioral change.

Both rewrites violate the invariant that an autofix must be behavior-preserving.

### Decision

We will make the `tolowerequalfold` rewrite **fail closed** and only permit cases we can prove equivalent. A comparison is flagged for EqualFold rewriting only when:

- one side is a `strings.ToLower`/`strings.ToUpper` call (or tracked alias), and
- the other side is an **ASCII string literal** that is already in the matching case (`ToLower` ↔ lowercase literal, `ToUpper` ↔ uppercase literal).

All other operand shapes are rejected (unknown variables, other function calls, and conversion-vs-conversion comparisons, including same-function pairs). This conservative choice avoids Unicode simple-fold edge cases (for example Greek sigma forms) where `ToLower`/`ToUpper` equality can diverge from `strings.EqualFold`.

The internal alias map type is changed from `map[types.Object]ast.Expr` to `map[types.Object]caseConvAliasInfo` to carry the function name alongside the argument, enabling alias-level guards.

### Alternatives Considered

#### Alternative 1: Suppress only mixed-case literals

Guard only against literals that contain both upper and lowercase characters (e.g., "Alice"). All-caps or all-lowercase literals paired with the wrong conversion function would still trigger the diagnostic.

Rejected because it does not cover the primary bug: `strings.ToLower(name) == "ALICE"` uses an all-uppercase literal and would still be incorrectly rewritten. The guard must be based on whether the literal's case is invariant under the conversion, not on whether it is "mixed case."

#### Alternative 2: Allow same-function conversion pairs (`ToLower(a)==ToLower(b)`)

Treat matching conversion functions as sufficient evidence of equivalence and continue rewriting those comparisons.

Rejected because Go Unicode semantics make this unsafe in general (for example, `strings.ToLower("ς")` is `"ς"` and `strings.ToLower("σ")` is `"σ"`, while `strings.EqualFold("ς", "σ")` is `true`). This would still permit behavior-changing rewrites.

### Consequences

#### Positive
- The linter no longer silently converts always-false (dead-code) comparisons into live case-insensitive checks.
- Mixed `ToLower`/`ToUpper` and same-function conversion-pair comparisons are excluded, preserving behavior across Unicode edge cases.
- Negative test fixtures lock in the new behavior and prevent regression.

#### Negative
- The rule becomes stricter and emits fewer diagnostics than before, intentionally favoring correctness over aggressiveness.
- The trigger condition is now more complex: `isEquivalentToEqualFold` delegates to compatibility helpers (`caseConvIsCompatible`, `caseConvAliasIsCompatible`, `literalCaseMatchesConv`, `stringLitValue`, `caseConvFuncAndArg`).
- The `caseConvAliasInfo` struct change is a breaking internal refactor: all functions accepting `map[types.Object]ast.Expr` must be updated to `map[types.Object]caseConvAliasInfo`.

#### Neutral
- The helper `caseConvFuncAndArg` is introduced as a single source of truth for extracting both the function name and argument from a conversion call; the existing `caseConvArg` and new `caseConvFuncName` become thin delegates to it.
- The literal guard uses ASCII-only matching so the analyzer does not need full Unicode fold-class reasoning.

---

*Finalized for PR #44888.*
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ func aliasImportExamples() {
a := "Alice"
b := "alice"

_ = str.ToLower(a) == str.ToLower(b) // want `use strings\.EqualFold`
_ = str.ToUpper(a) == str.ToUpper(b) // want `use strings\.EqualFold`
_ = str.ToLower(a) == str.ToLower(b)
_ = str.ToUpper(a) == str.ToUpper(b)
}

func aliasImportTrackedExamples() {
Expand All @@ -21,6 +21,23 @@ func aliasImportTrackedExamples() {
_ = "ALICE" == y // want `use strings\.EqualFold`
}

func aliasImportMismatchedExamples() {
a := "Alice"
b := "Bob"

// Alias with case-mismatched literal — must not be rewritten to EqualFold.
x := str.ToLower(a)
_ = x == "ALICE"

y := str.ToUpper(a)
_ = "alice" == y

// Alias-vs-alias with mismatched conversion functions must not be rewritten.
lower := str.ToLower(a)
upper := str.ToUpper(b)
_ = lower == upper
}

type shadowStrings struct{}

func (shadowStrings) ToLower(s string) string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ func aliasImportExamples() {
a := "Alice"
b := "alice"

_ = str.EqualFold(a, b) // want `use strings\.EqualFold`
_ = str.EqualFold(a, b) // want `use strings\.EqualFold`
_ = str.ToLower(a) == str.ToLower(b)
_ = str.ToUpper(a) == str.ToUpper(b)
}

func aliasImportTrackedExamples() {
Expand All @@ -21,6 +21,23 @@ func aliasImportTrackedExamples() {
_ = "ALICE" == y // want `use strings\.EqualFold`
}

func aliasImportMismatchedExamples() {
a := "Alice"
b := "Bob"

// Alias with case-mismatched literal — must not be rewritten to EqualFold.
x := str.ToLower(a)
_ = x == "ALICE"

y := str.ToUpper(a)
_ = "alice" == y

// Alias-vs-alias with mismatched conversion functions must not be rewritten.
lower := str.ToLower(a)
upper := str.ToUpper(b)
_ = lower == upper
}

type shadowStrings struct{}

func (shadowStrings) ToLower(s string) string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ func flaggedExamples() {
_ = strings.ToUpper(name) == "ALICE" // want `use strings\.EqualFold`
_ = "alice" == strings.ToLower(name) // want `use strings\.EqualFold`
_ = strings.ToLower(name) != "alice" // want `use strings\.EqualFold`
_ = strings.ToLower(name) == strings.ToLower("alice") // want `use strings\.EqualFold`

lower := strings.ToLower(name)
_ = lower == "alice" // want `use strings\.EqualFold`
Expand All @@ -31,10 +30,29 @@ func okExamples() {

lower := strings.ToLower(name)
_ = lower == name

// Case-mismatched literal: ToLower output can never equal an uppercase
// literal, so the comparison is always false — not a case-insensitive
// equality check and must not be rewritten to EqualFold.
_ = strings.ToLower(name) == "ALICE"
_ = strings.ToUpper(name) == "alice"
_ = "ALICE" == strings.ToLower(name)

// Mixed ToLower/ToUpper: lower(a)==upper(b) is false for any letters,
// not a case-insensitive equality — must not be rewritten to EqualFold.
_ = strings.ToLower(name) == strings.ToUpper(name)
_ = strings.ToLower(name) == strings.ToLower("alice")

// Alias with case-mismatched literal — same reasoning as above.
lowerName := strings.ToLower(name)
_ = lowerName == "ALICE"

// Unicode literals are conservatively excluded because ToLower/ToUpper
// equality may diverge from EqualFold semantics (e.g. Greek sigma forms).
_ = strings.ToLower(name) == "σ"
}

func suppressedExamples() {
name := "Alice"
_ = strings.ToLower(name) == "alice" //nolint:tolowerequalfold
}

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ func flaggedExamples() {
_ = strings.EqualFold(name, "ALICE") // want `use strings\.EqualFold`
_ = strings.EqualFold("alice", name) // want `use strings\.EqualFold`
_ = !strings.EqualFold(name, "alice") // want `use strings\.EqualFold`
_ = strings.EqualFold(name, "alice") // want `use strings\.EqualFold`

lower := strings.ToLower(name)
_ = lower == "alice" // want `use strings\.EqualFold`
Expand All @@ -31,6 +30,26 @@ func okExamples() {

lower := strings.ToLower(name)
_ = lower == name

// Case-mismatched literal: ToLower output can never equal an uppercase
// literal, so the comparison is always false — not a case-insensitive
// equality check and must not be rewritten to EqualFold.
_ = strings.ToLower(name) == "ALICE"
_ = strings.ToUpper(name) == "alice"
_ = "ALICE" == strings.ToLower(name)

// Mixed ToLower/ToUpper: lower(a)==upper(b) is false for any letters,
// not a case-insensitive equality — must not be rewritten to EqualFold.
_ = strings.ToLower(name) == strings.ToUpper(name)
_ = strings.ToLower(name) == strings.ToLower("alice")

// Alias with case-mismatched literal — same reasoning as above.
lowerName := strings.ToLower(name)
_ = lowerName == "ALICE"

// Unicode literals are conservatively excluded because ToLower/ToUpper
// equality may diverge from EqualFold semantics (e.g. Greek sigma forms).
_ = strings.ToLower(name) == "σ"
}

func suppressedExamples() {
Expand Down
Loading
Loading