Skip to content

Commit 5210e0c

Browse files
committed
gopls: wire in LangVersion and ModulePath for gofumpt formatting
If available, pass the LangVersion and ModulePath provided by go/packages to gofumpt. This allows gofumpt to apply additional formatting rules that require this information. Also add a regression test for gofumpt formatting. Fixes golang/go#51327 Change-Id: I47c8c96d595d62e1c444285ce69ce6a4e61fa74c Reviewed-on: https://go-review.googlesource.com/c/tools/+/387634 Trust: Robert Findley <[email protected]> Run-TryBot: Robert Findley <[email protected]> Trust: Daniel Martí <[email protected]> gopls-CI: kokoro <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent e6ef770 commit 5210e0c

File tree

7 files changed

+111
-15
lines changed

7 files changed

+111
-15
lines changed

gopls/internal/hooks/hooks.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,10 @@ func Options(options *source.Options) {
2222
options.ComputeEdits = ComputeEdits
2323
}
2424
options.URLRegexp = relaxedFullWord
25-
options.GofumptFormat = func(ctx context.Context, src []byte) ([]byte, error) {
25+
options.GofumptFormat = func(ctx context.Context, langVersion, modulePath string, src []byte) ([]byte, error) {
2626
return format.Source(src, format.Options{
27-
// TODO: fill LangVersion and ModulePath from the current go.mod.
28-
// The information is availabe as loaded by go/packages via the
29-
// Module type, but it needs to be wired up all the way here.
30-
// We likely want to change the GofumptFormat field type to take
31-
// extra parameters, to then be used in format.Options.
32-
// See https://pkg.go.dev/mvdan.cc/[email protected]/format#Options.
27+
LangVersion: langVersion,
28+
ModulePath: modulePath,
3329
})
3430
}
3531
updateAnalyzers(options)

gopls/internal/regtest/misc/formatting_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,3 +301,69 @@ func main() {
301301
}
302302
})
303303
}
304+
305+
func TestGofumptFormatting(t *testing.T) {
306+
307+
// Exercise some gofumpt formatting rules:
308+
// - No empty lines following an assignment operator
309+
// - Octal integer literals should use the 0o prefix on modules using Go
310+
// 1.13 and later. Requires LangVersion to be correctly resolved.
311+
// - std imports must be in a separate group at the top. Requires ModulePath
312+
// to be correctly resolved.
313+
const input = `
314+
-- go.mod --
315+
module foo
316+
317+
go 1.17
318+
-- foo.go --
319+
package foo
320+
321+
import (
322+
"foo/bar"
323+
"fmt"
324+
)
325+
326+
const perm = 0755
327+
328+
func foo() {
329+
foo :=
330+
"bar"
331+
fmt.Println(foo, bar.Bar)
332+
}
333+
-- foo.go.formatted --
334+
package foo
335+
336+
import (
337+
"fmt"
338+
339+
"foo/bar"
340+
)
341+
342+
const perm = 0o755
343+
344+
func foo() {
345+
foo := "bar"
346+
fmt.Println(foo, bar.Bar)
347+
}
348+
-- bar/bar.go --
349+
package bar
350+
351+
const Bar = 42
352+
`
353+
354+
WithOptions(
355+
EditorConfig{
356+
Settings: map[string]interface{}{
357+
"gofumpt": true,
358+
},
359+
},
360+
).Run(t, input, func(t *testing.T, env *Env) {
361+
env.OpenFile("foo.go")
362+
env.FormatBuffer("foo.go")
363+
got := env.Editor.BufferText("foo.go")
364+
want := env.ReadWorkspaceFile("foo.go.formatted")
365+
if got != want {
366+
t.Errorf("unexpected formatting result:\n%s", tests.Diff(t, want, got))
367+
}
368+
})
369+
}

internal/lsp/cache/metadata.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ func (m *Metadata) PackagePath() string {
5656
return string(m.PkgPath)
5757
}
5858

59+
// ModuleInfo implements the source.Metadata interface.
60+
func (m *Metadata) ModuleInfo() *packages.Module {
61+
return m.Module
62+
}
63+
5964
// KnownMetadata is a wrapper around metadata that tracks its validity.
6065
type KnownMetadata struct {
6166
*Metadata

internal/lsp/cache/snapshot.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1025,7 +1025,11 @@ func (s *snapshot) MetadataForFile(ctx context.Context, uri span.URI) ([]source.
10251025
var mds []source.Metadata
10261026
for _, id := range knownIDs {
10271027
md := s.getMetadata(id)
1028-
mds = append(mds, md)
1028+
// TODO(rfindley): knownIDs and metadata should be in sync, but existing
1029+
// code is defensive of nil metadata.
1030+
if md != nil {
1031+
mds = append(mds, md)
1032+
}
10291033
}
10301034
return mds, nil
10311035
}

internal/lsp/source/format.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,24 @@ func Format(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]protocol.T
6363

6464
// Apply additional formatting, if any is supported. Currently, the only
6565
// supported additional formatter is gofumpt.
66-
if format := snapshot.View().Options().Hooks.GofumptFormat; snapshot.View().Options().Gofumpt && format != nil {
67-
b, err := format(ctx, buf.Bytes())
66+
if format := snapshot.View().Options().GofumptFormat; snapshot.View().Options().Gofumpt && format != nil {
67+
// gofumpt can customize formatting based on language version and module
68+
// path, if available.
69+
//
70+
// Try to derive this information, but fall-back on the default behavior.
71+
//
72+
// TODO: under which circumstances can we fail to find module information?
73+
// Can this, for example, result in inconsistent formatting across saves,
74+
// due to pending calls to packages.Load?
75+
var langVersion, modulePath string
76+
mds, err := snapshot.MetadataForFile(ctx, fh.URI())
77+
if err == nil && len(mds) > 0 {
78+
if mi := mds[0].ModuleInfo(); mi != nil {
79+
langVersion = mi.GoVersion
80+
modulePath = mi.Path
81+
}
82+
}
83+
b, err := format(ctx, langVersion, modulePath, buf.Bytes())
6884
if err != nil {
6985
return nil, err
7086
}

internal/lsp/source/options.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -462,11 +462,16 @@ func (u *UserOptions) SetEnvSlice(env []string) {
462462
// Hooks contains configuration that is provided to the Gopls command by the
463463
// main package.
464464
type Hooks struct {
465-
LicensesText string
466-
GoDiff bool
467-
ComputeEdits diff.ComputeEdits
468-
URLRegexp *regexp.Regexp
469-
GofumptFormat func(ctx context.Context, src []byte) ([]byte, error)
465+
LicensesText string
466+
GoDiff bool
467+
ComputeEdits diff.ComputeEdits
468+
URLRegexp *regexp.Regexp
469+
470+
// GofumptFormat allows the gopls module to wire-in a call to
471+
// gofumpt/format.Source. langVersion and modulePath are used for some
472+
// Gofumpt formatting rules -- see the Gofumpt documentation for details.
473+
GofumptFormat func(ctx context.Context, langVersion, modulePath string, src []byte) ([]byte, error)
474+
470475
DefaultAnalyzers map[string]*Analyzer
471476
TypeErrorAnalyzers map[string]*Analyzer
472477
ConvenienceAnalyzers map[string]*Analyzer

internal/lsp/source/view.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"golang.org/x/mod/modfile"
1919
"golang.org/x/mod/module"
2020
"golang.org/x/tools/go/analysis"
21+
"golang.org/x/tools/go/packages"
2122
"golang.org/x/tools/internal/gocommand"
2223
"golang.org/x/tools/internal/imports"
2324
"golang.org/x/tools/internal/lsp/progress"
@@ -319,6 +320,9 @@ type Metadata interface {
319320

320321
// PackagePath is the package path.
321322
PackagePath() string
323+
324+
// ModuleInfo returns the go/packages module information for the given package.
325+
ModuleInfo() *packages.Module
322326
}
323327

324328
// Session represents a single connection from a client.

0 commit comments

Comments
 (0)