Go: cmd/gofmt: formatting change between go1.13.8 and go1.14, unknown if intentional or not

Created on 3 Mar 2020  ·  5Comments  ·  Source: golang/go

What version of Go are you using (go version)?

go version go1.14 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/kevin.conaway/Library/Caches/go-build"
GOENV="/Users/kevin.conaway/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/kevin.conaway/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.14/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.14/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/ft/kd6lkcqn0jb3l0jyxkqvl2zw0000gn/T/go-build005865273=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

After upgrading to go 1.14, we noticed that go fmt was reformatting some of our files that previously had not changed under go 1.13 and earlier versions.

I didn't see anything related to fmt in the release notes, is this behavior expected?

Below is a sample file that shows the issue

Run go fmt on the following file

package main

type NullCache struct{}

func (n NullCache) SetManyWithTTL(keys []string, values [][]byte, ttl time.Duration) error { return nil }

What did you expect to see?

No change in formatting

What did you see instead?

The formatting was was reformatted to

package main

type NullCache struct{}

func (n NullCache) SetManyWithTTL(keys []string, values [][]byte, ttl time.Duration) error {
        return nil
}
NeedsInvestigation

Most helpful comment

I've investigated this.

This is caused by CL 188818, which was an intentional change in order to fix issue #28082. /cc @eliben @mvdan

The single-line SetManyWithTTL method definition in the snippet is 105 characters, which is slightly over 100, and gets formatted into a multi-line definition. That is one of the side-effects of that CL, and as described in the commit message of CL 188818, it was deemed acceptable.

Closing since cmd/gofmt is working as intended and there's nothing to do here.

Thanks again for reporting this issue @kevinconaway!

All 5 comments

Thanks for reporting. I can reproduce this.

I'll copy what's written at https://pkg.go.dev/go/format:

Note that formatting of Go source code changes over time, so tools relying on consistent formatting should execute a specific version of the gofmt binary

That said, we should indeed confirm whether this was an intentional change in Go 1.14 or not. I'll apply NeedsInvestigation label to do that work.

/cc @griesemer

I'll point out that formatting the input file with Go 1.14 produces an output that is gofmt-compatible with previous versions of Go (1.13.x and 1.12.x). (If it hadn't, that would make it very obvious that the change is unintentional.)

Thanks. I should clarify that this isn't a problem for us apart from having to reformat a handful of places in our code base where this pattern occurs.

I just wanted to bring this up in case it wasn't intentional.

Thanks for making that clear, and for reporting this!

I've investigated this.

This is caused by CL 188818, which was an intentional change in order to fix issue #28082. /cc @eliben @mvdan

The single-line SetManyWithTTL method definition in the snippet is 105 characters, which is slightly over 100, and gets formatted into a multi-line definition. That is one of the side-effects of that CL, and as described in the commit message of CL 188818, it was deemed acceptable.

Closing since cmd/gofmt is working as intended and there's nothing to do here.

Thanks again for reporting this issue @kevinconaway!

Was this page helpful?
0 / 5 - 0 ratings