Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/cmd/goimports: inconsistency with go fmt #37584

Closed
yujiri8 opened this issue Feb 29, 2020 · 2 comments
Closed

x/tools/cmd/goimports: inconsistency with go fmt #37584

yujiri8 opened this issue Feb 29, 2020 · 2 comments

Comments

@yujiri8
Copy link

@yujiri8 yujiri8 commented Feb 29, 2020

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

$ go version
go version go1.14 freebsd/amd64

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/ryan/.cache/go-build"
GOENV="/home/ryan/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="freebsd"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="freebsd"
GOPATH="/home/ryan/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/freebsd_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=/tmp/go-build464992684=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Used the tools go fmt and goimports on the following file (a minimal test case):

package main

func f() {
	var d = struct {
		Shorty int
		Short int
		VeryMuchLongerFieldName int
	}{
		Shorty: 5,
		Short: 5,
		VeryMuchLongerFieldName: 5,
	}
}

Go fmt converts it to:

package main

func f() {
	var d = struct {
		Shorty                  int
		Short                   int
		VeryMuchLongerFieldName int
	}{
		Shorty:                  5,
		Short:                   5,
		VeryMuchLongerFieldName: 5,
	}
}

Goimports converts it to:

package main

func f() {
	var d = struct {
		Shorty                  int
		Short                   int
		VeryMuchLongerFieldName int
	}{
		Shorty: 5,
		Short:  5,
		VeryMuchLongerFieldName: 5,
	}
}

The two tools disagree. This results in them always trying to reformat each others' code.

Goimports starts to agree with gofmt if Shorty and Short are in the opposite order, so that the field length progresses upward; or if VeryMuchLongerFieldName is reduced to MuchLongerFieldName.

@gopherbot gopherbot added this to the Unreleased milestone Feb 29, 2020
@gopherbot gopherbot added the Tools label Feb 29, 2020
@dmitshur
Copy link
Member

@dmitshur dmitshur commented Feb 29, 2020

Did you build goimports command with Go 1.14, or is it an older binary? What does go version -m $(which goimports) print?

I can't reproduce this. I suspect your goimports binary was built with an older version of Go, where gofmt behavior was slightly different, and so it's producing formatting output consistent with that old version of Go.

Edit: Actually, I can reproduce this if I build goimports with Go 1.10. There was a change to gofmt behavior in Go 1.11 (see https://golang.org/doc/go1.11#gofmt). So your goimports binary is probably built with Go 1.10 or older. Can you confirm?

@yujiri8
Copy link
Author

@yujiri8 yujiri8 commented Feb 29, 2020

You were right, that was it. It works after updating goimports. Thanks for such a prompt response.

@yujiri8 yujiri8 closed this Feb 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.