From 8f214cc023a6b017af0a059081ba26988b642762 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Jul 2026 06:18:18 +0000 Subject: [PATCH 1/5] Initial plan From 22c3880c8a8674c4ea68f4c117205f2b1a0afeed Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Jul 2026 06:33:47 +0000 Subject: [PATCH 2/5] stringsconcatloop: also flag x = x + y pattern, add loop-scope guard Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/linters/doc.go | 2 +- .../stringsconcatloop/stringsconcatloop.go | 92 ++++++++++++++++--- .../stringsconcatloop/stringsconcatloop.go | 51 ++++++++++ 3 files changed, 130 insertions(+), 15 deletions(-) diff --git a/pkg/linters/doc.go b/pkg/linters/doc.go index 4194977f888..33576a41762 100644 --- a/pkg/linters/doc.go +++ b/pkg/linters/doc.go @@ -49,7 +49,7 @@ // - strconvparseignorederror — flags strconv parsing calls where the error is discarded with _ // - stringbytesroundtrip — reports redundant string/[]byte round-trip conversions such as string([]byte(s)) or []byte(string(b)) that produce a wasteful intermediate copy // - stringreplaceminusone — flags strings.Replace calls with n=-1 that should use strings.ReplaceAll -// - stringsconcatloop — flags string += concatenation inside for/range loops that should use strings.Builder +// - stringsconcatloop — flags string += or x = x + y concatenation inside for/range loops that should use strings.Builder // - stringscountcontains — reports strings.Count(s, sub) comparisons with 0 or 1 (e.g. > 0, >= 1, == 0, != 0, < 1, <= 0) and their yoda-order variants that should use strings.Contains(s, sub) or !strings.Contains(s, sub) // - stringsindexcontains — flags strings.Index(s, substr) comparisons that should use strings.Contains // - stringsindexhasprefix — reports strings.Index(s, sub) comparisons with 0 (== 0 and != 0) and their yoda-order variants that should use strings.HasPrefix(s, sub) or !strings.HasPrefix(s, sub) diff --git a/pkg/linters/stringsconcatloop/stringsconcatloop.go b/pkg/linters/stringsconcatloop/stringsconcatloop.go index 5c605484641..ac5ec196720 100644 --- a/pkg/linters/stringsconcatloop/stringsconcatloop.go +++ b/pkg/linters/stringsconcatloop/stringsconcatloop.go @@ -1,7 +1,7 @@ // Package stringsconcatloop implements a Go analysis linter that flags -// string += concatenation inside for/range loop bodies, which allocates a new -// string on every iteration and can lead to O(n²) total allocated bytes. The -// idiomatic fix is to use strings.Builder. +// string concatenation inside for/range loop bodies using += or the equivalent +// x = x + y form, which allocates a new string on every iteration and can lead +// to O(n²) total allocated bytes. The idiomatic fix is to use strings.Builder. package stringsconcatloop import ( @@ -20,7 +20,7 @@ import ( // Analyzer is the string-concat-in-loop analysis pass. var Analyzer = &analysis.Analyzer{ Name: "stringsconcatloop", - Doc: "reports string += concatenation inside for/range loops that should use strings.Builder", + Doc: "reports string += or x = x + y concatenation inside for/range loops that should use strings.Builder", URL: "https://github.com/github/gh-aw/tree/main/pkg/linters/stringsconcatloop", Requires: []*analysis.Analyzer{inspect.Analyzer, nolint.Analyzer, filecheck.Analyzer}, Run: run, @@ -45,7 +45,37 @@ func run(pass *analysis.Pass) (any, error) { if !ok { continue } - if assign.Tok != token.ADD_ASSIGN { + + // Match both `x += y` (ADD_ASSIGN) and `x = x + y` (ASSIGN with a + // self-referential binary addition). For the latter, also capture the + // LHS identifier so the loop-scope guard can be applied. + var lhsExpr ast.Expr + var assignLhsName string // non-empty only for the token.ASSIGN form + + switch assign.Tok { + case token.ADD_ASSIGN: + lhsExpr = assign.Lhs[0] + case token.ASSIGN: + if len(assign.Lhs) != 1 || len(assign.Rhs) != 1 { + continue + } + lhsIdent, ok := assign.Lhs[0].(*ast.Ident) + if !ok { + continue + } + binExpr, ok := assign.Rhs[0].(*ast.BinaryExpr) + if !ok || binExpr.Op != token.ADD { + continue + } + // Only match the direct self-referential form: x = x + rhs, + // where x is the same identifier on both sides. + rhsLeft, ok := binExpr.X.(*ast.Ident) + if !ok || rhsLeft.Name != lhsIdent.Name { + continue + } + lhsExpr = lhsIdent + assignLhsName = lhsIdent.Name + default: continue } @@ -54,7 +84,7 @@ func run(pass *analysis.Pass) (any, error) { continue } - loopPos, inLoop := enclosingLoopPosition(pass, cur) + loopPos, loopNode, inLoop := enclosingLoop(pass, cur) if !inLoop { continue } @@ -65,7 +95,14 @@ func run(pass *analysis.Pass) (any, error) { continue } - if !astutil.IsStringType(pass, assign.Lhs[0]) { + if !astutil.IsStringType(pass, lhsExpr) { + continue + } + + // For the x = x + y form, skip variables declared by the enclosing + // loop itself (range key/value or for-init short-decl): those are + // per-iteration rebinds, not cross-iteration accumulators. + if assignLhsName != "" && isLoopScopedIdent(loopNode, assignLhsName) { continue } @@ -76,10 +113,10 @@ func run(pass *analysis.Pass) (any, error) { return nil, nil } -// enclosingLoopPosition returns the nearest enclosing for/range statement -// position for cur (an AssignStmt), without crossing a function literal -// boundary. Assignments inside func literals are intentionally exempt. -func enclosingLoopPosition(pass *analysis.Pass, cur inspector.Cursor) (token.Position, bool) { +// enclosingLoop returns the nearest enclosing for/range statement, its source +// position, and true for cur (an AssignStmt), without crossing a function +// literal boundary. Assignments inside func literals are intentionally exempt. +func enclosingLoop(pass *analysis.Pass, cur inspector.Cursor) (token.Position, ast.Node, bool) { for encl := range cur.Enclosing( (*ast.ForStmt)(nil), (*ast.RangeStmt)(nil), @@ -87,10 +124,37 @@ func enclosingLoopPosition(pass *analysis.Pass, cur inspector.Cursor) (token.Pos ) { switch encl.Node().(type) { case *ast.ForStmt, *ast.RangeStmt: - return pass.Fset.PositionFor(encl.Node().Pos(), false), true + return pass.Fset.PositionFor(encl.Node().Pos(), false), encl.Node(), true case *ast.FuncLit: - return token.Position{}, false + return token.Position{}, nil, false + } + } + return token.Position{}, nil, false +} + +// isLoopScopedIdent reports whether name is declared by loopNode as a loop +// variable: the Key or Value identifier of a RangeStmt, or a variable in the +// short-declaration Init of a ForStmt. Such variables are per-iteration +// rebinds, not cross-iteration accumulators. +func isLoopScopedIdent(loopNode ast.Node, name string) bool { + switch n := loopNode.(type) { + case *ast.RangeStmt: + if id, ok := n.Key.(*ast.Ident); ok && id.Name == name { + return true + } + if id, ok := n.Value.(*ast.Ident); ok && id.Name == name { + return true + } + case *ast.ForStmt: + init, ok := n.Init.(*ast.AssignStmt) + if !ok || init.Tok != token.DEFINE { + return false + } + for _, lhs := range init.Lhs { + if id, ok := lhs.(*ast.Ident); ok && id.Name == name { + return true + } } } - return token.Position{}, false + return false } diff --git a/pkg/linters/stringsconcatloop/testdata/src/stringsconcatloop/stringsconcatloop.go b/pkg/linters/stringsconcatloop/testdata/src/stringsconcatloop/stringsconcatloop.go index 9d52b42550d..687fae6d289 100644 --- a/pkg/linters/stringsconcatloop/testdata/src/stringsconcatloop/stringsconcatloop.go +++ b/pkg/linters/stringsconcatloop/testdata/src/stringsconcatloop/stringsconcatloop.go @@ -26,6 +26,20 @@ func bad() { ms += myString(p) // want `string concatenation with \+= inside a loop` } _ = ms + + // x = x + y form in a range loop – should be flagged. + accum := "" + for _, p := range parts { + accum = accum + p // want `string concatenation with \+= inside a loop` + } + _ = accum + + // x = x + y form in a classic for loop – should be flagged. + s2 := "" + for i := 0; i < len(parts); i++ { + s2 = s2 + parts[i] // want `string concatenation with \+= inside a loop` + } + _ = s2 } func good() { @@ -59,6 +73,43 @@ func good() { }() } _ = acc + + // x = x + y outside any loop – not flagged. + outside := "prefix" + outside = outside + "suffix" + _ = outside + + // x = y + x (left operand is not the LHS) – not flagged. + accum2 := "" + for _, p := range parts { + accum2 = p + accum2 + } + _ = accum2 + + // Range value variable reassigned per iteration – not a cross-iteration + // accumulator, so not flagged. + for _, line := range parts { + line = line + " suffix" + _ = line + } + + // Range key variable over a string-keyed map – not a cross-iteration + // accumulator, so not flagged. + m := map[string]int{"a": 1, "b": 2} + for k := range m { + k = k + "_x" + _ = k + } + + // x = x + y inside a func literal inside a loop – not flagged. The linter + // intentionally stops at func literal boundaries. + accum3 := "" + for _, p := range parts { + func() { + accum3 = accum3 + p + }() + } + _ = accum3 } func nolintDirective() { From c21e53fe0ed1878c9fddc7325eb9630bc7ae701b Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 30 Jul 2026 06:57:32 +0000 Subject: [PATCH 3/5] docs(adr): add draft ADR-49066 for stringsconcatloop x=x+y extension Co-Authored-By: Claude Sonnet 4.6 --- ...sconcatloop-extend-to-x-equals-x-plus-y.md | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 docs/adr/49066-stringsconcatloop-extend-to-x-equals-x-plus-y.md diff --git a/docs/adr/49066-stringsconcatloop-extend-to-x-equals-x-plus-y.md b/docs/adr/49066-stringsconcatloop-extend-to-x-equals-x-plus-y.md new file mode 100644 index 00000000000..621f6fc1c0e --- /dev/null +++ b/docs/adr/49066-stringsconcatloop-extend-to-x-equals-x-plus-y.md @@ -0,0 +1,52 @@ +# ADR-49066: Extend stringsconcatloop to Detect `x = x + y` Accumulator Pattern + +**Date**: 2026-07-30 +**Status**: Draft +**Deciders**: pelikhan (copilot-swe-agent) + +--- + +### Context + +The `stringsconcatloop` linter existed to flag string `+=` inside for/range loops, which allocates a new string every iteration and creates O(n²) total-bytes cost for cross-iteration accumulators. However, the matcher only checked `token.ADD_ASSIGN` (`x += y`), leaving the semantically identical `x = x + y` (`token.ASSIGN` + `BinaryExpr{Op: ADD}`) undetected. + +This gap was being deliberately exploited: PR #49033 explicitly rewrote 17+ `x += y` sites to `x = x + y` to bypass the linter, and the PR description stated this as its intent. A production site (`pkg/workflow/schedule_preprocessing.go:447`) already exhibited the exact pattern. If left unaddressed, `x = x + y` would become a documented, permanent workaround that other contributors could copy. + +A naive "flag all `lhs = lhs + rhs` in loops" fix would cause false positives: when `lhs` is the loop's own iteration variable (e.g., `for _, line := range lines { line = line + suffix }`), the variable is reset each iteration and is not a cross-iteration accumulator — no O(n²) risk exists. + +### Decision + +We will extend `stringsconcatloop` to also match `token.ASSIGN` statements where `Lhs[0]` is an identifier whose name appears as the left operand of a `BinaryExpr{Op: ADD}` on the right-hand side (direct self-referential form only: `x = x + rhs`). A loop-scope guard (`isLoopScopedIdent`) will be added to exclude variables that are declared by the enclosing loop itself — range `Key`/`Value` identifiers and `ForStmt` `:=` init variables — so only genuine cross-iteration accumulators are flagged. The existing `enclosingLoopPosition` helper is refactored to `enclosingLoop`, returning the `ast.Node` needed by the guard. + +### Alternatives Considered + +#### Alternative 1: Do Nothing (Accept `x = x + y` as a Bypass) + +Accept that `x = x + y` is a valid pattern and leave the linter unchanged. Developers wishing to avoid the `strings.Builder` requirement could use this form. + +Not chosen because the bypass was explicitly documented and being actively promoted: PR #49033 demonstrated it as a deliberate strategy. Accepting it would permanently widen the enforcement gap and legitimize O(n²) string accumulation patterns. + +#### Alternative 2: Flag All `x = x + y` Inside Loops Without a Loop-Scope Guard + +Implement the `token.ASSIGN` + `BinaryExpr` check without adding `isLoopScopedIdent`, relying on the existing `FuncLit`-boundary stop alone. + +Not chosen because `pkg/workflow/schedule_preprocessing.go:447` demonstrates a real false positive: `for _, line := range lines { line = line + ... }` where `line` is the range iteration variable reset each pass. Without the guard, valid single-iteration rebinds would be flagged, producing noise and eroding linter trust. + +### Consequences + +#### Positive +- Both `x += y` and `x = x + y` forms of cross-iteration string accumulation are now detected; the documented bypass vector is closed. +- The loop-scope guard prevents false positives on per-iteration variables, preserving the linter's signal-to-noise ratio. +- Testdata explicitly covers the new true-positive forms (range loop, classic for loop) and all false-positive guard cases (range value var, range key var, func literal boundary, non-self-referential `accum = p + accum`, out-of-loop form). + +#### Negative +- The linter implementation is more complex: `enclosingLoop` now returns three values instead of two, and a new `isLoopScopedIdent` helper must be maintained. +- The scope guard only handles the direct single-step form `x = x + rhs`; chained forms such as `x = x + a + b` remain undetected by design, creating a narrower but still extant bypass for multi-operand expressions. + +#### Neutral +- The `enclosingLoopPosition` function is renamed to `enclosingLoop` and its signature changes; callers (currently only `run()`) must be updated. +- `doc.go` and `Analyzer.Doc` descriptions are updated to reflect both detection forms, keeping documentation in sync with implementation. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From 39d4f6df311b1dd9bfc91381080646a6365ac600 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Jul 2026 07:53:14 +0000 Subject: [PATCH 4/5] stringsconcatloop: fix diagnostic message and for-init accumulator exemption - Make diagnostic message syntax-neutral: remove '+=', now reads 'string concatenation inside a loop ...' - Remove ForStmt init-var exemption from isLoopScopedIdent: for-init vars run once and carry state across iterations (genuine accumulators), so they must be flagged. Only RangeStmt Key/Value remain exempted. - Update all testdata // want patterns to match new message - Add regression test: for-init string accumulator is flagged Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../stringsconcatloop/stringsconcatloop.go | 40 ++++++++----------- .../stringsconcatloop/stringsconcatloop.go | 17 +++++--- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/pkg/linters/stringsconcatloop/stringsconcatloop.go b/pkg/linters/stringsconcatloop/stringsconcatloop.go index ac5ec196720..fad9edb3ddd 100644 --- a/pkg/linters/stringsconcatloop/stringsconcatloop.go +++ b/pkg/linters/stringsconcatloop/stringsconcatloop.go @@ -20,7 +20,7 @@ import ( // Analyzer is the string-concat-in-loop analysis pass. var Analyzer = &analysis.Analyzer{ Name: "stringsconcatloop", - Doc: "reports string += or x = x + y concatenation inside for/range loops that should use strings.Builder", + Doc: "reports string concatenation (+= or x = x + y) inside for/range loops that should use strings.Builder", URL: "https://github.com/github/gh-aw/tree/main/pkg/linters/stringsconcatloop", Requires: []*analysis.Analyzer{inspect.Analyzer, nolint.Analyzer, filecheck.Analyzer}, Run: run, @@ -107,7 +107,7 @@ func run(pass *analysis.Pass) (any, error) { } pass.ReportRangef(assign, - "string concatenation with += inside a loop allocates O(n) strings and O(n²) total bytes; use strings.Builder instead") + "string concatenation inside a loop allocates O(n) strings and O(n²) total bytes; use strings.Builder instead") } return nil, nil @@ -133,28 +133,22 @@ func enclosingLoop(pass *analysis.Pass, cur inspector.Cursor) (token.Position, a } // isLoopScopedIdent reports whether name is declared by loopNode as a loop -// variable: the Key or Value identifier of a RangeStmt, or a variable in the -// short-declaration Init of a ForStmt. Such variables are per-iteration -// rebinds, not cross-iteration accumulators. +// variable: the Key or Value identifier of a RangeStmt. Such variables are +// per-iteration rebinds, not cross-iteration accumulators. +// +// Note: ForStmt init variables (e.g. for s := ""; ...) are intentionally NOT +// exempted — the init clause runs only once, so the variable carries state +// across all iterations and is a genuine accumulator. func isLoopScopedIdent(loopNode ast.Node, name string) bool { - switch n := loopNode.(type) { - case *ast.RangeStmt: - if id, ok := n.Key.(*ast.Ident); ok && id.Name == name { - return true - } - if id, ok := n.Value.(*ast.Ident); ok && id.Name == name { - return true - } - case *ast.ForStmt: - init, ok := n.Init.(*ast.AssignStmt) - if !ok || init.Tok != token.DEFINE { - return false - } - for _, lhs := range init.Lhs { - if id, ok := lhs.(*ast.Ident); ok && id.Name == name { - return true - } - } + n, ok := loopNode.(*ast.RangeStmt) + if !ok { + return false + } + if id, ok := n.Key.(*ast.Ident); ok && id.Name == name { + return true + } + if id, ok := n.Value.(*ast.Ident); ok && id.Name == name { + return true } return false } diff --git a/pkg/linters/stringsconcatloop/testdata/src/stringsconcatloop/stringsconcatloop.go b/pkg/linters/stringsconcatloop/testdata/src/stringsconcatloop/stringsconcatloop.go index 687fae6d289..146fc734136 100644 --- a/pkg/linters/stringsconcatloop/testdata/src/stringsconcatloop/stringsconcatloop.go +++ b/pkg/linters/stringsconcatloop/testdata/src/stringsconcatloop/stringsconcatloop.go @@ -8,14 +8,14 @@ func bad() { // Basic range loop – should be flagged. result := "" for _, p := range parts { - result += p // want `string concatenation with \+= inside a loop` + result += p // want `string concatenation inside a loop` } _ = result // Classic for loop – should be flagged. s := "" for i := 0; i < len(parts); i++ { - s += parts[i] // want `string concatenation with \+= inside a loop` + s += parts[i] // want `string concatenation inside a loop` } _ = s @@ -23,23 +23,30 @@ func bad() { type myString string var ms myString for _, p := range parts { - ms += myString(p) // want `string concatenation with \+= inside a loop` + ms += myString(p) // want `string concatenation inside a loop` } _ = ms // x = x + y form in a range loop – should be flagged. accum := "" for _, p := range parts { - accum = accum + p // want `string concatenation with \+= inside a loop` + accum = accum + p // want `string concatenation inside a loop` } _ = accum // x = x + y form in a classic for loop – should be flagged. s2 := "" for i := 0; i < len(parts); i++ { - s2 = s2 + parts[i] // want `string concatenation with \+= inside a loop` + s2 = s2 + parts[i] // want `string concatenation inside a loop` } _ = s2 + + // for-init string accumulator – the init clause runs once, so the variable + // carries state across all iterations and is a genuine accumulator. + for s3 := ""; len(s3) < 10; { + s3 = s3 + "x" // want `string concatenation inside a loop` + _ = s3 + } } func good() { From 40435a0a7f65b7885ceb92f6b2671eb37f350fa8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Jul 2026 07:59:49 +0000 Subject: [PATCH 5/5] stringsconcatloop: address all remaining review threads - Apply isLoopScopedIdent + isLoopBodyLocal to ADD_ASSIGN form too, not just ASSIGN; range-value vars using += are now correctly exempted - Add len(assign.Lhs) != 1 guard to ADD_ASSIGN case (symmetric with ASSIGN) - Add isLoopBodyLocal helper: uses TypesInfo.ObjectOf to detect vars declared inside the loop body (per-iteration locals, not accumulators) - Testdata: add chained x = x + a + b case (not flagged), loop-body-local var case (not flagged), and ADD_ASSIGN range-value guard case Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../stringsconcatloop/stringsconcatloop.go | 50 +++++++++++++++---- .../stringsconcatloop/stringsconcatloop.go | 23 +++++++++ 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/pkg/linters/stringsconcatloop/stringsconcatloop.go b/pkg/linters/stringsconcatloop/stringsconcatloop.go index fad9edb3ddd..0a73117a00d 100644 --- a/pkg/linters/stringsconcatloop/stringsconcatloop.go +++ b/pkg/linters/stringsconcatloop/stringsconcatloop.go @@ -47,13 +47,14 @@ func run(pass *analysis.Pass) (any, error) { } // Match both `x += y` (ADD_ASSIGN) and `x = x + y` (ASSIGN with a - // self-referential binary addition). For the latter, also capture the - // LHS identifier so the loop-scope guard can be applied. + // self-referential binary addition on a plain identifier). var lhsExpr ast.Expr - var assignLhsName string // non-empty only for the token.ASSIGN form switch assign.Tok { case token.ADD_ASSIGN: + if len(assign.Lhs) != 1 { + continue + } lhsExpr = assign.Lhs[0] case token.ASSIGN: if len(assign.Lhs) != 1 || len(assign.Rhs) != 1 { @@ -68,13 +69,14 @@ func run(pass *analysis.Pass) (any, error) { continue } // Only match the direct self-referential form: x = x + rhs, - // where x is the same identifier on both sides. + // where x is the same identifier on both sides (not chained + // forms like x = x + a + b which parse with a BinaryExpr on + // the left of the outer add, not an Ident). rhsLeft, ok := binExpr.X.(*ast.Ident) if !ok || rhsLeft.Name != lhsIdent.Name { continue } lhsExpr = lhsIdent - assignLhsName = lhsIdent.Name default: continue } @@ -99,11 +101,17 @@ func run(pass *analysis.Pass) (any, error) { continue } - // For the x = x + y form, skip variables declared by the enclosing - // loop itself (range key/value or for-init short-decl): those are - // per-iteration rebinds, not cross-iteration accumulators. - if assignLhsName != "" && isLoopScopedIdent(loopNode, assignLhsName) { - continue + // Skip variables that are per-iteration rather than cross-iteration + // accumulators. These checks apply only when the LHS is a plain + // identifier (the ADD_ASSIGN form also accepts field/index lvalues, + // but those can only be tested via the ASSIGN form which requires Ident). + if lhsIdent, ok := lhsExpr.(*ast.Ident); ok { + if isLoopScopedIdent(loopNode, lhsIdent.Name) { + continue + } + if isLoopBodyLocal(pass, loopNode, lhsIdent) { + continue + } } pass.ReportRangef(assign, @@ -152,3 +160,25 @@ func isLoopScopedIdent(loopNode ast.Node, name string) bool { } return false } + +// isLoopBodyLocal reports whether ident is declared inside the loop body +// (rather than before the loop). Such variables are freshly created on every +// iteration and are therefore not cross-iteration accumulators. +func isLoopBodyLocal(pass *analysis.Pass, loopNode ast.Node, ident *ast.Ident) bool { + obj := pass.TypesInfo.ObjectOf(ident) + if obj == nil { + return false + } + var body *ast.BlockStmt + switch n := loopNode.(type) { + case *ast.ForStmt: + body = n.Body + case *ast.RangeStmt: + body = n.Body + } + if body == nil { + return false + } + pos := obj.Pos() + return pos >= body.Lbrace && pos < body.Rbrace +} diff --git a/pkg/linters/stringsconcatloop/testdata/src/stringsconcatloop/stringsconcatloop.go b/pkg/linters/stringsconcatloop/testdata/src/stringsconcatloop/stringsconcatloop.go index 146fc734136..96b512f9159 100644 --- a/pkg/linters/stringsconcatloop/testdata/src/stringsconcatloop/stringsconcatloop.go +++ b/pkg/linters/stringsconcatloop/testdata/src/stringsconcatloop/stringsconcatloop.go @@ -117,6 +117,29 @@ func good() { }() } _ = accum3 + + // x = x + a + b (chained addition) – binExpr.X is itself a BinaryExpr, + // not an Ident, so the self-referential check fails and it is not flagged. + accum4 := "" + for _, p := range parts { + accum4 = accum4 + p + " extra" + } + _ = accum4 + + // Variable declared inside the loop body is a per-iteration local, not a + // cross-iteration accumulator – not flagged. + for _, p := range parts { + var local string + local = local + p + _ = local + } + + // Range value variable reassigned via += – not a cross-iteration + // accumulator, so not flagged. + for _, line := range parts { + line += " suffix" + _ = line + } } func nolintDirective() {