lenstringsplit: empty raw-string separator (``) escapes the non-empty guard — false positive + semantically-wrong autofix #41163

Summary

lenstringsplit correctly excludes empty separators because len(strings.Split(s, sep)) only equals strings.Count(s, sep)+1 when sep is non-empty (its own testdata documents this: "empty separators are not equivalent to strings.Count(...)+1"). But the non-empty guard's fast-path compares the raw source token text instead of the decoded string value, so an empty raw-string separator `` slips through as "provably non-empty". The linter then fires and emits a SuggestedFix whose semantics differ from the original.

Location

pkg/linters/lenstringsplit/lenstringsplit.go:104-116 — hasProvablyNonEmptySeparator:

if lit, ok := call.Args[1].(*ast.BasicLit); ok && lit.Kind == token.STRING {
    return lit.Value != `""`            // <-- compares RAW token text
}
tv, ok := pass.TypesInfo.Types[call.Args[1]]
if !ok || tv.Value == nil || tv.Value.Kind() != constant.String {
    return false
}
return constant.StringVal(tv.Value) != ""   // correct: decoded value

lit.Value is the verbatim source text. For a double-quoted empty string "", lit.Value == "\"\"", so != ""is false → correctly rejected. For an **empty raw string** (two backticks), lit.Value == "``", which is != ""`` → returns true → treated as a provably non-empty separator.

Impact

Given len(strings.Split(s, ``)) (empty raw-string separator):

• False positive: the linter reports it despite the separator being empty — the exact case it intends to exclude.
• Incorrect autofix: the suggested rewrite strings.Count(s, ``)+1 is not semantically equivalent. For an empty separator, strings.Split(s, "") returns one element per UTF-8 rune (len == RuneCount), while strings.Count(s, "")+1 == RuneCount+2. Applying the fix changes the result.

The ironic part: the general constant branch directly below (constant.StringVal(tv.Value) != "") would decode `` to "" and correctly reject it — but it is unreachable because the `*ast.BasicLit` fast-path returns first.

Why the test gap hid it

testdata/src/lenstringsplit/lenstringsplit.go:34-36 (countRunes) covers the empty-separator exclusion using only the double-quoted form strings.Split(s, ""). The empty raw-string form strings.Split(s, ``) is never exercised, so the fast-path divergence is latent.

Recommendation

Decode the literal instead of text-comparing it. Simplest fix: drop the *ast.BasicLit special-case and rely solely on the constant evaluation below, which already handles all string-constant forms (double-quoted, raw, named const, concatenation) uniformly:

func hasProvablyNonEmptySeparator(pass *analysis.Pass, call *ast.CallExpr) bool {
    if len(call.Args) != 2 {
        return false
    }
    tv, ok := pass.TypesInfo.Types[call.Args[1]]
    if !ok || tv.Value == nil || tv.Value.Kind() != constant.String {
        return false
    }
    return constant.StringVal(tv.Value) != ""
}

Validation checklist

• Add a not flagged testdata case: return len(strings.Split(s, ``)) (empty raw-string separator).
• Add a flagged testdata case with a non-empty raw-string separator, e.g. len(strings.Split(s, ,)), to confirm non-empty raw strings still report.
• Confirm existing flagged/not flagged cases (double-quoted "\n", named const comma, "") are unaffected.

Effort: small (single-file, ~10 lines + testdata).

References: §28076275497

Generated by 🤖 Sergo - Serena Go Expert · 302.6 AIC · ⌖ 11.3 AIC · ⊞ 5.9K · ◷

• expires on Jun 30, 2026, 9:12 PM UTC-08:00