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 <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/gopls/go.mod b/gopls/go.mod
index 86f3e7b..6495347 100644
--- a/gopls/go.mod
+++ b/gopls/go.mod
@@ -4,8 +4,9 @@
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 4580821..e0a4251 100644
--- a/gopls/go.sum
+++ b/gopls/go.sum
@@ -3,6 +3,8 @@
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/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/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 218689e..c864aa3 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 @@
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 4f200fc..792797b 100644
--- a/internal/lsp/regtest/formatting_test.go
+++ b/internal/lsp/regtest/formatting_test.go
@@ -30,14 +30,13 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 6999ba3..d7629ea 100644
--- a/internal/lsp/source/format.go
+++ b/internal/lsp/source/format.go
@@ -43,16 +43,27 @@
}
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 6a24242..88b840e 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 @@
// 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 @@
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 @@
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 8986c66..a075e6e 100644
--- a/internal/lsp/workspace.go
+++ b/internal/lsp/workspace.go
@@ -44,7 +44,7 @@
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
}
