Skip to content

Commit 70d7433

Browse files
committed
revamp //gofumpt:diagnose for real use cases
Right now, it simply reports "(devel)" as the gofumpt version when gofumpt is used as a library via the format package, which is the case for many users like golangci-lint. To properly test this behavior, we add a gofumpt-external program which simply formats stdin with one option and prints to stdout. We `go install` this tool, which depends on a released gofumpt version. It also does not report the Go version, which is important as we rely on packages like go/printer, which can influence our behavior. Similar to the case above, we `go install` a released gofumpt version. It also does not use VCS information when it is the main module; add that as well, along with a test, borrowed from mvdan.cc/garble. While here, make the output easier to read by adding "version" and "flags" words as descriptions. Finally, we remove version.Print, which was an unnecessary wrapper. The following PR will update the released version of gofumpt being used, meaning that the "released" and "external" tests will pick up the improvement.
1 parent bf2870b commit 70d7433

File tree

8 files changed

+219
-30
lines changed

8 files changed

+219
-30
lines changed

format/format.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,9 @@ func (f *fumpter) applyPre(c *astutil.Cursor) {
394394
if comment.Text == "//gofumpt:diagnose" || strings.HasPrefix(comment.Text, "//gofumpt:diagnose ") {
395395
slc := []string{
396396
"//gofumpt:diagnose",
397+
"version:",
397398
version.String(),
399+
"flags:",
398400
"-lang=" + f.LangVersion,
399401
"-modpath=" + f.ModulePath,
400402
}

gofmt.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ func gofmtMain(s *sequencer) {
463463

464464
// Print the gofumpt version if the user asks for it.
465465
if *showVersion {
466-
version.Print()
466+
fmt.Println(version.String())
467467
return
468468
}
469469

internal/version/version.go

Lines changed: 84 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,28 +4,99 @@
44
package version
55

66
import (
7+
"encoding/json"
78
"fmt"
89
"os"
10+
"runtime"
911
"runtime/debug"
12+
"time"
13+
14+
"golang.org/x/mod/module"
1015
)
1116

12-
var version = "(devel)" // to match the default from runtime/debug
17+
// Note that this is not a main package, so a "var version" will not work with
18+
// our go-cross script which uses -ldflags=main.version=xxx.
1319

14-
func String() string {
15-
if testVersion := os.Getenv("GOFUMPT_VERSION_TEST"); testVersion != "" {
16-
return testVersion
20+
const ourModulePath = "mvdan.cc/gofumpt"
21+
22+
const fallbackVersion = "(devel)" // to match the default from runtime/debug
23+
24+
func findModule(info *debug.BuildInfo, modulePath string) *debug.Module {
25+
if info.Main.Path == modulePath {
26+
return &info.Main
1727
}
18-
// don't overwrite the version if it was set by -ldflags=-X
19-
if info, ok := debug.ReadBuildInfo(); ok && version == "(devel)" {
20-
mod := &info.Main
21-
if mod.Replace != nil {
22-
mod = mod.Replace
28+
for _, dep := range info.Deps {
29+
if dep.Path == modulePath {
30+
return dep
2331
}
24-
version = mod.Version
2532
}
26-
return version
33+
return nil
2734
}
2835

29-
func Print() {
30-
fmt.Println(String())
36+
func gofumptVersion() string {
37+
info, ok := debug.ReadBuildInfo()
38+
if !ok {
39+
return fallbackVersion // no build info available
40+
}
41+
// Note that gofumpt may be used as a library via the format package,
42+
// so we cannot assume it is the main module in the build.
43+
mod := findModule(info, ourModulePath)
44+
if mod == nil {
45+
return fallbackVersion // not found?
46+
}
47+
if mod.Replace != nil {
48+
mod = mod.Replace
49+
}
50+
51+
// If we found a meaningful version, we are done.
52+
// If gofumpt is not the main module, stop as well,
53+
// as VCS info is only for the main module.
54+
if mod.Version != "(devel)" || mod != &info.Main {
55+
return mod.Version
56+
}
57+
58+
// Fall back to trying to use VCS information.
59+
// Until https://github.com/golang/go/issues/50603 is implemented,
60+
// manually construct something like a pseudo-version.
61+
// TODO: remove when this code is dead, hopefully in Go 1.20.
62+
63+
// For the tests, as we don't want the VCS information to change over time.
64+
if v := os.Getenv("GARBLE_TEST_BUILDSETTINGS"); v != "" {
65+
var extra []debug.BuildSetting
66+
if err := json.Unmarshal([]byte(v), &extra); err != nil {
67+
panic(err)
68+
}
69+
info.Settings = append(info.Settings, extra...)
70+
}
71+
72+
var vcsTime time.Time
73+
var vcsRevision string
74+
for _, setting := range info.Settings {
75+
switch setting.Key {
76+
case "vcs.time":
77+
// If the format is invalid, we'll print a zero timestamp.
78+
vcsTime, _ = time.Parse(time.RFC3339Nano, setting.Value)
79+
case "vcs.revision":
80+
vcsRevision = setting.Value
81+
if len(vcsRevision) > 12 {
82+
vcsRevision = vcsRevision[:12]
83+
}
84+
}
85+
}
86+
if vcsRevision != "" {
87+
return module.PseudoVersion("", "", vcsTime, vcsRevision)
88+
}
89+
return fallbackVersion
90+
}
91+
92+
func goVersion() string {
93+
// For the tests, as we don't want the Go version to change over time.
94+
if testVersion := os.Getenv("GO_VERSION_TEST"); testVersion != "" {
95+
return testVersion
96+
}
97+
return runtime.Version()
98+
}
99+
100+
func String() string {
101+
return fmt.Sprintf("%s (%s)", gofumptVersion(), goVersion())
31102
}

main_test.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,14 @@
44
package main
55

66
import (
7+
"encoding/json"
78
"flag"
89
"os"
910
"path/filepath"
1011
"testing"
1112

1213
qt "github.com/frankban/quicktest"
14+
exec "golang.org/x/sys/execabs"
1315

1416
"github.com/rogpeppe/go-internal/gotooltest"
1517
"github.com/rogpeppe/go-internal/testscript"
@@ -25,11 +27,31 @@ var update = flag.Bool("u", false, "update testscript output files")
2527

2628
func TestScripts(t *testing.T) {
2729
t.Parallel()
30+
31+
var goEnv struct {
32+
GOCACHE string
33+
GOMODCACHE string
34+
GOMOD string
35+
}
36+
out, err := exec.Command("go", "env", "-json").CombinedOutput()
37+
if err != nil {
38+
t.Fatal(err)
39+
}
40+
if err := json.Unmarshal(out, &goEnv); err != nil {
41+
t.Fatal(err)
42+
}
43+
2844
p := testscript.Params{
2945
Dir: filepath.Join("testdata", "script"),
3046
UpdateScripts: *update,
47+
Setup: func(env *testscript.Env) error {
48+
env.Setenv("GOCACHE", goEnv.GOCACHE)
49+
env.Setenv("GOMODCACHE", goEnv.GOMODCACHE)
50+
env.Setenv("GOMOD_DIR", filepath.Dir(goEnv.GOMOD))
51+
return nil
52+
},
3153
}
32-
err := gotooltest.Setup(&p)
54+
err = gotooltest.Setup(&p)
3355
qt.Assert(t, err, qt.IsNil)
3456
testscript.Run(t, p)
3557
}

testdata/gofumpt-external/go.mod

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
module test/gofumpt-external
2+
3+
go 1.18
4+
5+
require mvdan.cc/gofumpt v0.3.2-0.20220627183521-8dda8068d9f3
6+
7+
require (
8+
github.com/google/go-cmp v0.5.8 // indirect
9+
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
10+
golang.org/x/tools v0.1.11 // indirect
11+
)

testdata/gofumpt-external/go.sum

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
github.com/frankban/quicktest v1.14.3 h1:FJKSZTDHjyhriyC81FLQ0LY93eSai0ZyR/ZIkd3ZUKE=
2+
github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
3+
github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
4+
github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0=
5+
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
6+
github.com/rogpeppe/go-internal v1.8.2-0.20220624104257-af73bbc5c731 h1:LkP6LNQyXrQoVVXMpb+sbb0iibcaOZH97feeh778heA=
7+
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s=
8+
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
9+
golang.org/x/sys v0.0.0-20220624220833-87e55d714810 h1:rHZQSjJdAI4Xf5Qzeh2bBc5YJIkPFVM6oDtMFYmgws0=
10+
golang.org/x/tools v0.1.11 h1:loJ25fNOEhSXfHrpoGj91eCUThwdNX6u24rO1xnNteY=
11+
golang.org/x/tools v0.1.11/go.mod h1:SgwaegtQh8clINPpECJMqnxLv9I09HLqnW3RMqW0CA4=
12+
mvdan.cc/gofumpt v0.3.2-0.20220627183521-8dda8068d9f3 h1:Kk6yeaipDobigAr+NZqq37LjLN1q3LHSOpcaDLDwyoI=
13+
mvdan.cc/gofumpt v0.3.2-0.20220627183521-8dda8068d9f3/go.mod h1:TSc7K1qXnyCCK7LUmDAWp4UMntOys3CzN8ksMKaxFrE=

testdata/gofumpt-external/main.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package main
2+
3+
import (
4+
"io"
5+
"os"
6+
7+
"mvdan.cc/gofumpt/format"
8+
)
9+
10+
func main() {
11+
orig, err := io.ReadAll(os.Stdin)
12+
if err != nil {
13+
panic(err)
14+
}
15+
formatted, err := format.Source(orig, format.Options{
16+
LangVersion: "v1.16",
17+
})
18+
if err != nil {
19+
panic(err)
20+
}
21+
os.Stdout.Write(formatted)
22+
}

testdata/script/diagnose.txtar

Lines changed: 63 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,21 @@
1-
cp foo.go foo.go.orig
2-
env GOFUMPT_VERSION_TEST=v0.1.1-0.20210524140936-ba6406b58f36
1+
env GO_VERSION_TEST=go1.18.29
32

4-
gofumpt -w foo.go
5-
cmp foo.go foo.go.golden
3+
# First, test a local build of gofumpt resulting from 'git clone'.
4+
# Its version will be inferred from VCS, but since we want a stable test,
5+
# we mock the VCS information. Note that test binaries do not have VCS info.
6+
# Data obtained from a real build while developing.
7+
env GARBLE_TEST_BUILDSETTINGS='[{"Key":"vcs","Value":"git"},{"Key":"vcs.revision","Value":"8dda8068d9f339047fc1777b688afb66a0a0db17"},{"Key":"vcs.time","Value":"2022-07-27T15:58:40Z"},{"Key":"vcs.modified","Value":"true"}]'
8+
gofumpt foo.go
9+
cmp stdout foo.go.golden
610

7-
gofumpt -w outdated.go
8-
cmp outdated.go foo.go.golden
11+
gofumpt outdated.go
12+
cmp stdout foo.go.golden
913

10-
cp foo.go.orig foo.go
11-
gofumpt -w -extra foo.go
12-
cmp foo.go foo.go.golden-extra
14+
gofumpt -extra foo.go
15+
cmp stdout foo.go.golden-extra
1316

14-
cp foo.go.orig foo.go
15-
gofumpt -w -lang=v1 foo.go
16-
cmp foo.go foo.go.golden-lang
17+
gofumpt -lang=v1 foo.go
18+
cmp stdout foo.go.golden-lang
1719

1820
gofumpt -d nochange.go
1921
! stdout .
@@ -24,6 +26,40 @@ gofumpt -d foo.go.golden
2426
gofumpt -d -extra foo.go.golden-extra
2527
! stdout .
2628

29+
# A local build without VCS information will result in a missing version.
30+
env GARBLE_TEST_BUILDSETTINGS='[]'
31+
gofumpt foo.go
32+
cmp stdout foo.go.golden-devel
33+
34+
[short] stop 'the rest of this test builds gofumpt binaries'
35+
36+
# We want a published version of gofumpt on the public module proxies,
37+
# because that's the only way that its module version will be included.
38+
# Using a directory replace directive will not work.
39+
# This means that any change in how gofumpt reports its own version
40+
# will require two pull requests, the second one updating the test script.
41+
# We could consider using go-internal/goproxytest, but then we would need to
42+
# manually run something like go-internal/cmd/txtar-addmod reguarly.
43+
# Or teach goproxytest to serve a mock version of gofumpt from its local checkout.
44+
# Either way, both are relatively overkill for now.
45+
env GOBIN=${WORK}/bin
46+
env GOFUMPT_PUBLISHED_VERSION=v0.3.2-0.20220627183521-8dda8068d9f3
47+
48+
# TODO: update these once the library fix hits master
49+
50+
# gofumpt as the main binary with a real module version.
51+
go install mvdan.cc/gofumpt@${GOFUMPT_PUBLISHED_VERSION}
52+
exec ${GOBIN}/gofumpt foo.go
53+
cmp stdout foo.go.golden-released
54+
55+
# gofumpt as a library with a real module version.
56+
cd ${GOMOD_DIR}/testdata/gofumpt-external
57+
go install .
58+
cd ${WORK}
59+
stdin foo.go
60+
exec ${GOBIN}/gofumpt-external
61+
cmp stdout foo.go.golden-external
62+
2763
-- go.mod --
2864
module test
2965

@@ -43,12 +79,24 @@ package p
4379
-- foo.go.golden --
4480
package p
4581

46-
//gofumpt:diagnose v0.1.1-0.20210524140936-ba6406b58f36 -lang=v1.16 -modpath=test
82+
//gofumpt:diagnose version: v0.0.0-20220727155840-8dda8068d9f3 (go1.18.29) flags: -lang=v1.16 -modpath=test
83+
-- foo.go.golden-devel --
84+
package p
85+
86+
//gofumpt:diagnose version: (devel) (go1.18.29) flags: -lang=v1.16 -modpath=test
4787
-- foo.go.golden-extra --
4888
package p
4989

50-
//gofumpt:diagnose v0.1.1-0.20210524140936-ba6406b58f36 -lang=v1.16 -modpath=test -extra
90+
//gofumpt:diagnose version: v0.0.0-20220727155840-8dda8068d9f3 (go1.18.29) flags: -lang=v1.16 -modpath=test -extra
5191
-- foo.go.golden-lang --
5292
package p
5393

54-
//gofumpt:diagnose v0.1.1-0.20210524140936-ba6406b58f36 -lang=v1 -modpath=test
94+
//gofumpt:diagnose version: v0.0.0-20220727155840-8dda8068d9f3 (go1.18.29) flags: -lang=v1 -modpath=test
95+
-- foo.go.golden-released --
96+
package p
97+
98+
//gofumpt:diagnose v0.3.2-0.20220627183521-8dda8068d9f3 -lang=v1.16 -modpath=test
99+
-- foo.go.golden-external --
100+
package p
101+
102+
//gofumpt:diagnose (devel) -lang=v1.16 -modpath=

0 commit comments

Comments
 (0)