Skip to content

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

Closed

Description

@thaJeztah

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;

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

func main() {
	_ = map[string]string{
		"a": "",
		"123456789012345678": "",
	}
	_ = map[string]string{
		"a": "",
		"1234567890123456789": "",
	}
}
  1. 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 🤗

Activity

  1. mvdan commented on Jul 5, 2018

    @mvdan
    Member

    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.

  2. changed the title [-]gofmt: threshold for aligning values changed in Go 1.11beta1[/-] [+]doc: document changes in behavior of gofmt in 1.11[/+] on Jul 5, 2018
  3. added this to the Go1.11 milestone on Jul 5, 2018
  4. added
    NeedsFixThe path to resolution is known, but the work has not been done.
    on Jul 5, 2018
  5. thaJeztah commented on Jul 5, 2018

    @thaJeztah
    ContributorAuthor

    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 😕 😅
  6. mvdan commented on Jul 5, 2018

    @mvdan
    Member

    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.

  7. thaJeztah commented on Jul 5, 2018

    @thaJeztah
    ContributorAuthor

    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 👍

  8. mvdan commented on Jul 5, 2018

    @mvdan
    Member

    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.

  9. thaJeztah commented on Jul 5, 2018

    @thaJeztah
    ContributorAuthor

    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

  10. mvdan commented on Jul 5, 2018

    @mvdan
    Member

    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.

  11. thaJeztah commented on Jul 5, 2018

    @thaJeztah
    ContributorAuthor

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

  12. gopherbot commented on Jul 5, 2018

    @gopherbot
    Contributor

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

  13. kortschak commented on Sep 9, 2018

    @kortschak
    Contributor

    @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.

  14. mvdan commented on Sep 9, 2018

    @mvdan
    Member

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

  15. locked and limited conversation to collaborators on Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    DocumentationIssues describing a change to documentation.FrozenDueToAgeNeedsFixThe path to resolution is known, but the work has not been done.

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions