trimleftright: widen heuristic to catch distinct-character prefix/suffix bugs

docs/adr/46465-add-trimleftright-linter.md

@@ -12,7 +12,13 @@
 
 ### Decision
 
-We will add a new `trimleftright` analyzer at `pkg/linters/trimleftright/` with a conservative heuristic: flag `strings.TrimLeft`/`strings.TrimRight` only when the cutset is a string literal that is (1) multi-rune, (2) fully alphanumeric, and (3) contains at least one repeated rune (for example `"foo"` or `"barr"`). This sharply reduces false positives for idiomatic character-set trimming such as whitespace classes (`" \t\n\r"`), punctuation sets, or unique-rune sets (`"aeiou"`).  
+We will add a new `trimleftright` analyzer at `pkg/linters/trimleftright/` with a heuristic that flags `strings.TrimLeft`/`strings.TrimRight` when the cutset is a string literal that is (1) multi-rune, (2) fully alphanumeric, and (3) does not match a recognised intentional character-class idiom. The recognised exceptions are:
+
+- **Pure decimal-digit sets** (e.g. `"0123456789"`, `"012"`) — digit-class trimming.
+- **Pure ASCII-vowel sets** (e.g. `"aeiou"`, `"aei"`) — vowel-class trimming.
+- **Complete hex-letter alphabet** (all six of `a`–`f` present, in any case, optionally mixed with digits; e.g. `"abcdef"`, `"ABCDEF"`, `"0123456789abcdef"`) — hex-class trimming.
+
+All other multi-rune alphanumeric cutsets — including common prefix/suffix bugs such as `"http"`, `"bar"`, `"abc"`, `"v1"`, `"0x"` — are flagged. This is a broader trigger than the original repeated-rune heuristic, which systematically missed the most common form of the bug (distinct-character cutsets).  
 
 The analyzer will emit diagnostics only (no `SuggestedFix`) because replacing `TrimLeft`/`TrimRight` with `TrimPrefix`/`TrimSuffix` changes semantics and cannot be proven safe from syntax alone. Generated files are excluded and callers can suppress intentional cases with `//nolint:trimleftright`. The analyzer is registered in `cmd/linters/main.go` alongside existing analyzers.
 
@@ -40,7 +46,7 @@ Not chosen because: documentation-only approaches do not catch new instances at
 #### Negative
 - Any intentional multi-character cutset usage (e.g., trimming a set of punctuation characters) must be explicitly annotated with `//nolint:trimleftright`, adding a small maintenance burden for valid usages.
 - The analyzer adds one more entry to `cmd/linters/main.go` and must be kept in sync with future refactors of the linter registry.
-- The conservative heuristic may miss some genuine mistakes (for example, literal prefixes without repeated runes such as `"bar"`).
+- Partial hex-letter subsets (e.g. `"abc"`, `"abce"`) are flagged even when the intent is genuinely hex-class trimming; such cases should be annotated with `//nolint:trimleftright`.
 
 #### Neutral
 - The linter applies only to string literals; dynamic cutset values (variables, expressions) are not flagged, which is a deliberate scope limitation—inferring intent from non-literal arguments is not feasible at analysis time.

pkg/linters/trimleftright/testdata/src/trimleftright/trimleftright.go

@@ -13,6 +13,26 @@ func badTrimRight(s string) string {
 	return strings.TrimRight(s, "barr") // want `strings\.TrimRight with a multi-character cutset`
 }
 
+// bad: distinct-char prefix — no repeated rune but still a prefix bug
+func badHTTP(s string) string {
+	return strings.TrimLeft(s, "http") // want `strings\.TrimLeft with a multi-character cutset`
+}
+
+// bad: distinct-char prefix with no repeated rune
+func badABC(s string) string {
+	return strings.TrimLeft(s, "abc") // want `strings\.TrimLeft with a multi-character cutset`
+}
+
+// bad: distinct-char suffix
+func badTXT(s string) string {
+	return strings.TrimRight(s, "txt") // want `strings\.TrimRight with a multi-character cutset`
+}
+
+// bad: mixed letter+digit prefix
+func badV1(s string) string {
+	return strings.TrimLeft(s, "v1") // want `strings\.TrimLeft with a multi-character cutset`
+}
+
 // good: TrimLeft with single character — valid cutset usage
 func goodSingleChar(s string) string {
 	return strings.TrimLeft(s, " ")
@@ -38,14 +58,24 @@ func goodUnicodeRune(s string) string {
 	return strings.TrimLeft(s, "→")
 }
 
-// good: intentional multi-character cutset
-func goodIntentionalCutset(s string) string {
+// good: intentional vowel-class cutset
+func goodVowels(s string) string {
 	return strings.TrimLeft(s, "aeiou")
 }
 
-// good: alphanumeric literal with no repeated runes
-func goodNoRepeatedAlnum(s string) string {
-	return strings.TrimLeft(s, "abc")
+// good: intentional decimal-digit class
+func goodDigits(s string) string {
+	return strings.TrimLeft(s, "0123456789")
+}
+
+// good: complete hex-letter alphabet — intentional hex-class trimming
+func goodHexLetters(s string) string {
+	return strings.TrimLeft(s, "abcdef")
+}
+
+// good: full lowercase hex alphabet including digits
+func goodFullHex(s string) string {
+	return strings.TrimLeft(s, "0123456789abcdef")
 }
 
 // suppressed: nolint directive suppresses the diagnostic

pkg/linters/trimleftright/trimleftright.go

@@ -25,7 +25,7 @@ import (
 // Analyzer is the trimleftright analysis pass.
 var Analyzer = &analysis.Analyzer{
 	Name:     "trimleftright",
-	Doc:      "reports likely mistaken strings.TrimLeft/TrimRight calls using repeated alphanumeric literal cutsets",
+	Doc:      "reports likely mistaken strings.TrimLeft/TrimRight calls using multi-character alphanumeric literal cutsets",
 	URL:      "https://github.com/github/gh-aw/tree/main/pkg/linters/trimleftright",
 	Requires: []*analysis.Analyzer{inspect.Analyzer, nolint.Analyzer, filecheck.Analyzer},
 	Run:      run,
@@ -78,9 +78,8 @@ func run(pass *analysis.Pass) (any, error) {
 			return
 		}
 
-		// Only flag suspicious cutsets where a multi-rune alphanumeric literal
-		// contains repeated runes (e.g., "foo"). This avoids flagging common,
-		// intentional character-set trimming like whitespace classes.
+		// Only flag suspicious cutsets: multi-rune all-alphanumeric literals
+		// that do not look like intentional character-set trimming.
 		cutset, isCutset := stringLitValue(call.Args[1])
 		if !isCutset || !looksSuspiciousCutset(cutset) {
 			return
@@ -119,9 +118,19 @@ func stringLitValue(expr ast.Expr) (string, bool) {
 }
 
 // looksSuspiciousCutset reports likely TrimPrefix/TrimSuffix confusion.
-// We require a multi-rune alphanumeric cutset with at least one repeated rune
-// so valid character-set trimming (whitespace, punctuation, unique rune sets)
-// is not flagged.
+// It returns true for any multi-rune all-alphanumeric cutset that does not
+// look like an intentional character-class trimmer.
+//
+// Recognised character-class exceptions (returned false):
+//   - Pure decimal-digit sets (e.g. "0123456789", "012")  — digit trimming.
+//   - Pure ASCII-vowel sets (e.g. "aeiou", "aei")         — vowel trimming.
+//   - Complete hex-letter alphabet in any case (all six of a–f present,
+//     mixed with optional digits, e.g. "abcdef", "ABCDEF",
+//     "0123456789abcdef")                                  — hex trimming.
+//
+// Single-rune cutsets and cutsets containing non-alphanumeric runes are
+// returned false because they are either trivially correct or already
+// covered by other idioms (whitespace trimming, punctuation sets).
 func looksSuspiciousCutset(cutset string) bool {
 	runes := []rune(cutset)
 	if len(runes) <= 1 {
@@ -134,13 +143,77 @@ func looksSuspiciousCutset(cutset string) bool {
 		}
 	}
 
-	seen := make(map[rune]struct{})
+	// Exception: intentional decimal-digit set.
+	if isAllASCIIDigits(runes) {
+		return false
+	}
+
+	// Exception: intentional ASCII-vowel set.
+	if isAllASCIIVowels(runes) {
+		return false
+	}
+
+	// Exception: complete hex-letter alphabet (all six of a–f in any case,
+	// optionally accompanied by decimal digits).
+	if isCompleteHexAlphabet(runes) {
+		return false
+	}
+
+	return true
+}
+
+// isAllASCIIDigits reports whether every rune is an ASCII decimal digit.
+// An empty slice returns true (vacuous truth); callers must apply the length
+// guard in looksSuspiciousCutset before invoking this helper.
+func isAllASCIIDigits(runes []rune) bool {
 	for _, r := range runes {
-		if _, ok := seen[r]; ok {
-			return true
+		if r < '0' || r > '9' {
+			return false
 		}
-		seen[r] = struct{}{}
 	}
+	return true
+}
+
+// isAllASCIIVowels reports whether every rune is an ASCII vowel (a e i o u),
+// case-insensitive.
+// An empty slice returns true (vacuous truth); callers must apply the length
+// guard in looksSuspiciousCutset before invoking this helper.
+func isAllASCIIVowels(runes []rune) bool {
+	for _, r := range runes {
+		switch unicode.ToLower(r) {
+		case 'a', 'e', 'i', 'o', 'u':
+		default:
+			return false
+		}
+	}
+	return true
+}
 
-	return false
+// isCompleteHexAlphabet reports whether runes consist solely of ASCII decimal
+// digits and/or ASCII hex letters (a–f, A–F), and include all six hex-letter
+// code points (a b c d e f, case-insensitively). This matches intentional
+// hex-class trimming like "abcdef", "ABCDEF", or "0123456789abcdef".
+// Partial hex-letter subsets such as "abc" are not matched and return false.
+// Duplicate hex letters (e.g. "aabcdef") are also rejected and return false;
+// a repeated hex letter is almost certainly a bug rather than a character class.
+func isCompleteHexAlphabet(runes []rune) bool {
+	if len(runes) > 16 { // 6 unique hex letters (a–f, case-folded to lowercase) + 10 digits
+		return false
+	}
+	seen := make(map[rune]struct{})
+	for _, r := range runes {
+		lower := unicode.ToLower(r)
+		switch {
+		case lower >= 'a' && lower <= 'f':
+			if _, ok := seen[lower]; ok {
+				return false // repeated hex letter → suspicious
+			}
+			seen[lower] = struct{}{}
+		case r >= '0' && r <= '9':
+			// decimal digits are permissible alongside hex letters
+		default:
+			return false
+		}
+	}
+	return len(seen) == 6
 }

pkg/linters/trimleftright/trimleftright_internal_test.go

@@ -10,12 +10,49 @@ func TestLooksSuspiciousCutset(t *testing.T) {
 		cutset string
 		want   bool
 	}{
+		// Trivially safe: too short or non-alphanumeric.
 		{name: "empty", cutset: "", want: false},
 		{name: "single rune", cutset: "a", want: false},
-		{name: "repeated alnum", cutset: "foo", want: true},
-		{name: "unique alnum", cutset: "aeiou", want: false},
 		{name: "contains whitespace", cutset: " \t", want: false},
 		{name: "contains punctuation", cutset: "a!", want: false},
+
+		// Suspicious: repeated rune (legacy coverage).
+		{name: "repeated alnum", cutset: "foo", want: true},
+		{name: "repeated suffix alnum", cutset: "barr", want: true},
+
+		// Suspicious: distinct-char prefixes/suffixes — the main gap fixed.
+		{name: "http prefix", cutset: "http", want: true},
+		{name: "abc prefix", cutset: "abc", want: true},
+		{name: "txt suffix", cutset: "txt", want: true},
+		{name: "bar prefix", cutset: "bar", want: true},
+		{name: "v1 version prefix", cutset: "v1", want: true},
+		{name: "mixed alpha digit", cutset: "0x", want: true},
+
+		// Safe: intentional decimal-digit class.
+		{name: "all digits", cutset: "0123456789", want: false},
+		{name: "digit subset", cutset: "012", want: false},
+
+		// Safe: intentional ASCII-vowel class.
+		{name: "all vowels", cutset: "aeiou", want: false},
+		{name: "vowel subset", cutset: "aei", want: false},
+		{name: "vowels uppercase", cutset: "AEIOU", want: false},
+
+		// Safe: complete hex-letter alphabet (all six of a–f present).
+		{name: "hex letters lower", cutset: "abcdef", want: false},
+		{name: "hex letters upper", cutset: "ABCDEF", want: false},
+		{name: "full hex with digits lower", cutset: "0123456789abcdef", want: false},
+		{name: "full hex with digits upper", cutset: "0123456789ABCDEF", want: false},
+		{name: "hex mixed case", cutset: "abcDEF", want: false},
+
+		// Suspicious: partial hex-letter subset (not the complete alphabet).
+		// "abc" is tested above via "abc prefix"; here we confirm the
+		// 4-letter subset also escapes the hex exception.
+		{name: "partial hex abce", cutset: "abce", want: true},
+		// Boundary: all six hex letters plus a non-hex character.
+		{name: "almost hex abcdefg", cutset: "abcdefg", want: true},
+		// Regression: repeated hex letters look like a bug, not a character class.
+		{name: "repeated hex letters", cutset: "aabbccddeeff", want: true},
+		{name: "hex with duplicate letter", cutset: "aabcdef", want: true},
 	}
 
 	for _, tt := range tests {