Skip to content

Commit 49e4010

Browse files
committed
internal/lsp/regtest: implement formatting and organizeImports
Add two new fake editor commands: Formatting and OrganizeImports, which delegate to textDocument/formatting and textDocument/codeAction respectively. Use this in simple regtests, as well as on save. Implementing this required fixing a broken assumption about text edits in the editor: previously these edits were incrementally mutating the buffer, but the correct implementation should simultaneously mutate the buffer (i.e., all positions in an edit set refer to the starting buffer state). This never mattered before because we were only operating on one edit at a time. Updates golang/go#36879 Change-Id: I6dec343c4e202288fa20c26df2fbafe9340a1bce Reviewed-on: https://go-review.googlesource.com/c/tools/+/221539 Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Rohan Challa <[email protected]>
1 parent 9aa23ab commit 49e4010

File tree

7 files changed

+280
-61
lines changed

7 files changed

+280
-61
lines changed

Diff for: internal/lsp/fake/client.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,7 @@ func (c *Client) ApplyEdit(ctx context.Context, params *protocol.ApplyWorkspaceE
9898
}
9999
for _, change := range params.Edit.DocumentChanges {
100100
path := c.ws.URIToPath(change.TextDocument.URI)
101-
var edits []Edit
102-
for _, lspEdit := range change.Edits {
103-
edits = append(edits, fromProtocolTextEdit(lspEdit))
104-
}
101+
edits := convertEdits(change.Edits)
105102
c.EditBuffer(ctx, path, edits)
106103
}
107104
return &protocol.ApplyWorkspaceEditResponse{Applied: true}, nil

Diff for: internal/lsp/fake/edit.go

+45-14
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package fake
66

77
import (
88
"fmt"
9+
"sort"
910
"strings"
1011

1112
"golang.org/x/tools/internal/lsp/protocol"
@@ -71,7 +72,7 @@ func inText(p Pos, content []string) bool {
7172
}
7273
// Note the strict right bound: the column indexes character _separators_,
7374
// not characters.
74-
if p.Column < 0 || p.Column > len(content[p.Line]) {
75+
if p.Column < 0 || p.Column > len([]rune(content[p.Line])) {
7576
return false
7677
}
7778
return true
@@ -80,20 +81,50 @@ func inText(p Pos, content []string) bool {
8081
// editContent implements a simplistic, inefficient algorithm for applying text
8182
// edits to our buffer representation. It returns an error if the edit is
8283
// invalid for the current content.
83-
func editContent(content []string, edit Edit) ([]string, error) {
84-
if edit.End.Line < edit.Start.Line || (edit.End.Line == edit.Start.Line && edit.End.Column < edit.Start.Column) {
85-
return nil, fmt.Errorf("invalid edit: end %v before start %v", edit.End, edit.Start)
84+
func editContent(content []string, edits []Edit) ([]string, error) {
85+
newEdits := make([]Edit, len(edits))
86+
copy(newEdits, edits)
87+
sort.Slice(newEdits, func(i, j int) bool {
88+
if newEdits[i].Start.Line < newEdits[j].Start.Line {
89+
return true
90+
}
91+
if newEdits[i].Start.Line > newEdits[j].Start.Line {
92+
return false
93+
}
94+
return newEdits[i].Start.Column < newEdits[j].Start.Column
95+
})
96+
97+
// Validate edits.
98+
for _, edit := range newEdits {
99+
if edit.End.Line < edit.Start.Line || (edit.End.Line == edit.Start.Line && edit.End.Column < edit.Start.Column) {
100+
return nil, fmt.Errorf("invalid edit: end %v before start %v", edit.End, edit.Start)
101+
}
102+
if !inText(edit.Start, content) {
103+
return nil, fmt.Errorf("start position %v is out of bounds", edit.Start)
104+
}
105+
if !inText(edit.End, content) {
106+
return nil, fmt.Errorf("end position %v is out of bounds", edit.End)
107+
}
86108
}
87-
if !inText(edit.Start, content) {
88-
return nil, fmt.Errorf("start position %v is out of bounds", edit.Start)
109+
110+
var (
111+
b strings.Builder
112+
line, column int
113+
)
114+
advance := func(toLine, toColumn int) {
115+
for ; line < toLine; line++ {
116+
b.WriteString(string([]rune(content[line])[column:]) + "\n")
117+
column = 0
118+
}
119+
b.WriteString(string([]rune(content[line])[column:toColumn]))
120+
column = toColumn
89121
}
90-
if !inText(edit.End, content) {
91-
return nil, fmt.Errorf("end position %v is out of bounds", edit.End)
122+
for _, edit := range newEdits {
123+
advance(edit.Start.Line, edit.Start.Column)
124+
b.WriteString(edit.Text)
125+
line = edit.End.Line
126+
column = edit.End.Column
92127
}
93-
// Splice the edit text in between the first and last lines of the edit.
94-
prefix := string([]rune(content[edit.Start.Line])[:edit.Start.Column])
95-
suffix := string([]rune(content[edit.End.Line])[edit.End.Column:])
96-
newLines := strings.Split(prefix+edit.Text+suffix, "\n")
97-
newContent := append(content[:edit.Start.Line], newLines...)
98-
return append(newContent, content[edit.End.Line+1:]...), nil
128+
advance(len(content)-1, len([]rune(content[len(content)-1])))
129+
return strings.Split(b.String(), "\n"), nil
99130
}

Diff for: internal/lsp/fake/edit_test.go

+13-13
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ func TestApplyEdit(t *testing.T) {
1313
tests := []struct {
1414
label string
1515
content string
16-
edit Edit
16+
edits []Edit
1717
want string
1818
wantErr bool
1919
}{
@@ -23,57 +23,57 @@ func TestApplyEdit(t *testing.T) {
2323
{
2424
label: "empty edit",
2525
content: "hello",
26-
edit: Edit{},
26+
edits: []Edit{},
2727
want: "hello",
2828
},
2929
{
3030
label: "unicode edit",
3131
content: "hello, 日本語",
32-
edit: Edit{
32+
edits: []Edit{{
3333
Start: Pos{Line: 0, Column: 7},
3434
End: Pos{Line: 0, Column: 10},
3535
Text: "world",
36-
},
36+
}},
3737
want: "hello, world",
3838
},
3939
{
4040
label: "range edit",
4141
content: "ABC\nDEF\nGHI\nJKL",
42-
edit: Edit{
42+
edits: []Edit{{
4343
Start: Pos{Line: 1, Column: 1},
4444
End: Pos{Line: 2, Column: 3},
4545
Text: "12\n345",
46-
},
46+
}},
4747
want: "ABC\nD12\n345\nJKL",
4848
},
4949
{
5050
label: "end before start",
5151
content: "ABC\nDEF\nGHI\nJKL",
52-
edit: Edit{
52+
edits: []Edit{{
5353
End: Pos{Line: 1, Column: 1},
5454
Start: Pos{Line: 2, Column: 3},
5555
Text: "12\n345",
56-
},
56+
}},
5757
wantErr: true,
5858
},
5959
{
6060
label: "out of bounds line",
6161
content: "ABC\nDEF\nGHI\nJKL",
62-
edit: Edit{
62+
edits: []Edit{{
6363
Start: Pos{Line: 1, Column: 1},
6464
End: Pos{Line: 4, Column: 3},
6565
Text: "12\n345",
66-
},
66+
}},
6767
wantErr: true,
6868
},
6969
{
7070
label: "out of bounds column",
7171
content: "ABC\nDEF\nGHI\nJKL",
72-
edit: Edit{
72+
edits: []Edit{{
7373
Start: Pos{Line: 1, Column: 4},
7474
End: Pos{Line: 2, Column: 3},
7575
Text: "12\n345",
76-
},
76+
}},
7777
wantErr: true,
7878
},
7979
}
@@ -82,7 +82,7 @@ func TestApplyEdit(t *testing.T) {
8282
test := test
8383
t.Run(test.label, func(t *testing.T) {
8484
lines := strings.Split(test.content, "\n")
85-
newLines, err := editContent(lines, test.edit)
85+
newLines, err := editContent(lines, test.edits)
8686
if (err != nil) != test.wantErr {
8787
t.Errorf("got err %v, want error: %t", err, test.wantErr)
8888
}

Diff for: internal/lsp/fake/editor.go

+95-22
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,13 @@ func (e *Editor) CloseBuffer(ctx context.Context, path string) error {
235235
// SaveBuffer writes the content of the buffer specified by the given path to
236236
// the filesystem.
237237
func (e *Editor) SaveBuffer(ctx context.Context, path string) error {
238+
if err := e.OrganizeImports(ctx, path); err != nil {
239+
return fmt.Errorf("organizing imports before save: %v", err)
240+
}
241+
if err := e.FormatBuffer(ctx, path); err != nil {
242+
return fmt.Errorf("formatting before save: %v", err)
243+
}
244+
238245
e.mu.Lock()
239246
buf, ok := e.buffers[path]
240247
if !ok {
@@ -282,41 +289,46 @@ func (e *Editor) SaveBuffer(ctx context.Context, path string) error {
282289

283290
// EditBuffer applies the given test edits to the buffer identified by path.
284291
func (e *Editor) EditBuffer(ctx context.Context, path string, edits []Edit) error {
285-
params, err := e.doEdits(ctx, path, edits)
286-
if err != nil {
287-
return err
288-
}
289-
if e.server != nil {
290-
if err := e.server.DidChange(ctx, params); err != nil {
291-
return fmt.Errorf("DidChange: %v", err)
292-
}
293-
}
294-
return nil
292+
e.mu.Lock()
293+
defer e.mu.Unlock()
294+
return e.editBufferLocked(ctx, path, edits)
295295
}
296296

297-
func (e *Editor) doEdits(ctx context.Context, path string, edits []Edit) (*protocol.DidChangeTextDocumentParams, error) {
297+
// BufferText returns the content of the buffer with the given name.
298+
func (e *Editor) BufferText(name string) string {
298299
e.mu.Lock()
299300
defer e.mu.Unlock()
301+
return e.buffers[name].text()
302+
}
303+
304+
func (e *Editor) editBufferLocked(ctx context.Context, path string, edits []Edit) error {
300305
buf, ok := e.buffers[path]
301306
if !ok {
302-
return nil, fmt.Errorf("unknown buffer %q", path)
307+
return fmt.Errorf("unknown buffer %q", path)
303308
}
304309
var (
305310
content = make([]string, len(buf.content))
306311
err error
307312
evts []protocol.TextDocumentContentChangeEvent
308313
)
309314
copy(content, buf.content)
310-
for _, edit := range edits {
311-
content, err = editContent(content, edit)
312-
if err != nil {
313-
return nil, err
314-
}
315-
evts = append(evts, edit.toProtocolChangeEvent())
315+
content, err = editContent(content, edits)
316+
if err != nil {
317+
return err
316318
}
319+
317320
buf.content = content
318321
buf.version++
319322
e.buffers[path] = buf
323+
// A simple heuristic: if there is only one edit, send it incrementally.
324+
// Otherwise, send the entire content.
325+
if len(edits) == 1 {
326+
evts = append(evts, edits[0].toProtocolChangeEvent())
327+
} else {
328+
evts = append(evts, protocol.TextDocumentContentChangeEvent{
329+
Text: buf.text(),
330+
})
331+
}
320332
params := &protocol.DidChangeTextDocumentParams{
321333
TextDocument: protocol.VersionedTextDocumentIdentifier{
322334
Version: float64(buf.version),
@@ -326,7 +338,12 @@ func (e *Editor) doEdits(ctx context.Context, path string, edits []Edit) (*proto
326338
},
327339
ContentChanges: evts,
328340
}
329-
return params, nil
341+
if e.server != nil {
342+
if err := e.server.DidChange(ctx, params); err != nil {
343+
return fmt.Errorf("DidChange: %v", err)
344+
}
345+
}
346+
return nil
330347
}
331348

332349
// GoToDefinition jumps to the definition of the symbol at the given position
@@ -354,6 +371,65 @@ func (e *Editor) GoToDefinition(ctx context.Context, path string, pos Pos) (stri
354371
return newPath, newPos, nil
355372
}
356373

374+
// OrganizeImports requests and performs the source.organizeImports codeAction.
375+
func (e *Editor) OrganizeImports(ctx context.Context, path string) error {
376+
if e.server == nil {
377+
return nil
378+
}
379+
params := &protocol.CodeActionParams{}
380+
params.TextDocument.URI = e.ws.URI(path)
381+
382+
actions, err := e.server.CodeAction(ctx, params)
383+
if err != nil {
384+
return fmt.Errorf("textDocument/codeAction: %v", err)
385+
}
386+
e.mu.Lock()
387+
defer e.mu.Unlock()
388+
for _, action := range actions {
389+
if action.Kind == protocol.SourceOrganizeImports {
390+
for _, change := range action.Edit.DocumentChanges {
391+
path := e.ws.URIToPath(change.TextDocument.URI)
392+
if float64(e.buffers[path].version) != change.TextDocument.Version {
393+
// Skip edits for old versions.
394+
continue
395+
}
396+
edits := convertEdits(change.Edits)
397+
if err := e.editBufferLocked(ctx, path, edits); err != nil {
398+
return fmt.Errorf("editing buffer %q: %v", path, err)
399+
}
400+
}
401+
}
402+
}
403+
return nil
404+
}
405+
406+
func convertEdits(protocolEdits []protocol.TextEdit) []Edit {
407+
var edits []Edit
408+
for _, lspEdit := range protocolEdits {
409+
edits = append(edits, fromProtocolTextEdit(lspEdit))
410+
}
411+
return edits
412+
}
413+
414+
// FormatBuffer gofmts a Go file.
415+
func (e *Editor) FormatBuffer(ctx context.Context, path string) error {
416+
if e.server == nil {
417+
return nil
418+
}
419+
// Because textDocument/formatting has no versions, we must block while
420+
// performing formatting.
421+
e.mu.Lock()
422+
defer e.mu.Unlock()
423+
params := &protocol.DocumentFormattingParams{}
424+
params.TextDocument.URI = e.ws.URI(path)
425+
resp, err := e.server.Formatting(ctx, params)
426+
if err != nil {
427+
return fmt.Errorf("textDocument/formatting: %v", err)
428+
}
429+
edits := convertEdits(resp)
430+
return e.editBufferLocked(ctx, path, edits)
431+
}
432+
357433
func (e *Editor) checkBufferPosition(path string, pos Pos) error {
358434
e.mu.Lock()
359435
defer e.mu.Unlock()
@@ -366,6 +442,3 @@ func (e *Editor) checkBufferPosition(path string, pos Pos) error {
366442
}
367443
return nil
368444
}
369-
370-
// TODO: expose more client functionality, for example Hover, CodeAction,
371-
// Rename, Completion, etc. setting the content of an entire buffer, etc.

Diff for: internal/lsp/fake/editor_test.go

+5-8
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,17 @@ func main() {
2323
`
2424

2525
func TestClientEditing(t *testing.T) {
26-
ws, err := NewWorkspace("test", []byte(exampleProgram))
26+
ws, err := NewWorkspace("TestClientEditing", []byte(exampleProgram))
2727
if err != nil {
2828
t.Fatal(err)
2929
}
3030
defer ws.Close()
3131
ctx := context.Background()
32-
client := NewEditor(ws)
33-
if err != nil {
34-
t.Fatal(err)
35-
}
36-
if err := client.OpenFile(ctx, "main.go"); err != nil {
32+
editor := NewEditor(ws)
33+
if err := editor.OpenFile(ctx, "main.go"); err != nil {
3734
t.Fatal(err)
3835
}
39-
if err := client.EditBuffer(ctx, "main.go", []Edit{
36+
if err := editor.EditBuffer(ctx, "main.go", []Edit{
4037
{
4138
Start: Pos{5, 14},
4239
End: Pos{5, 26},
@@ -45,7 +42,7 @@ func TestClientEditing(t *testing.T) {
4542
}); err != nil {
4643
t.Fatal(err)
4744
}
48-
got := client.buffers["main.go"].text()
45+
got := editor.buffers["main.go"].text()
4946
want := `package main
5047
5148
import "fmt"

0 commit comments

Comments
 (0)