cmd/fix,x/tools/go/analysis/passes/modernize: stringscut rewrite changes behavior

[stapelberg]

Go version

go version go1.26.0 linux/amd64

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/michael/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/michael/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3478247958=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/home/michael/dcs/go.mod'
GOMODCACHE='/home/michael/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/michael/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/michael/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.26.0.linux-amd64'
GOSUMDB='sum.golang.org'
GOTELEMETRY='on'
GOTELEMETRYDIR='/home/michael/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/michael/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.26.0.linux-amd64/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.26.0'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

Ran go fix ./..., which broke my tests

What did you see happen?

I expected my code to be rewritten without changing behavior :)

cc @adonovan

What did you expect to see?

I asked Claude Opus 4.6 to extract a reproducer and have verified that the reproducer illustrates the problem. Allow me to post AI slop, with a hypothesis on where the problem actually is (I have not verified that part):

---

The pattern

Consider the following Go code, which I found in the Debian Code
Search project's dpkgversion package:

colon := strings.Index(trimmed, ":")
if colon != -1 {
    // parse epoch from trimmed[:colon]
    result.Epoch = uint(epoch)
}
result.Version = trimmed[colon+1:]   // unconditional

This is a common (if subtle) idiom:

1. strings.Index returns -1 when the separator is not found.
2. When found, the if block processes the part before the separator.
3. The unconditional trimmed[colon+1:] works in both cases: when the separator
   is found, it gives everything after it. When the separator is absent,
   -1 + 1 = 0, so trimmed[0:] is the entire string.

After running go fix, the code becomes:

before, after, ok := strings.Cut(trimmed, ":")
if ok {
    // parse epoch from before
    result.Epoch = uint(epoch)
}
result.Version = after   // BUG: "" when no colon found

The problem: when ok is false (no separator), after is "". The original
code returned the whole string, but now it returns an empty string.

Minimal reproducer

I distilled this into a self-contained reproducer:

package main

import (
	"fmt"
	"strings"
)

func parse(s string) (string, error) {
	colon := strings.Index(s, ":")
	if colon != -1 {
		fmt.Println("has epoch:", s[:colon])
	}

	version := s[colon+1:]
	if len(version) == 0 {
		return "", fmt.Errorf("empty version")
	}
	return version, nil
}

func main() {
	v, err := parse("0")
	if err != nil {
		fmt.Println("error:", err)
	} else {
		fmt.Printf("version=%q\n", v)
	}
}

Steps:

$ go run main.go
version="0"

$ go fix ./...

$ go run main.go
error: empty version

The go fix rewrite turns version := s[colon+1:] into version := after,
which is "" when the input has no colon.

Root cause in the analyzer

The bug is in
stringscut.go,
specifically in the checkIdxUses function (line 377).

This function classifies every use of the index variable i into four buckets:

1. negative — conditions like i < 0 or i == -1
2. nonnegative — conditions like i >= 0 or i != -1
3. beforeSlice — s[:i]
4. afterSlice — s[i+len(substr):]

If every use of i fits one of these buckets, the rewrite is considered safe
(line 190). All afterSlice occurrences are then replaced with after.

The problem is purely syntactic classification. The function checks that each
use pattern-matches one of the valid forms, but it never checks control
flow. It does not verify that beforeSlice and afterSlice uses are dominated
by a nonnegative guard on the index variable.

In my reproducer, s[colon+1:] on line 15 (outside the if colon != -1 block)
is classified as afterSlice because it syntactically matches s[i + const:]
where const == len(":"). The rewrite proceeds and replaces it with after.

Here's the relevant code path in checkIdxUses:

case edge.SliceExpr_Low, edge.SliceExpr_High:
    slice := n.(*ast.SliceExpr)
    if sameObject(info, s, slice.X) && slice.Max == nil {
        if isBeforeSlice(info, ek, slice) {
            beforeSlice = append(beforeSlice, slice)
            return true
        } else if isAfterSlice(info, ek, slice, substr) {
            afterSlice = append(afterSlice, slice)
            return true
        }
    }

No dominator check. No control flow analysis. Just syntax matching.

Existing issues

I checked all open and closed stringscut issues on the Go issue tracker as of
today (2026-02-22). None of them cover this specific bug:

• #77566: := vs = variable reuse (different)
• #77529: OOB panic on buf.Bytes() (different)
• #76687: false positives with i > 0 (different, fixed)
• #77563: stringscutprefix empty prefix semantics (different analyzer)

---

[gabyhelp]

Related Issues

• x/tools/go/analysis/passes/modernize: stringscutprefix changes semantics for empty prefix #77563
• cmd/fix,x/tools/go/analysis/passes/modernize: stringscut does not fully recognize variable definition reuse #77566
• tools/gopls/internal/analysis/modernize: stringscutprefix offers invalid fix #73547 (closed)
• x/tools/go/analysis/passes/modernize: stringscut reports false positives #76687 (closed)
• cmd/fix: OOB panic in stringscut modernizer #77595 (closed)
• cmd/fix, x/tools/go/analysis/passes/modernize: stringscut: OOB panic in indexArgValid analyzing "buf.Bytes()" call #77208 (closed)
• cmd/fix,x/tools/go/analysis/passes/modernize: slicescontains produces invalid result #77616
• x/tools/gopls: modernize slices.Contains rewrite affects program behaviour #76210 (closed)
• cmd/fix, x/tools/go/analysis/passes/modernize: stringsbuilder breaks code when combining two `strings.Builder`s #76983 (closed)
• modernize linter stringscut panic #77451 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[gopherbot]

Change https://go.dev/cl/747900 mentions this issue: go/analysis/passes/modernize: fix stringscut false positive for unguarded afterSlice

[dr2chase]

@adonovan

[gopherbot]

Change https://go.dev/cl/749180 mentions this issue: go/analysis/passes/modernize: fix stringscut false positive for unguarded afterSlice

[dmitshur]

This issue was marked as fixed by CL 747900, but that CL is in the main branch of x/tools. That's sufficient to fix the problem in time for Go 1.27, but this issue is currently in the Go 1.26.1 milestone, giving a misleading impression it will be fixed in Go 1.26.1. To actually make that happen it requires backporting, so I'll correct this issue back to the Go1.27 milestone and create a backport issue to track what's left to complete the backport.