[tools] go/analysis/passes/modernize: fix stringsbuilder bug with in var(...)

0 views

Skip to first unread message

Gopher Robot (Gerrit)

unread,

Dec 29, 2025, 4:38:56 PM (14 hours ago) Dec 29

to Alan Donovan, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Peter Weinberger, Rian Brady, golang-co...@googlegroups.com

Gopher Robot submitted the change with unreviewed changes

Unreviewed changes

3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: go/analysis/passes/modernize/testdata/src/stringsbuilder/stringsbuilder.go
Insertions: 29, Deletions: 0.

@@ -1,5 +1,7 @@
 package stringsbuilder
 
+import "strings"
+
 // basic test
 func _() {
 	var s string
@@ -100,6 +102,35 @@
 	return msg
 }
 
+// Regression test for https://go.dev/issue/76983.
+// We offer only the first fix if the second would overlap.
+// This is an ad-hoc mitigation of the more general issue #76476.
+func _(slice []string) string {
+	a := "12"
+	for range 2 {
+		a += "34" // want "using string \\+= string in a loop is inefficient"
+	}
+	b := "56"
+	for range 2 {
+		b += "78"
+	}
+	a += b
+	return a
+}
+func _(slice []string) string {
+	var a strings.Builder
+	a.WriteString("12")
+	for range 2 {
+		a.WriteString("34")
+	}
+	b := "56"
+	for range 2 {
+		b += "78" // want "using string \\+= string in a loop is inefficient"
+	}
+	a.WriteString(b)
+	return a.String()
+}
+
 // Regression test for go.dev/issue/76934, which mutilated the var decl.
 func stringDeclaredWithVarDecl() {
 	var (
```
```
The name of the file: go/analysis/passes/modernize/testdata/src/stringsbuilder/stringsbuilder.go.golden
Insertions: 30, Deletions: 0.

@@ -105,6 +105,36 @@
 	return msg.String()
 }
 
+// Regression test for https://go.dev/issue/76983.
+// We offer only the first fix if the second would overlap.
+// This is an ad-hoc mitigation of the more general issue #76476.
+func _(slice []string) string {
+	var a strings.Builder; a.WriteString("12")
+	for range 2 {
+		a.WriteString("34") // want "using string \\+= string in a loop is inefficient"
+	}
+	b := "56"
+	for range 2 {
+		b += "78"
+	}
+	a.WriteString(b)
+	return a.String()
+}
+func _(slice []string) string {
+	var a strings.Builder
+	a.WriteString("12")
+	for range 2 {
+		a.WriteString("34")
+	}
+	var b strings.Builder
+	b.WriteString("56")
+	for range 2 {
+		b.WriteString("78") // want "using string \\+= string in a loop is inefficient"
+	}
+	a.WriteString(b.String())
+	return a.String()
+}
+
 // Regression test for go.dev/issue/76934, which mutilated the var decl.
 func stringDeclaredWithVarDecl() {
 	var (
```

Change information

Commit message:

go/analysis/passes/modernize: fix stringsbuilder bug with in var(...)

The previous logic neglected to consider the possibility of a var decl
with explicit parens, and inserted a statement where it wouldn't go.
Now, we check for parens, and if so, explicitly move the close paren
(by insertion and deletion).

Byte-level surgery is good for avoiding comment loss, but it is so
fiddly and low level.  We really need a better calculus of edits.

Fixes golang/go#76934

Change-Id: Ib21c9cd379035e0fdb9c098e7682e38347bdf81c

Reviewed-on: https://go-review.googlesource.com/c/tools/+/731881

Auto-Submit: Alan Donovan <adon...@google.com>

LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Reviewed-by: Peter Weinberger <p...@google.com>

Files:

M go/analysis/passes/modernize/modernize.go
M go/analysis/passes/modernize/slices.go
M go/analysis/passes/modernize/stditerators.go
M go/analysis/passes/modernize/stringsbuilder.go
M go/analysis/passes/modernize/testdata/src/stringsbuilder/stringsbuilder.go
M go/analysis/passes/modernize/testdata/src/stringsbuilder/stringsbuilder.go.golden

Change size: M

Delta: 6 files changed, 92 insertions(+), 10 deletions(-)

Branch: refs/heads/master

Submit Requirements:

Code-Review: +2 by Peter Weinberger
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

0 new messages