Skip to content

x/tools/gopls: modernizer max changes logic #71878

@nemith

Description

@nemith

Go version

go1.24.0

Output of go env in your module/workspace:

go env
AR='ar'
CC='cc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='c++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/bbennett/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/bbennett/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/ch/51hb_xjn10z7z6kvtc94dqv00000gp/T/go-build543263727=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/bbennett/tmp/modernizerbugrepro/go.mod'
GOMODCACHE='/Users/bbennett/.go/pkg/mod'
GONOPROXY='<THIS IS PRIVATE!>'
GONOSUMDB='<THIS IS PRIVATE!>'
GOOS='darwin'
GOPATH='/Users/bbennett/.go'
# GOPRIVATE='<THIS IS PRIVATE!>'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.24.0/libexec'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/bbennett/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.24.0/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24.0'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

Given this code:

package modernizebugrepo

import "time"

var defaultInterval = 60 * time.Second

func foo(interval time.Duration) time.Duration {
	var i time.Duration // imagine this was actually a part of an already initialized structure and isn't set to defaultInterval

	if interval <= 0 {
		i = defaultInterval
	} else {
		i = interval
	}
	return i
}

I ran go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -fix -test .

What did you see happen?

The code was "modernized" into a call to max which got rid of the defaultInterval all together.

package modernizebugrepo

import "time"

var defaultInterval = 60 * time.Second

func foo(interval time.Duration) time.Duration {
	var i time.Duration // imagine this was actually a part of an already initialized structure and isn't set to defaultInterval

	i = max(interval, 0)
	return i
}

What did you expect to see?

In this case I am guessing modernizer needs to leave the code alone. There may be some magic refactoring for this to try to use max (or min) but it really probably worth it.

Metadata

Metadata

Assignees

Labels

BugReportIssues describing a possible bug in the Go implementation.ToolsThis label describes issues relating to any tools in the x/tools repository.goplsIssues related to the Go language server, gopls.

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions