trimleftright: the "repeated rune" cutset heuristic is an arbitrary discriminator that misses the most common TrimLeft/TrimRight [Content truncated due to length]

[github-actions]

Summary

trimleftright (new 53rd registered analyzer, pkg/linters/trimleftright/trimleftright.go) exists to catch the well-known Go gotcha where strings.TrimLeft(s, "foo") is used when strings.TrimPrefix(s, "foo") was intended (TrimLeft treats the second arg as a character set, not a prefix). But its trigger heuristic looksSuspiciousCutset only fires when the cutset contains a repeated rune, which systematically misses the most common form of the bug: short, distinct-character prefixes/suffixes such as "http", "https", "bar", "abc", "0x", "v1", "github", "prefix".

Evidence

pkg/linters/trimleftright/trimleftright.go:125-146 — looksSuspiciousCutset returns true only if some rune repeats:

func looksSuspiciousCutset(cutset string) bool {
	runes := []rune(cutset)
	if len(runes) <= 1 { return false }
	for _, r := range runes { // must be ALL alphanumeric
		if !unicode.IsLetter(r) && !unicode.IsDigit(r) { return false }
	}
	seen := make(map[rune]struct{})
	for _, r := range runes {
		if _, ok := seen[r]; ok { return true } // <-- only fires on a REPEATED rune
		seen[r] = struct{}{}
	}
	return false
}

The linter's own testdata (pkg/linters/trimleftright/testdata/src/trimleftright/trimleftright.go:46-49) encodes the gap as intended behavior:

// good: alphanumeric literal with no repeated runes
func goodNoRepeatedAlnum(s string) string {
	return strings.TrimLeft(s, "abc") // NOT flagged
}

Yet strings.TrimLeft(s, "abc") is exactly the confusion the analyzer is meant to detect: it strips any leading run of a/b/c characters, almost never what someone writing "abc" intends.

Why the heuristic is the wrong axis

The package doc-comment justifies the narrowing as avoiding false positives on "intentional character-set trimming like whitespace classes" (trimleftright.go:81-83). But whitespace and punctuation are already excluded by the all-alphanumeric guard (trimleftright.go:131-135). So the repeated-rune requirement contributes nothing toward the stated FP-avoidance goal; it only removes true positives.

Crucially, "has a repeated rune" does not correlate with "is a word/token rather than a character class":

cutset	repeated rune?	flagged?	likely intent
"foo"	yes (o)	✅	prefix bug (correctly caught)
"http"	no	❌	prefix bug (missed)
"bar" / "abc"	no	❌	prefix bug (missed)
"0x" / "v1"	no	❌	prefix bug (missed)
"aeiou"	no	❌	intentional class (correctly ignored)
"0123456789"	no	❌	intentional class (correctly ignored)

The filter drops both distinct-char classes (good) and distinct-char prefixes (bad) together, because it cannot tell them apart on the repeated-rune axis. The short, distinct-character prefixes it misses are precisely the highest-frequency real-world instances of this bug.

Impact

• False negatives on the linter's core case. Any codebase that mistakenly writes TrimLeft(url, "http") / TrimRight(name, ".go")-style distinct-char cutsets is silently unprotected.
• Latent only: trimleftright is not in the CI enforce list (.github/workflows/cgo.yml:1208), so this does not regress CI today — but it undercuts the analyzer's value if/when it is enforced.

This matches the recurring pattern-too-narrow vein previously filed for other analyzers (e.g. #40244 errstringmatch, #40434 sprintferrdot, #40581 lenstringzero), where a linter matches only a subset of an equivalent family.

Recommendation

Replace the repeated-rune proxy with a discriminator that actually separates "intentional character class" from "probable prefix/suffix":

1. Invert the logic — flag all multi-rune alphanumeric cutsets except recognized character-class idioms (contiguous rune ranges, known classes such as hex 0123456789abcdefABCDEF, digit runs, vowel sets). This catches "http"/"bar"/"abc" while still ignoring "aeiou"/"0123456789"/"abcdef".
2. Or, if a conservative scope is deliberate, document the limitation explicitly in the analyzer doc and rename the internal helper away from the misleading "suspicious" framing, and update testdata to record "abc" as a known uncovered case rather than a good case.

Validation checklist

• Add testdata cases for distinct-char prefixes: TrimLeft(s, "http"), TrimLeft(s, "abc"), TrimRight(s, "txt") → expect a diagnostic.
• Keep negative cases for genuine classes: TrimLeft(s, "aeiou"), TrimLeft(s, "0123456789"), TrimLeft(s, "abcdef") → expect no diagnostic.
• Preserve single-rune (" ", "→") and empty-cutset no-ops.
• Re-run go test ./pkg/linters/trimleftright/....

Effort: small–medium (single-file heuristic change + testdata).

References: §29673838676

Generated by 🤖 Sergo - Serena Go Expert · 326.4 AIC · ⌖ 11.1 AIC · ⊞ 5.8K · ◷

• expires on Jul 25, 2026, 9:04 PM UTC-08:00

[github-actions]

🍪 Issue Monster selected this for Copilot

I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · 2.35 AIC · ⌖ 4.83 AIC · ⊞ 6.8K · ◷