[go] cmd/fix: avoid non-bool assignment rewrite in slicescontains

4 views
Skip to first unread message

Chencheng Jiang (Gerrit)

unread,
Mar 13, 2026, 1:19:53 PM (6 days ago) Mar 13
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Chencheng Jiang has uploaded the change for review

Commit message

cmd/fix: avoid non-bool assignment rewrite in slicescontains

The slicescontains modernizer folds certain search loops into a
slices.Contains assignment. It previously checked whether the
pre-loop and loop-body assignments were opposite boolean literals,
but did not verify that they were actually boolean literals.

For non-boolean expressions isTrueOrFalse returns 0, which caused the
condition to match accidentally (0 == -0). This could incorrectly
rewrite non-boolean assignments.

Require the loop-body assignment to be a boolean literal before
applying the rewrite.

Fixes #78149
Change-Id: Ia5108e855ec7bbf7b91c3362624cc6f08b861954

Change diff

diff --git a/src/cmd/go/testdata/script/fix_slicescontains_regression.txt b/src/cmd/go/testdata/script/fix_slicescontains_regression.txt
new file mode 100644
index 0000000..23e886d
--- /dev/null
+++ b/src/cmd/go/testdata/script/fix_slicescontains_regression.txt
@@ -0,0 +1,54 @@
+# Regression test for the slicescontains modernizer.
+# A loop that mutates a non-bool value before breaking must stay on the generic
+# if-Contains path instead of being folded into an assignment.
+# issue #78149
+
+go fix example.com/x
+cmp x.go x.go.want
+
+-- go.mod --
+module example.com/x
+go 1.26
+
+-- x.go --
+package x
+
+import "fmt"
+
+type Object struct{}
+
+func (o Object) Do() Object { return Object{} }
+func (o Object) Print()     { fmt.Println("Hello, World!") }
+
+func doSomething(obj Object, opts ...string) {
+	o := obj
+	for _, opt := range opts {
+		if opt == "something" {
+			o = o.Do()
+			break
+		}
+	}
+
+	o.Print()
+}
+
+-- x.go.want --
+package x
+
+import "slices"
+
+import "fmt"
+
+type Object struct{}
+
+func (o Object) Do() Object { return Object{} }
+func (o Object) Print()     { fmt.Println("Hello, World!") }
+
+func doSomething(obj Object, opts ...string) {
+	o := obj
+	if slices.Contains(opts, "something") {
+		o = o.Do()
+	}
+
+	o.Print()
+}
diff --git a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/slicescontains.go b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/slicescontains.go
index 3b32685..e5eb49f 100644
--- a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/slicescontains.go
+++ b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/slicescontains.go
@@ -316,12 +316,13 @@
 					len(assign.Rhs) == 1 {
 
 					// Have: body={ lhs = rhs; break }
+					assignBool := isTrueOrFalse(info, assign.Rhs[0])
 					if prevAssign, ok := prevStmt.(*ast.AssignStmt); ok &&
 						len(prevAssign.Lhs) == 1 &&
 						len(prevAssign.Rhs) == 1 &&
 						astutil.EqualSyntax(prevAssign.Lhs[0], assign.Lhs[0]) &&
-						isTrueOrFalse(info, assign.Rhs[0]) ==
-							-isTrueOrFalse(info, prevAssign.Rhs[0]) {
+						assignBool != 0 &&
+						assignBool == -isTrueOrFalse(info, prevAssign.Rhs[0]) {
 
 						// Have:
 						//    lhs = false
@@ -332,7 +333,7 @@
 						// TODO(adonovan):
 						// - support "var lhs bool = false" and variants.
 						// - allow the break to be omitted.
-						neg := cond(isTrueOrFalse(info, assign.Rhs[0]) < 0, "!", "")
+						neg := cond(assignBool < 0, "!", "")
 						report([]analysis.TextEdit{
 							// Replace "rhs" of previous assignment by [!]slices.Contains(...)
 							{

Change information

Files:
Change size: M
Delta: 2 files changed, 58 insertions(+), 3 deletions(-)
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newchange
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: Ia5108e855ec7bbf7b91c3362624cc6f08b861954
Gerrit-Change-Number: 755141
Gerrit-PatchSet: 1
Gerrit-Owner: Chencheng Jiang <dorb...@gmail.com>
unsatisfied_requirement
satisfied_requirement
open
diffy

Madeline Kalil (Gerrit)

unread,
Mar 16, 2026, 10:41:09 AM (3 days ago) Mar 16
to Chencheng Jiang, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Chencheng Jiang

Madeline Kalil voted Commit-Queue+1

Commit-Queue+1
Open in Gerrit

Related details

Attention is currently required from:
  • Chencheng Jiang
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: Ia5108e855ec7bbf7b91c3362624cc6f08b861954
Gerrit-Change-Number: 755141
Gerrit-PatchSet: 1
Gerrit-Owner: Chencheng Jiang <dorb...@gmail.com>
Gerrit-Reviewer: Madeline Kalil <mka...@google.com>
Gerrit-Attention: Chencheng Jiang <dorb...@gmail.com>
Gerrit-Comment-Date: Mon, 16 Mar 2026 14:41:05 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
unsatisfied_requirement
satisfied_requirement
open
diffy

Madeline Kalil (Gerrit)

unread,
Mar 16, 2026, 11:27:16 AM (3 days ago) Mar 16
to Chencheng Jiang, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
Attention needed from Chencheng Jiang

Madeline Kalil added 1 comment

Patchset-level comments
File-level comment, Patchset 1 (Latest):
Madeline Kalil . unresolved

Hello, thank you for working on this. We'll need to make this change in the x/tools repo first and then vendor it to the main go repo.

Open in Gerrit

Related details

Attention is currently required from:
  • Chencheng Jiang
Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: Ia5108e855ec7bbf7b91c3362624cc6f08b861954
    Gerrit-Change-Number: 755141
    Gerrit-PatchSet: 1
    Gerrit-Owner: Chencheng Jiang <dorb...@gmail.com>
    Gerrit-Reviewer: Madeline Kalil <mka...@google.com>
    Gerrit-Attention: Chencheng Jiang <dorb...@gmail.com>
    Gerrit-Comment-Date: Mon, 16 Mar 2026 15:27:13 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    unsatisfied_requirement
    open
    diffy

    Madeline Kalil (Gerrit)

    unread,
    Mar 16, 2026, 3:19:17 PM (3 days ago) Mar 16
    to Chencheng Jiang, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
    Attention needed from Chencheng Jiang

    Madeline Kalil added 1 comment

    Patchset-level comments
    Madeline Kalil . unresolved

    Hello, thank you for working on this. We'll need to make this change in the x/tools repo first and then vendor it to the main go repo.

    Madeline Kalil

    So instead of changing src/cmd/go/vendor.. you'll need to send a CL to update x/tools/go/analysis/passes/modernize/slicescontains.go. Let me know if you're still interested in working on this! If not, I'm happy to finish it up

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Chencheng Jiang
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: Ia5108e855ec7bbf7b91c3362624cc6f08b861954
    Gerrit-Change-Number: 755141
    Gerrit-PatchSet: 1
    Gerrit-Owner: Chencheng Jiang <dorb...@gmail.com>
    Gerrit-Reviewer: Madeline Kalil <mka...@google.com>
    Gerrit-Attention: Chencheng Jiang <dorb...@gmail.com>
    Gerrit-Comment-Date: Mon, 16 Mar 2026 19:19:12 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Madeline Kalil <mka...@google.com>
    unsatisfied_requirement
    open
    diffy

    Madeline Kalil (Gerrit)

    unread,
    2:55 PM (7 hours ago) 2:55 PM
    to Chencheng Jiang, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
    Attention needed from Chencheng Jiang

    Madeline Kalil added 1 comment

    Patchset-level comments
    Madeline Kalil . unresolved

    Hello, thank you for working on this. We'll need to make this change in the x/tools repo first and then vendor it to the main go repo.

    Madeline Kalil

    So instead of changing src/cmd/go/vendor.. you'll need to send a CL to update x/tools/go/analysis/passes/modernize/slicescontains.go. Let me know if you're still interested in working on this! If not, I'm happy to finish it up

    Madeline Kalil
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Chencheng Jiang
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: Ia5108e855ec7bbf7b91c3362624cc6f08b861954
    Gerrit-Change-Number: 755141
    Gerrit-PatchSet: 1
    Gerrit-Owner: Chencheng Jiang <dorb...@gmail.com>
    Gerrit-Reviewer: Madeline Kalil <mka...@google.com>
    Gerrit-Attention: Chencheng Jiang <dorb...@gmail.com>
    Gerrit-Comment-Date: Thu, 19 Mar 2026 18:55:09 +0000
    unsatisfied_requirement
    open
    diffy
    Reply all
    Reply to author
    Forward
    0 new messages