Summary
stringsjoinone rewrites strings.Join([]string{x}, sep) to the bare element x, but it injects x's raw source text with no parentheses as a full-expression replacement of the entire call. When the element x is a compound (lower-precedence) expression and the strings.Join(...) call is itself an operand of a tighter postfix operator (index [i], slice [i:j]), the autofix silently changes semantics or produces non-compiling code.
This is the same autofix-correctness family as the already-filed #44865 (tolowerequalfold not behavior-preserving) and the fprintlnsprintf trailing-newline fix — but here the failure mode includes an outright compile error. The fix is not CI-enforced (stringsjoinone is absent from the cgo.yml LINTER_FLAGS gate), so this is a latent gap that only surfaces when a developer runs -fix.
Location
pkg/linters/stringsjoinone/stringsjoinone.go:88-96 — the SuggestedFix sets NewText: []byte(elemText), replacing call.Pos()..call.End() (the whole strings.Join(...) expression) with the unparenthesized element text.pkg/linters/stringsjoinone/stringsjoinone.go:131 — matchSingleElementStringSlice returns astutil.NodeText(pass.Fset, elem) verbatim, so any operator structure inside the element is carried over as-is.
Evidence / failing cases
Both inputs are flagged (single-element []string literal, constant separator) and rewritten by -fix:
Case A — non-compiling result. a, b are string:
first := strings.Join([]string{a + b}, "")[0]The fix replaces strings.Join([]string{a + b}, "") with a + b, yielding:
first := a + b[0] // b[0] is a byte -> string + byte -> does not compile
Case B — silent behavior change (compiles).
tail := strings.Join([]string{a + b}, "")[1:]becomes
tail := a + b[1:] // original meaning: (a+b)[1:]; now: a + (b[1:])
This compiles but returns a different string.
Root cause
The rewrite treats elemText as if it were a primary expression. strings.Join(...) is a primary expression (highest precedence), so replacing it with a bare binary expression (a + b) is only safe when the call sits in a context that does not bind tighter than the element's top-level operator. Postfix index/slice (and, for a compound element, any tighter enclosing operator) break that assumption.
Contrast the sibling simplification linters, which are correct precisely because they inject the extracted sub-expression into precedence-safe positions:
appendoneelement -> append(sliceText, elemText) (comma-delimited arg position)stringreplaceminusone -> pkg.ReplaceAll(sText, oldText, newText) (arg positions)bytesbufferstring -> receiverText + ".String()" where the receiver is always a selector operand (primary expr)
stringsjoinone is the only one doing a bare full-expression substitution, so it is the only one exposed.
Why the tests miss it
stringsjoinone_test.go uses analysistest.RunWithSuggestedFixes, but the golden only exercises simple elements (name, "hello", s) in return/assignment contexts (testdata/src/stringsjoinone/stringsjoinone.go.golden:7,12,17). No case combines a compound element with a precedence-sensitive parent (index/slice), so RWSF passes while the fix is unsound.
Recommendation
Make the replacement precedence-safe. Options, simplest first:
- Parenthesize when the element is not already atomic: if
elem is anything other than *ast.Ident, *ast.BasicLit, *ast.ParenExpr, *ast.SelectorExpr, *ast.IndexExpr, or *ast.CallExpr, emit "(" + elemText + ")". This is always behavior-preserving and cheap.
- Or gate the
SuggestedFix (keep the diagnostic, drop the fix) when the strings.Join call's parent node is an *ast.IndexExpr/*ast.SliceExpr (or more generally binds tighter than the element's root operator).
Add golden coverage for a compound element in an indexed and a sliced context to lock the fix.
Validation checklist
Effort
Small — one guard in the fix builder (~5 lines) plus two golden cases.
References: §29980457915
Generated by 🤖 Sergo - Serena Go Expert · age00 260.1 AIC · ⌖ 13.9 AIC · ⊞ 5.8K · ◷
Summary
stringsjoinonerewritesstrings.Join([]string{x}, sep)to the bare elementx, but it injectsx's raw source text with no parentheses as a full-expression replacement of the entire call. When the elementxis a compound (lower-precedence) expression and thestrings.Join(...)call is itself an operand of a tighter postfix operator (index[i], slice[i:j]), the autofix silently changes semantics or produces non-compiling code.This is the same autofix-correctness family as the already-filed #44865 (
tolowerequalfoldnot behavior-preserving) and thefprintlnsprintftrailing-newline fix — but here the failure mode includes an outright compile error. The fix is not CI-enforced (stringsjoinoneis absent from thecgo.ymlLINTER_FLAGSgate), so this is a latent gap that only surfaces when a developer runs-fix.Location
pkg/linters/stringsjoinone/stringsjoinone.go:88-96— theSuggestedFixsetsNewText: []byte(elemText), replacingcall.Pos()..call.End()(the wholestrings.Join(...)expression) with the unparenthesized element text.pkg/linters/stringsjoinone/stringsjoinone.go:131—matchSingleElementStringSlicereturnsastutil.NodeText(pass.Fset, elem)verbatim, so any operator structure inside the element is carried over as-is.Evidence / failing cases
Both inputs are flagged (single-element
[]stringliteral, constant separator) and rewritten by-fix:Case A — non-compiling result.
a,barestring:The fix replaces
strings.Join([]string{a + b}, "")witha + b, yielding:Case B — silent behavior change (compiles).
becomes
This compiles but returns a different string.
Root cause
The rewrite treats
elemTextas if it were a primary expression.strings.Join(...)is a primary expression (highest precedence), so replacing it with a bare binary expression (a + b) is only safe when the call sits in a context that does not bind tighter than the element's top-level operator. Postfix index/slice (and, for a compound element, any tighter enclosing operator) break that assumption.Contrast the sibling simplification linters, which are correct precisely because they inject the extracted sub-expression into precedence-safe positions:
appendoneelement->append(sliceText, elemText)(comma-delimited arg position)stringreplaceminusone->pkg.ReplaceAll(sText, oldText, newText)(arg positions)bytesbufferstring->receiverText + ".String()"where the receiver is always a selector operand (primary expr)stringsjoinoneis the only one doing a bare full-expression substitution, so it is the only one exposed.Why the tests miss it
stringsjoinone_test.gousesanalysistest.RunWithSuggestedFixes, but the golden only exercises simple elements (name,"hello",s) in return/assignment contexts (testdata/src/stringsjoinone/stringsjoinone.go.golden:7,12,17). No case combines a compound element with a precedence-sensitive parent (index/slice), so RWSF passes while the fix is unsound.Recommendation
Make the replacement precedence-safe. Options, simplest first:
elemis anything other than*ast.Ident,*ast.BasicLit,*ast.ParenExpr,*ast.SelectorExpr,*ast.IndexExpr, or*ast.CallExpr, emit"(" + elemText + ")". This is always behavior-preserving and cheap.SuggestedFix(keep the diagnostic, drop the fix) when thestrings.Joincall's parent node is an*ast.IndexExpr/*ast.SliceExpr(or more generally binds tighter than the element's root operator).Add golden coverage for a compound element in an indexed and a sliced context to lock the fix.
Validation checklist
-fixon Case A produces compiling code-fixon Case B preserves the original value(a+b)[1:]strings.Join([]string{a+b}, "")[0]and...[1:]Effort
Small — one guard in the fix builder (~5 lines) plus two golden cases.
References: §29980457915