doc: document changes in behavior of gofmt in 1.11 #26228

thaJeztah · 2018-07-05T10:42:38Z

Please answer these questions before submitting your issue. Thanks!

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

go version go1.11beta1 linux/amd64

Does this issue reproduce with the latest release?

yes, latest beta

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build926389312=/tmp/go-build -gno-record-gcc-switches"
VGOMODROOT=""

What did you do?

Running gofmt in Go 1.11beta1 causes a diff 😄

To reproduce;

Create a file example.go, with the following content;

package main

func main() {
	_ = map[string]string{
		"a": "",
		"123456789012345678": "",
	}
	_ = map[string]string{
		"a": "",
		"1234567890123456789": "",
	}
}

gofmt the file;

gofmt -s -d ./example.go

What did you expect to see?

On Go 1.10.3, only the first map is re-formatted (the threshold for aligning values lies at 19 characters)

--- example.go.orig	2018-07-05 10:23:56.358365143 +0000
+++ example.go	2018-07-05 10:23:56.358365143 +0000
@@ -2,7 +2,7 @@
 
 func main() {
 	_ = map[string]string{
-		"a": "",
+		"a":                  "",
 		"123456789012345678": "",
 	}
 	_ = map[string]string{

What did you see instead?

On Go 1.11beta1, both maps are re-formatted (the threshold for aligning values lies at 18 characters):

--- example.go.orig	2018-07-05 10:24:32.709844185 +0000
+++ example.go	2018-07-05 10:24:32.709844185 +0000
@@ -2,11 +2,11 @@
 
 func main() {
 	_ = map[string]string{
-		"a": "",
+		"a":                  "",
 		"123456789012345678": "",
 	}
 	_ = map[string]string{
-		"a": "",
+		"a":                   "",
 		"1234567890123456789": "",
 	}
 }

This change complicates projects that test against multiple versions of Go (see related issues moby/moby#37358, containerd/containerd#2436)

If this was a deliberate change, please add a mention of this in the changelog 🤗

The text was updated successfully, but these errors were encountered:

mvdan · 2018-07-05T10:52:48Z

This change was done on purpose in 542ea5a.

As to why gofmt changes every release - see @griesemer's comment in #25161 (comment).

However, I agree that this change should be documented in the release notes. I can't see it in https://tip.golang.org/doc/go1.11.

thaJeztah · 2018-07-05T11:05:33Z

Oh! actually found that commit, but see I forgot to include it in my description (thanks!)

hm, alright, so I understand Go should not stop progression; just thinking what the best approach is for projects that test against, and support multiple go versions;

for CI; exclude gofmt check on version X
more problematic; accepting contributions from users running different versions of Go 😕 😅

mvdan · 2018-07-05T11:10:17Z

Yes - if a project wants to enforce gofmt via CI, it should stick to exactly one version of gofmt. That tends to be the latest stable version, such as 1.10 now.

The other option is to not enforce it via CI, and instead enforce it via pre-commit checks and by humans, like Go itself does.

thaJeztah · 2018-07-05T11:22:50Z

True; just that, pre-commit checks won't resolve accepting contributions from users running different versions of go. In an ideal world, every project would switch to (and be compatible with) latest stable Go the moment it's released. Although upgrading Go versions has become less painful over the years, but we're not there yet, so unfortunately this isn't always possible (hence, contributors pinning to different versions of Go).

Thanks for your suggestions; it's appreciated 👍

mvdan · 2018-07-05T11:31:33Z

I agree that enforcing this server-side instead of client-side is better. Perhaps it should be made easier for developers to download specific versions of gofmt, so that they could use gofmt1.10 for work projects even if they were using 1.9 or 1.11beta1 (master) on their laptop.

gofmt could even include a version flag, so that scripts could check if the right version is being used. Although it might be a bad idea for the gofmt that ships with Go itself to report a version, equal but separate from go version.

#22695 didn't seem to reach any conclusion that would help with this. I might open a proposal issue along the lines of what I described above, once this documentation issue is fixed.

thaJeztah · 2018-07-05T12:01:07Z

gofmt could even include a version flag, so that scripts could check if the right version is being used. Although it might be a bad idea for the gofmt that ships with Go itself to report a version, equal but separate from go version.

Yes, while writing my previous comment, I wanted to suggest a "version" option, so that a different gofmt could be used to ease a project's transition between Go versions. I decided not to, because doing so may defeat the whole purpose of gofmt (end all discussions around code-style/formatting).

Having said the above, perhaps there's still merit in that option; introduce an option to select a "slow" version of gofmt, limited to versions of Go that are actively maintained;

For example, currently, Go 1.11 (next stable), 1.10.x (current stable), and 1.9.x (previous stable) are actively maintained; I think it's ok to assume (most) projects that support multiple Go versions won't Go beyond those versions

Could look something like this; (gofmt [--version=<option>]);

Option	Description
(not set)	default; use `gofmt` of the installed version
`stable`	use current stable; same as default under normal circumstances, but can be useful when running `go tip` or a beta
`previous-stable`	use previous stable; useful during migration between versions, and if a project supports both "current" and "previous" stable

edit: removed --version=current option from the table, as it didn't make much sense

mvdan · 2018-07-05T12:07:02Z

When I mentioned a version flag, I meant just a flag to print its version :) Then, scripts could do something like:

gofmt="gofmt"
if [[ $(gofmt -version) != go1.10.* ]]; then
    # download gofmt1.10
    gofmt="gofmt1.10"
fi
# use $gofmt

I'll send a proposal about this in a few days. @thaJeztah if you want, I can email you a draft before publishing.

thaJeztah · 2018-07-05T12:30:31Z

Looks like that would indeed help 👍 You can /cc me at sebastiaan <swirly-thingy> docker.com

gopherbot · 2018-07-05T13:19:45Z

Change https://golang.org/cl/122295 mentions this issue: doc: explain minor change to gofmt in go1.11

golang/go#26228

kortschak · 2018-09-09T11:51:18Z

@mvdan

The other option is to not enforce it via CI, and instead enforce it via pre-commit checks and by humans, like Go itself does.

Yes, Russ has made this comment too in the past. Pre-commit checks don't work well in practice unless there is either a requirement that contributors run a Makefile to install the hooks or there is a bespoke codereview tool as exists for Go contributions. These are not necessarily feasible for small open source projects (a good example is Gonum where we already have difficulty managing new and drive-by contributions).

I'll just go and eat my brioche.

mvdan · 2018-09-09T17:38:33Z

@kortschak please note that this issue is closed, and that I filed a proposal precisely to make enforcing gofmt easier - #26397.

go 1.11's "go fmt" has a backwards incompatible formatting change due to golang/go#26228 which only affects the csi/csi_test.go file. This is a temporary workaround for difference in go fmt between 1.9/1.10 and 1.11+, to fix travis. We can remove this when we drop go 1.9 and go 1.10 from travis. Signed-off-by: Craig Rodrigues <craig@portworx.com>

This was referenced Jul 5, 2018

Workaround for gofmt change in Go 1.11 containerd/containerd#2436

Merged

Bump to Go 1.11.0 containerd/containerd#2435

Merged

Bump golang to 1.11.0 moby/moby#37358

Merged

mvdan changed the title ~~gofmt: threshold for aligning values changed in Go 1.11beta1~~ doc: document changes in behavior of gofmt in 1.11 Jul 5, 2018

gopherbot added the Documentation label Jul 5, 2018

mvdan added this to the Go1.11 milestone Jul 5, 2018

gopherbot added the Documentation label Jul 5, 2018

mvdan added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 5, 2018

agnivade mentioned this issue Jul 12, 2018

cmd/gofmt: empty line doesn't reset value alignment #26352

Closed

mvdan mentioned this issue Jul 16, 2018

dl: add versioned gofmts #26397

Open

dmitris mentioned this issue Jul 20, 2018

fix go vet error with go1.11 google/go-github#954

Merged

gopherbot closed this as completed in 8898097 Jul 20, 2018

menghanl mentioned this issue Aug 6, 2018

regenerate generated code grpc/grpc-go#2255

Closed

dvyukov mentioned this issue Aug 15, 2018

[fuchsia] Change QEMU args to use multiboot shim. google/syzkaller#681

Closed

agnivade mentioned this issue Aug 16, 2018

go/format: go1.11 gofmt regression in map entries indent #27025

Closed

dvyukov mentioned this issue Aug 17, 2018

[doc] Update contributing instructions. google/syzkaller#685

Merged

Hexcles mentioned this issue Aug 20, 2018

Lock the version of gofmt or stop using it as a lint check web-platform-tests/wpt.fyi#459

Closed

suzuki-shunsuke added a commit to suzuki-shunsuke/go-graylog that referenced this issue Sep 3, 2018

ci: upgrade ci's golang to 1.11 to prevent from lint error

4aeece9

golang/go#26228

adarobin mentioned this issue Dec 19, 2018

Add Synergy Support for Logical Interconnect Groups HewlettPackard/terraform-provider-oneview#56

Merged

gliptak mentioned this issue Jan 5, 2019

Bring Go current for Travis PaloAltoNetworks/terraform-provider-panos#99

Closed

adrien-f mentioned this issue Jan 19, 2019

chore: added formatting check to circle-ci thanos-io/thanos#733

Closed

elfosardo mentioned this issue Feb 22, 2019

Changes in behavior of go fmt in 1.11 gophercloud/gophercloud#1465

Closed

fharding1 mentioned this issue May 27, 2019

Improve CORS Method Middleware gorilla/mux#477

Merged

igm mentioned this issue Jun 21, 2019

Run gofmt on session.go igm/sockjs-go#74

Closed

golang locked and limited conversation to collaborators Sep 9, 2019

gopherbot added the FrozenDueToAge label Sep 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: document changes in behavior of gofmt in 1.11 #26228

doc: document changes in behavior of gofmt in 1.11 #26228

thaJeztah commented Jul 5, 2018

mvdan commented Jul 5, 2018

thaJeztah commented Jul 5, 2018

mvdan commented Jul 5, 2018

thaJeztah commented Jul 5, 2018

mvdan commented Jul 5, 2018

thaJeztah commented Jul 5, 2018 •

edited

mvdan commented Jul 5, 2018

thaJeztah commented Jul 5, 2018

gopherbot commented Jul 5, 2018

kortschak commented Sep 9, 2018

mvdan commented Sep 9, 2018

doc: document changes in behavior of gofmt in 1.11 #26228

doc: document changes in behavior of gofmt in 1.11 #26228

Comments

thaJeztah commented Jul 5, 2018

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

mvdan commented Jul 5, 2018

thaJeztah commented Jul 5, 2018

mvdan commented Jul 5, 2018

thaJeztah commented Jul 5, 2018

mvdan commented Jul 5, 2018

thaJeztah commented Jul 5, 2018 • edited

mvdan commented Jul 5, 2018

thaJeztah commented Jul 5, 2018

gopherbot commented Jul 5, 2018

kortschak commented Sep 9, 2018

mvdan commented Sep 9, 2018

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

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

thaJeztah commented Jul 5, 2018 •

edited