tools/gopls/internal/analysis/modernize: stringscutprefix offers invalid fix

[cuishuang]

Go version

go1.24.2

Output of go env in your module/workspace:

AR='ar'
CC='clang'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='0'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='clang++'
GCCGO='gccgo'
GO111MODULE='auto'
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/xxx/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/xxx/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/9t/839s3jmj73bcgyp5x_xh3gw00000gn/T/go-build2973783802=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/xxxx/20250428/tools/gopls/go.mod'
GOMODCACHE='/Users/xxx/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/xxx/go'
GOPRIVATE=''
GOPROXY='https://goproxy.cn,direct'
GOROOT='/usr/local/go'
GOSUMDB='off'
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/xxx/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24.2'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

package main

import (
	"fmt"
	"strings"
)

func main() {

	var tokenStr string
	demoMap := make(map[string]string)

	demoMap["Authorization"] = "Bearer 123456789abc"

	if auth := demoMap["Authorization"]; strings.HasPrefix(auth, "Bearer ") {
		tokenStr = strings.TrimPrefix(auth, "Bearer ")
	}

	fmt.Println("tokenStr is:", tokenStr)

}

What did you see happen?

After executing modernize -fix -test ./..., it will become.

package main

import (
	"fmt"
	"strings"
)

func main() {

	var tokenStr string
	demoMap := make(map[string]string)

	demoMap["Authorization"] = "Bearer 123456789abc"

	if auth := demoMap["Authorization"]; after, ok :=strings.CutPrefix(auth, "Bearer "); ok  {
		tokenStr = after
	}

	fmt.Println("tokenStr is:", tokenStr)

}

This will result in a compilation failure.

What did you expect to see?

In fact, what we expect is something like this:

package main

import (
	"fmt"
	"strings"
)

func main() {

	var tokenStr string
	demoMap := make(map[string]string)

	demoMap["Authorization"] = "Bearer 123456789abc"

	if after, ok := strings.CutPrefix(demoMap["Authorization"], "Bearer "); ok {
		tokenStr = after
	}

	fmt.Println("tokenStr is:", tokenStr)

}

[gopherbot]

Change https://go.dev/cl/669055 mentions this issue: gopls/internal/analysis/modernize: add more scenarios for stringscutprefix

[gabyhelp]

Related Issues

• strings.TrimLeft("xxx "), In case of loss of a #62344 (closed)
• x/tools/gopls/internal/analysis/modernize: rangeint: transformation unsound when loop variable is defined outside loop #73009 (closed)
• string.CutPrefix not functioning as expected #71523 (closed)
• x/tools/gopls/internal/analysis/modernize: replacing non-equivalent code #73073 (closed)
• x/tools/gopls: support `gofmt -s` in formatting #37221 (closed)

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

[adonovan]

cc: @xieyuschen

[xieyuschen]

cc: @xieyuschen

I saw there is an ongoing CL, and left some comments already. My idea is just to skip offering a fix for this scenario rather than moving init declaration to the if stmt.

[cuishuang]

cc: @xieyuschen

I saw there is an ongoing CL, and left some comments already. My idea is just to skip offering a fix for this scenario rather than moving init declaration to the if stmt.

Ignoring the variable initialization scenario seems to be achievable with minimal changes based on the existing code.

However, if I move demoMap["Authorization"] into strings.CutPrefix, my attempts have led to significant code modifications.

I'm not sure if this situation should also be modified in the same way. I would like to get more opinions from two contributors on this. cc @xieyuschen @adonovan

Perhaps I can first make the changes to the code and temporarily ignore this scenario.

[cuishuang]

cc: @xieyuschen

Oh, Thanks. I saw your comment regarding the example above where auth might be used later. I conducted a test using the current code of CL669055 with significant changes (though it still needs test fixes), and it won't make any modifications in that case.

package main

import (
	"fmt"
	"strings"
)

func main() {

	var tokenStr string
	demoMap := make(map[string]string)

	demoMap["Authorization"] = "Bearer 123456789abc"

	if auth := demoMap["Authorization"]; strings.HasPrefix(auth, "Bearer ") {
		fmt.Println("auth is:", auth)
		tokenStr = strings.TrimPrefix(auth, "Bearer ")
	}

	fmt.Println("tokenStr is:", tokenStr)

}

(Running modernize -fix -test ./... will not make any changes)

However, if auth is not used later, It would proceed with the changes as mentioned above.

So we may have to make a choice between functionality and the complexity of code refactoring.

I also tend to prefer ignoring this scenario for now.

[xieyuschen]

I prefer to separate the fix CL and another CL to support more patterns. So the first CL could get a quick win.

However, if I move demoMap["Authorization"] into strings.CutPrefix, my attempts have led to significant code modifications.

Looks like you want to inline auth variable in this case, but you need to ensure whether auth has been used inside ifStmt.Body.
Moreover, if you plan to support this pattern, should we support if s, pre := “”, ""; strings.HasPrefix(s, pre) { ... } as well? Btw, you may need equalSyntax to check whether a variable is used inside ifStmt.Body.

[cuishuang]

Looks like you want to inline auth variable in this case, but you need to ensure whether auth has been used inside ifStmt.Body. Moreover, if you plan to support this pattern, should we support if s, pre := “”, ""; strings.HasPrefix(s, pre) { ... } as well? Btw, you may need equalSyntax to check whether a variable is used inside ifStmt.Body.

Yes, this can get complicated. Maybe we should keep it simple and just ignore this case. I'll update the code later, thanks