From 62a0bb781d9f0ed535b3310c1d3b1616f9fa8ee3 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Sat, 11 Jul 2020 23:09:29 -0400 Subject: [PATCH] gopls, internal/lsp: support an extra formatting hook for gofumpt This change reworks CL 240118 to apply gofumpt directly as a formatter, not an analyzer. Depending on how gofumpt changes, we may be able to use it as an analyzer in the future, but for now it's just easier to add it as a formatting hook. Fixes golang/go#39805 Change-Id: I227fde4b1916d8a82557f30dfca88e155136dff5 Reviewed-on: https://go-review.googlesource.com/c/tools/+/241985 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- gopls/go.mod | 3 ++- gopls/go.sum | 5 +++++ gopls/internal/hooks/hooks.go | 5 +++++ internal/lsp/regtest/formatting_test.go | 14 ++++++-------- internal/lsp/source/format.go | 15 +++++++++++++-- internal/lsp/source/options.go | 8 ++++++++ internal/lsp/workspace.go | 2 +- 7 files changed, 40 insertions(+), 12 deletions(-) diff --git a/gopls/go.mod b/gopls/go.mod index 86f3e7b3e8..64953478ad 100644 --- a/gopls/go.mod +++ b/gopls/go.mod @@ -4,8 +4,9 @@ go 1.13 require ( github.com/sergi/go-diff v1.1.0 - golang.org/x/tools v0.0.0-20200515220128-d3bf790afa53 + golang.org/x/tools v0.0.0-20200626171337-aa94e735be7f honnef.co/go/tools v0.0.1-2020.1.4 + mvdan.cc/gofumpt v0.0.0-20200709182408-4fd085cb6d5f mvdan.cc/xurls/v2 v2.2.0 ) diff --git a/gopls/go.sum b/gopls/go.sum index 458082140a..e0a42514d8 100644 --- a/gopls/go.sum +++ b/gopls/go.sum @@ -3,6 +3,8 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/google/go-cmp v0.5.0 h1:/QaMHBdZ26BB3SSst0Iwl10Epc+xhTquomWX0oZEB6w= +github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= @@ -14,6 +16,7 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.5.2/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= +github.com/rogpeppe/go-internal v1.6.0/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0= github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= @@ -48,5 +51,7 @@ gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= honnef.co/go/tools v0.0.1-2020.1.4 h1:UoveltGrhghAA7ePc+e+QYDHXrBps2PqFZiHkGR/xK8= honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= +mvdan.cc/gofumpt v0.0.0-20200709182408-4fd085cb6d5f h1:gi7cb8HTDZ6q8VqsUpkdoFi3vxwHMneQ6+Q5Ap5hjPE= +mvdan.cc/gofumpt v0.0.0-20200709182408-4fd085cb6d5f/go.mod h1:9VQ397fNXEnF84t90W4r4TRCQK+pg9f8ugVfyj+S26w= mvdan.cc/xurls/v2 v2.2.0 h1:NSZPykBXJFCetGZykLAxaL6SIpvbVy/UFEniIfHAa8A= mvdan.cc/xurls/v2 v2.2.0/go.mod h1:EV1RMtya9D6G5DMYPGD8zTQzaHet6Jh8gFlRgGRJeO8= diff --git a/gopls/internal/hooks/hooks.go b/gopls/internal/hooks/hooks.go index 218689ea31..c864aa37c7 100644 --- a/gopls/internal/hooks/hooks.go +++ b/gopls/internal/hooks/hooks.go @@ -8,9 +8,11 @@ package hooks // import "golang.org/x/tools/gopls/internal/hooks" import ( + "context" "regexp" "golang.org/x/tools/internal/lsp/source" + "mvdan.cc/gofumpt/format" "mvdan.cc/xurls/v2" ) @@ -19,6 +21,9 @@ func Options(options *source.Options) { options.ComputeEdits = ComputeEdits } options.URLRegexp = urlRegexp() + options.GofumptFormat = func(ctx context.Context, src []byte) ([]byte, error) { + return format.Source(src, format.Options{}) + } updateAnalyzers(options) } diff --git a/internal/lsp/regtest/formatting_test.go b/internal/lsp/regtest/formatting_test.go index 4f200fc2f2..792797b574 100644 --- a/internal/lsp/regtest/formatting_test.go +++ b/internal/lsp/regtest/formatting_test.go @@ -30,14 +30,13 @@ func TestFormatting(t *testing.T) { got := env.Editor.BufferText("main.go") want := env.ReadWorkspaceFile("main.go.golden") if got != want { - t.Errorf("\n## got formatted file:\n%s\n## want:\n%s", got, want) + t.Errorf("unexpected formatting result:\n%s", tests.Diff(want, got)) } }) } // Tests golang/go#36824. func TestFormattingOneLine36824(t *testing.T) { - const onelineProgram = ` -- a.go -- package main; func f() {} @@ -53,14 +52,13 @@ func f() {} got := env.Editor.BufferText("a.go") want := env.ReadWorkspaceFile("a.go.formatted") if got != want { - t.Errorf("got\n%q wanted\n%q", got, want) + t.Errorf("unexpected formatting result:\n%s", tests.Diff(want, got)) } }) } // Tests golang/go#36824. func TestFormattingOneLineImports36824(t *testing.T) { - const onelineProgramA = ` -- a.go -- package x; func f() {fmt.Println()} @@ -78,7 +76,7 @@ func f() { fmt.Println() } got := env.Editor.BufferText("a.go") want := env.ReadWorkspaceFile("a.go.imported") if got != want { - t.Errorf("OneLineImports3824:\n%s", tests.Diff(want, got)) + t.Errorf("unexpected formatting result:\n%s", tests.Diff(want, got)) } }) } @@ -99,7 +97,7 @@ func f() {} got := env.Editor.BufferText("a.go") want := env.ReadWorkspaceFile("a.go.imported") if got != want { - t.Errorf("OneLineRmImports:\n%s", tests.Diff(want, got)) + t.Errorf("unexpected formatting result:\n%s", tests.Diff(want, got)) } }) } @@ -145,7 +143,7 @@ func TestOrganizeImports(t *testing.T) { got := env.Editor.BufferText("main.go") want := env.ReadWorkspaceFile("main.go.organized") if got != want { - t.Errorf("\n## got formatted file:\n%s\n## want:\n%s", got, want) + t.Errorf("unexpected formatting result:\n%s", tests.Diff(want, got)) } }) } @@ -157,7 +155,7 @@ func TestFormattingOnSave(t *testing.T) { got := env.Editor.BufferText("main.go") want := env.ReadWorkspaceFile("main.go.formatted") if got != want { - t.Errorf("\n## got formatted file:\n%s\n## want:\n%s", got, want) + t.Errorf("unexpected formatting result:\n%s", tests.Diff(want, got)) } }) } diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index 6999ba3137..d7629eaa95 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -43,16 +43,27 @@ func Format(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]protocol.T } fset := snapshot.View().Session().Cache().FileSet() - buf := &bytes.Buffer{} // format.Node changes slightly from one release to another, so the version // of Go used to build the LSP server will determine how it formats code. // This should be acceptable for all users, who likely be prompted to rebuild // the LSP server on each Go release. + buf := &bytes.Buffer{} if err := format.Node(buf, fset, file); err != nil { return nil, err } - return computeTextEdits(ctx, snapshot.View(), pgh.File(), m, buf.String()) + formatted := buf.String() + + // Apply additional formatting, if any is supported. Currently, the only + // supported additional formatter is gofumpt. + if format := snapshot.View().Options().Hooks.GofumptFormat; snapshot.View().Options().Gofumpt && format != nil { + b, err := format(ctx, buf.Bytes()) + if err != nil { + return nil, err + } + formatted = string(b) + } + return computeTextEdits(ctx, snapshot.View(), pgh.File(), m, formatted) } func formatSource(ctx context.Context, fh FileHandle) ([]byte, error) { diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 6a2424295a..88b840ea70 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -5,6 +5,7 @@ package source import ( + "context" "fmt" "os" "regexp" @@ -235,6 +236,9 @@ type UserOptions struct { // Placeholders adds placeholders to parameters and structs in completion // results. Placeholders bool + + // Gofumpt indicates if we should run gofumpt formatting. + Gofumpt bool } type completionOptions struct { @@ -257,6 +261,7 @@ type Hooks struct { DefaultAnalyzers map[string]Analyzer TypeErrorAnalyzers map[string]Analyzer ConvenienceAnalyzers map[string]Analyzer + GofumptFormat func(ctx context.Context, src []byte) ([]byte, error) } func (o Options) AddDefaultAnalyzer(a *analysis.Analyzer) { @@ -530,6 +535,9 @@ func (o *Options) set(name string, value interface{}) OptionResult { case "tempModfile": result.setBool(&o.TempModfile) + case "gofumpt": + result.setBool(&o.Gofumpt) + // Replaced settings. case "experimentalDisabledAnalyses": result.State = OptionDeprecated diff --git a/internal/lsp/workspace.go b/internal/lsp/workspace.go index 8986c66246..a075e6e3d6 100644 --- a/internal/lsp/workspace.go +++ b/internal/lsp/workspace.go @@ -44,7 +44,7 @@ func (s *Server) addView(ctx context.Context, name string, uri span.URI) (source func (s *Server) didChangeConfiguration(ctx context.Context, changed interface{}) error { // go through all the views getting the config for _, view := range s.session.Views() { - options := s.session.Options() + options := view.Options() if err := s.fetchConfig(ctx, view.Name(), view.Folder(), &options); err != nil { return err }