Skip to content

Commit 5ea3631

Browse files
stamblerrestamblerre
committed
internal/lsp: change the way that we pass arguments to command
Our approach to commands and their arguments has been ad-hoc until this point. This CL creates a standard way of defining and passing the arguments to different commands. The arguments to a command are now json.RawMessages, so that we don't have to double encode. This also allows us to check the expected number of arguments without defining a struct for every command. Change-Id: Ic765c9b059e8ec3e1985046d13bf321be21f16ab Reviewed-on: https://go-review.googlesource.com/c/tools/+/242697 Run-TryBot: Rebecca Stambler <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Robert Findley <[email protected]>
1 parent b8e13e1 commit 5ea3631

File tree

10 files changed

+177
-129
lines changed

10 files changed

+177
-129
lines changed

‎internal/lsp/code_action.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package lsp
66

77
import (
88
"context"
9-
"encoding/json"
109
"fmt"
1110
"sort"
1211
"strings"
@@ -330,22 +329,17 @@ func convenienceFixes(ctx context.Context, snapshot source.Snapshot, ph source.P
330329
// The fix depends on the category of the analyzer.
331330
switch d.Category {
332331
case fillstruct.Analyzer.Name:
333-
arg, err := json.Marshal(CommandRangeArgument{
334-
URI: protocol.URIFromSpanURI(d.URI),
335-
Range: d.Range,
336-
})
332+
jsonArgs, err := source.EncodeArgs(d.URI, d.Range)
337333
if err != nil {
338334
return nil, err
339335
}
340336
action := protocol.CodeAction{
341337
Title: d.Message,
342338
Kind: protocol.RefactorRewrite,
343339
Command: &protocol.Command{
344-
Command: source.CommandFillStruct,
345-
Title: d.Message,
346-
Arguments: []interface{}{
347-
string(arg),
348-
},
340+
Command: source.CommandFillStruct,
341+
Title: d.Message,
342+
Arguments: jsonArgs,
349343
},
350344
}
351345
codeActions = append(codeActions, action)

‎internal/lsp/code_lens.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Copyright 2020 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package lsp
6+
7+
import (
8+
"context"
9+
10+
"golang.org/x/tools/internal/lsp/mod"
11+
"golang.org/x/tools/internal/lsp/protocol"
12+
"golang.org/x/tools/internal/lsp/source"
13+
)
14+
15+
func (s *Server) codeLens(ctx context.Context, params *protocol.CodeLensParams) ([]protocol.CodeLens, error) {
16+
snapshot, fh, ok, err := s.beginFileRequest(ctx, params.TextDocument.URI, source.UnknownKind)
17+
if !ok {
18+
return nil, err
19+
}
20+
switch fh.Kind() {
21+
case source.Mod:
22+
return mod.CodeLens(ctx, snapshot, fh.URI())
23+
case source.Go:
24+
return source.CodeLens(ctx, snapshot, fh)
25+
}
26+
// Unsupported file kind for a code action.
27+
return nil, nil
28+
}

‎internal/lsp/command.go

Lines changed: 51 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,8 @@ package lsp
66

77
import (
88
"context"
9-
"encoding/json"
109
"fmt"
1110
"io"
12-
"strings"
1311

1412
"golang.org/x/tools/internal/event"
1513
"golang.org/x/tools/internal/lsp/debug/tag"
@@ -20,11 +18,6 @@ import (
2018
errors "golang.org/x/xerrors"
2119
)
2220

23-
type CommandRangeArgument struct {
24-
URI protocol.DocumentURI `json:"uri,omitempty"`
25-
Range protocol.Range `json:"range,omitempty"`
26-
}
27-
2821
func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCommandParams) (interface{}, error) {
2922
var found bool
3023
for _, command := range s.session.Options().SupportedCommands {
@@ -36,82 +29,86 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
3629
if !found {
3730
return nil, fmt.Errorf("unsupported command detected: %s", params.Command)
3831
}
39-
switch params.Command {
40-
case source.CommandTest:
41-
unsaved := false
42-
for _, overlay := range s.session.Overlays() {
43-
if !overlay.Saved() {
44-
unsaved = true
45-
break
46-
}
47-
}
48-
if unsaved {
32+
// Some commands require that all files are saved to disk. If we detect
33+
// unsaved files, warn the user instead of running the commands.
34+
unsaved := false
35+
for _, overlay := range s.session.Overlays() {
36+
if !overlay.Saved() {
37+
unsaved = true
38+
break
39+
}
40+
}
41+
if unsaved {
42+
switch params.Command {
43+
case source.CommandTest, source.CommandGenerate:
4944
return nil, s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
5045
Type: protocol.Error,
51-
Message: "could not run tests, there are unsaved files in the view",
46+
Message: fmt.Sprintf("cannot run command %s: unsaved files in the view", params.Command),
5247
})
5348
}
54-
funcName, uri, err := getRunTestArguments(params.Arguments)
55-
if err != nil {
49+
}
50+
switch params.Command {
51+
case source.CommandTest:
52+
var uri protocol.DocumentURI
53+
var funcName string
54+
if err := source.DecodeArgs(params.Arguments, &uri, &funcName); err != nil {
5655
return nil, err
5756
}
58-
view, err := s.session.ViewOf(uri)
59-
if err != nil {
57+
snapshot, _, ok, err := s.beginFileRequest(ctx, uri, source.UnknownKind)
58+
if !ok {
6059
return nil, err
6160
}
62-
go s.runTest(ctx, view.Snapshot(), funcName)
61+
go s.runGoTest(ctx, snapshot, funcName)
6362
case source.CommandGenerate:
64-
dir, recursive, err := getGenerateRequest(params.Arguments)
65-
if err != nil {
63+
var uri protocol.DocumentURI
64+
var recursive bool
65+
if err := source.DecodeArgs(params.Arguments, &uri, &recursive); err != nil {
6666
return nil, err
6767
}
68-
go s.runGoGenerate(xcontext.Detach(ctx), dir, recursive)
68+
go s.runGoGenerate(xcontext.Detach(ctx), uri.SpanURI(), recursive)
6969
case source.CommandRegenerateCgo:
70+
var uri protocol.DocumentURI
71+
if err := source.DecodeArgs(params.Arguments, &uri); err != nil {
72+
return nil, err
73+
}
7074
mod := source.FileModification{
71-
URI: protocol.DocumentURI(params.Arguments[0].(string)).SpanURI(),
75+
URI: uri.SpanURI(),
7276
Action: source.InvalidateMetadata,
7377
}
7478
_, err := s.didModifyFiles(ctx, []source.FileModification{mod}, FromRegenerateCgo)
7579
return nil, err
7680
case source.CommandTidy, source.CommandVendor:
77-
if len(params.Arguments) == 0 || len(params.Arguments) > 1 {
78-
return nil, errors.Errorf("expected 1 argument, got %v", params.Arguments)
81+
var uri protocol.DocumentURI
82+
if err := source.DecodeArgs(params.Arguments, &uri); err != nil {
83+
return nil, err
7984
}
80-
uri := protocol.DocumentURI(params.Arguments[0].(string))
81-
8285
// The flow for `go mod tidy` and `go mod vendor` is almost identical,
8386
// so we combine them into one case for convenience.
84-
arg := "tidy"
87+
a := "tidy"
8588
if params.Command == source.CommandVendor {
86-
arg = "vendor"
89+
a = "vendor"
8790
}
88-
err := s.directGoModCommand(ctx, uri, "mod", []string{arg}...)
91+
err := s.directGoModCommand(ctx, uri, "mod", []string{a}...)
8992
return nil, err
9093
case source.CommandUpgradeDependency:
91-
if len(params.Arguments) < 2 {
92-
return nil, errors.Errorf("expected 2 arguments, got %v", params.Arguments)
94+
var uri protocol.DocumentURI
95+
var deps []string
96+
if err := source.DecodeArgs(params.Arguments, &uri, &deps); err != nil {
97+
return nil, err
9398
}
94-
uri := protocol.DocumentURI(params.Arguments[0].(string))
95-
deps := params.Arguments[1].(string)
96-
err := s.directGoModCommand(ctx, uri, "get", strings.Split(deps, " ")...)
99+
err := s.directGoModCommand(ctx, uri, "get", deps...)
97100
return nil, err
98101
case source.CommandFillStruct:
99-
if len(params.Arguments) != 1 {
100-
return nil, fmt.Errorf("expected 1 arguments, got %v: %v", len(params.Arguments), params.Arguments)
101-
}
102-
var arg CommandRangeArgument
103-
str, ok := params.Arguments[0].(string)
104-
if !ok {
105-
return nil, fmt.Errorf("expected string, got %v (%T)", params.Arguments[0], params.Arguments[0])
106-
}
107-
if err := json.Unmarshal([]byte(str), &arg); err != nil {
102+
var uri protocol.DocumentURI
103+
var rng protocol.Range
104+
if err := source.DecodeArgs(params.Arguments, &uri, &rng); err != nil {
108105
return nil, err
109106
}
110-
snapshot, fh, ok, err := s.beginFileRequest(ctx, arg.URI, source.Go)
107+
snapshot, fh, ok, err := s.beginFileRequest(ctx, uri, source.Go)
111108
if !ok {
112109
return nil, err
113110
}
114-
edits, err := source.FillStruct(ctx, snapshot, fh, arg.Range)
111+
edits, err := source.FillStruct(ctx, snapshot, fh, rng)
115112
if err != nil {
116113
return nil, err
117114
}
@@ -129,6 +126,8 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
129126
Message: fmt.Sprintf("fillstruct failed: %v", r.FailureReason),
130127
})
131128
}
129+
default:
130+
return nil, fmt.Errorf("unknown command: %s", params.Command)
132131
}
133132
return nil, nil
134133
}
@@ -141,7 +140,7 @@ func (s *Server) directGoModCommand(ctx context.Context, uri protocol.DocumentUR
141140
return view.Snapshot().RunGoCommandDirect(ctx, verb, args)
142141
}
143142

144-
func (s *Server) runTest(ctx context.Context, snapshot source.Snapshot, funcName string) error {
143+
func (s *Server) runGoTest(ctx context.Context, snapshot source.Snapshot, funcName string) error {
145144
ctx, cancel := context.WithCancel(ctx)
146145
defer cancel()
147146

@@ -171,7 +170,7 @@ func (s *Server) runTest(ctx context.Context, snapshot source.Snapshot, funcName
171170
// generate commands. It is exported for testing purposes.
172171
const GenerateWorkDoneTitle = "generate"
173172

174-
func (s *Server) runGoGenerate(ctx context.Context, dir string, recursive bool) error {
173+
func (s *Server) runGoGenerate(ctx context.Context, uri span.URI, recursive bool) error {
175174
ctx, cancel := context.WithCancel(ctx)
176175
defer cancel()
177176

@@ -184,7 +183,6 @@ func (s *Server) runGoGenerate(ctx context.Context, dir string, recursive bool)
184183
}
185184

186185
stderr := io.MultiWriter(er, wc)
187-
uri := span.URIFromPath(dir)
188186
view, err := s.session.ViewOf(uri)
189187
if err != nil {
190188
return err
@@ -202,33 +200,3 @@ func (s *Server) runGoGenerate(ctx context.Context, dir string, recursive bool)
202200
}
203201
return nil
204202
}
205-
206-
func getRunTestArguments(args []interface{}) (string, span.URI, error) {
207-
if len(args) != 2 {
208-
return "", "", errors.Errorf("expected one test func name and one file path, got %v", args)
209-
}
210-
funcName, ok := args[0].(string)
211-
if !ok {
212-
return "", "", errors.Errorf("expected func name to be a string, got %T", args[0])
213-
}
214-
filename, ok := args[1].(string)
215-
if !ok {
216-
return "", "", errors.Errorf("expected file to be a string, got %T", args[1])
217-
}
218-
return funcName, span.URIFromPath(filename), nil
219-
}
220-
221-
func getGenerateRequest(args []interface{}) (string, bool, error) {
222-
if len(args) != 2 {
223-
return "", false, errors.Errorf("expected exactly 2 arguments but got %d", len(args))
224-
}
225-
dir, ok := args[0].(string)
226-
if !ok {
227-
return "", false, errors.Errorf("expected dir to be a string value but got %T", args[0])
228-
}
229-
recursive, ok := args[1].(bool)
230-
if !ok {
231-
return "", false, errors.Errorf("expected recursive to be a boolean but got %T", args[1])
232-
}
233-
return dir, recursive, nil
234-
}

‎internal/lsp/fake/editor.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import (
1616

1717
"golang.org/x/tools/internal/jsonrpc2"
1818
"golang.org/x/tools/internal/lsp/protocol"
19+
"golang.org/x/tools/internal/lsp/source"
20+
"golang.org/x/tools/internal/span"
1921
)
2022

2123
// Editor is a fake editor client. It keeps track of client state and can be
@@ -714,9 +716,13 @@ func (e *Editor) RunGenerate(ctx context.Context, dir string) error {
714716
return nil
715717
}
716718
absDir := e.sandbox.Workdir.filePath(dir)
719+
jsonArgs, err := source.EncodeArgs(span.URIFromPath(absDir), false)
720+
if err != nil {
721+
return err
722+
}
717723
params := &protocol.ExecuteCommandParams{
718-
Command: "generate",
719-
Arguments: []interface{}{absDir, false},
724+
Command: source.CommandGenerate,
725+
Arguments: jsonArgs,
720726
}
721727
if _, err := e.Server.ExecuteCommand(ctx, params); err != nil {
722728
return fmt.Errorf("running generate: %v", err)

‎internal/lsp/mod/code_lens.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package mod
33
import (
44
"context"
55
"fmt"
6-
"strings"
76

87
"golang.org/x/mod/modfile"
98
"golang.org/x/tools/internal/event"
@@ -60,12 +59,16 @@ func CodeLens(ctx context.Context, snapshot source.Snapshot, uri span.URI) ([]pr
6059
if err != nil {
6160
return nil, err
6261
}
62+
jsonArgs, err := source.EncodeArgs(uri, dep)
63+
if err != nil {
64+
return nil, err
65+
}
6366
codelens = append(codelens, protocol.CodeLens{
6467
Range: rng,
6568
Command: protocol.Command{
6669
Title: fmt.Sprintf("Upgrade dependency to %s", latest),
6770
Command: source.CommandUpgradeDependency,
68-
Arguments: []interface{}{uri, dep},
71+
Arguments: jsonArgs,
6972
},
7073
})
7174
allUpgrades = append(allUpgrades, dep)
@@ -77,12 +80,16 @@ func CodeLens(ctx context.Context, snapshot source.Snapshot, uri span.URI) ([]pr
7780
if err != nil {
7881
return nil, err
7982
}
83+
jsonArgs, err := source.EncodeArgs(uri, append([]string{"-u"}, allUpgrades...))
84+
if err != nil {
85+
return nil, err
86+
}
8087
codelens = append(codelens, protocol.CodeLens{
8188
Range: rng,
8289
Command: protocol.Command{
8390
Title: "Upgrade all dependencies",
8491
Command: source.CommandUpgradeDependency,
85-
Arguments: []interface{}{uri, strings.Join(append([]string{"-u"}, allUpgrades...), " ")},
92+
Arguments: jsonArgs,
8693
},
8794
})
8895
}

‎internal/lsp/progress.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -149,14 +149,11 @@ func (s *Server) newProgressWriter(ctx context.Context, title, beginMsg, msg str
149149
return mw
150150
}
151151

152-
// messageWriter implements progressWriter
153-
// and only tells the user that "go generate"
154-
// has started through window/showMessage but does not
155-
// report anything afterwards. This is because each
156-
// log shows up as a separate window and therefore
157-
// would be obnoxious to show every incoming line.
158-
// Request cancellation happens synchronously through
159-
// the ShowMessageRequest response.
152+
// messageWriter implements progressWriter and only tells the user that
153+
// a command has started through window/showMessage, but does not report
154+
// anything afterwards. This is because each log shows up as a separate window
155+
// and therefore would be obnoxious to show every incoming line. Request
156+
// cancellation happens synchronously through the ShowMessageRequest response.
160157
type messageWriter struct {
161158
ctx context.Context
162159
cancel func()

0 commit comments

Comments
 (0)