Skip to content

Commit 96877f2

Browse files
stamblerrestamblerre
committed
internal/lsp, gopls: require a "gopls_" prefix on all commands
Updated the generator to check for this. Necessary to fix command name collision in VS Code Go. Not the nicest solution, but seemed like the least invasive one. The codelens configuration is a little strange now, with the "gopls_" prefixes, but the alternative is adding the prefix when processing the config and that would make the default look different from the example. Fixes golang/go#41187 Change-Id: I5cf42f4a96f6252016dcd5c40a4ac401e1b30a8f Reviewed-on: https://go-review.googlesource.com/c/tools/+/259204 Trust: Rebecca Stambler <[email protected]> Run-TryBot: Rebecca Stambler <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Heschi Kreinick <[email protected]>
1 parent 9ab7e51 commit 96877f2

File tree

13 files changed

+44
-31
lines changed

13 files changed

+44
-31
lines changed

‎gopls/doc/commands.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ Identifier: `generate`
99
generate runs `go generate` for a given directory.
1010

1111

12-
### **fill_struct**
12+
### **Fill struct**
1313
Identifier: `fill_struct`
1414

1515
fill_struct is a gopls command to fill a struct with default
@@ -34,7 +34,7 @@ Identifier: `tidy`
3434
tidy runs `go mod tidy` for a module.
3535

3636

37-
### **undeclared_name**
37+
### **Undeclared name**
3838
Identifier: `undeclared_name`
3939

4040
undeclared_name adds a variable declaration for an undeclared

‎gopls/doc/implementation.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,3 @@ Package | Description
4646
[internal/memoize]: https://github.com/golang/tools/tree/master/internal/memoize
4747
[internal/span]: https://github.com/golang/tools/tree/master/internal/span
4848
[x/tools]: https://github.com/golang/tools
49-

‎gopls/internal/regtest/codelens_test.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ const (
5050
for _, test := range tests {
5151
t.Run(test.label, func(t *testing.T) {
5252
withOptions(
53-
EditorConfig{CodeLens: test.enabled},
53+
EditorConfig{
54+
CodeLens: test.enabled,
55+
},
5456
).run(t, workspace, func(t *testing.T, env *Env) {
5557
env.OpenFile("lib.go")
5658
lens := env.CodeLens("lib.go")
@@ -251,7 +253,10 @@ func main() {
251253
}
252254
`
253255
withOptions(
254-
EditorConfig{CodeLens: map[string]bool{"gc_details": true}},
256+
EditorConfig{
257+
CodeLens: map[string]bool{
258+
"gc_details": true,
259+
}},
255260
).run(t, mod, func(t *testing.T, env *Env) {
256261
env.OpenFile("main.go")
257262
env.ExecuteCodeLensCommand("main.go", source.CommandToggleDetails)

‎gopls/internal/regtest/wrappers.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,13 +275,13 @@ func (e *Env) ExecuteCodeLensCommand(path string, cmd *source.Command) {
275275
var lens protocol.CodeLens
276276
var found bool
277277
for _, l := range lenses {
278-
if l.Command.Command == cmd.Name {
278+
if l.Command.Command == cmd.ID() {
279279
lens = l
280280
found = true
281281
}
282282
}
283283
if !found {
284-
e.T.Fatalf("found no command with the title %s", cmd.Name)
284+
e.T.Fatalf("found no command with the ID %s", cmd.ID())
285285
}
286286
if _, err := e.Editor.Server.ExecuteCommand(e.Ctx, &protocol.ExecuteCommandParams{
287287
Command: lens.Command.Command,

‎internal/lsp/cmd/workspace.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func (c *generateWorkspaceMod) Run(ctx context.Context, args ...string) error {
8282
return err
8383
}
8484
defer conn.terminate(ctx)
85-
params := &protocol.ExecuteCommandParams{Command: source.CommandGenerateGoplsMod.Name}
85+
params := &protocol.ExecuteCommandParams{Command: source.CommandGenerateGoplsMod.ID()}
8686
if _, err := conn.ExecuteCommand(ctx, params); err != nil {
8787
return fmt.Errorf("executing server command: %v", err)
8888
}

‎internal/lsp/code_action.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ func diagnosticToCommandCodeAction(ctx context.Context, snapshot source.Snapshot
400400
Kind: kind,
401401
Diagnostics: diagnostics,
402402
Command: &protocol.Command{
403-
Command: analyzer.Command.Name,
403+
Command: analyzer.Command.ID(),
404404
Title: e.Message,
405405
Arguments: jsonArgs,
406406
},
@@ -431,7 +431,7 @@ func extractionFixes(ctx context.Context, snapshot source.Snapshot, pkg source.P
431431
Title: command.Title,
432432
Kind: protocol.RefactorExtract,
433433
Command: &protocol.Command{
434-
Command: command.Name,
434+
Command: command.ID(),
435435
Arguments: jsonArgs,
436436
},
437437
})

‎internal/lsp/command.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCommandParams) (interface{}, error) {
2525
var command *source.Command
2626
for _, c := range source.Commands {
27-
if c.Name == params.Command {
27+
if c.ID() == params.Command {
2828
command = c
2929
break
3030
}
@@ -40,7 +40,7 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
4040
}
4141
}
4242
if !match {
43-
return nil, fmt.Errorf("%s is not a supported command", command.Name)
43+
return nil, fmt.Errorf("%s is not a supported command", command.ID())
4444
}
4545
title := command.Title
4646
if title == "" {
@@ -57,7 +57,7 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
5757
}
5858
if unsaved {
5959
switch params.Command {
60-
case source.CommandTest.Name, source.CommandGenerate.Name, source.CommandToggleDetails.Name:
60+
case source.CommandTest.ID(), source.CommandGenerate.ID(), source.CommandToggleDetails.ID():
6161
// TODO(PJW): for Toggle, not an error if it is being disabled
6262
err := errors.New("unsaved files in the view")
6363
s.showCommandError(ctx, title, err)
@@ -95,7 +95,7 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
9595
// message is typically dismissed immediately by LSP clients.
9696
s.showCommandError(ctx, title, err)
9797
default:
98-
work.end(command.Name + ": completed")
98+
work.end(command.ID() + ": completed")
9999
}
100100
}()
101101
return nil, nil
@@ -252,7 +252,7 @@ func (s *Server) runCommand(ctx context.Context, work *workDone, command *source
252252
return errors.Errorf("writing mod file: %w", err)
253253
}
254254
default:
255-
return fmt.Errorf("unsupported command: %s", command.Name)
255+
return fmt.Errorf("unsupported command: %s", command.ID())
256256
}
257257
return nil
258258
}

‎internal/lsp/fake/editor.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -782,7 +782,7 @@ func (e *Editor) RunGenerate(ctx context.Context, dir string) error {
782782
return err
783783
}
784784
params := &protocol.ExecuteCommandParams{
785-
Command: source.CommandGenerate.Name,
785+
Command: source.CommandGenerate.ID(),
786786
Arguments: jsonArgs,
787787
}
788788
if _, err := e.Server.ExecuteCommand(ctx, params); err != nil {

‎internal/lsp/lsp_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string)
518518
func commandToEdits(ctx context.Context, snapshot source.Snapshot, fh source.VersionedFileHandle, rng protocol.Range, cmd string) ([]protocol.TextDocumentEdit, error) {
519519
var command *source.Command
520520
for _, c := range source.Commands {
521-
if c.Name == cmd {
521+
if c.ID() == cmd {
522522
command = c
523523
break
524524
}
@@ -527,7 +527,7 @@ func commandToEdits(ctx context.Context, snapshot source.Snapshot, fh source.Ver
527527
return nil, fmt.Errorf("no known command for %s", cmd)
528528
}
529529
if !command.Applies(ctx, snapshot, fh, rng) {
530-
return nil, fmt.Errorf("cannot apply %v", command.Name)
530+
return nil, fmt.Errorf("cannot apply %v", command.ID())
531531
}
532532
return command.SuggestedFix(ctx, snapshot, fh, rng)
533533
}

‎internal/lsp/mod/code_lens.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func upgradeLens(ctx context.Context, snapshot source.Snapshot, fh source.FileHa
6060
Range: rng,
6161
Command: protocol.Command{
6262
Title: fmt.Sprintf("Upgrade dependency to %s", latest),
63-
Command: source.CommandUpgradeDependency.Name,
63+
Command: source.CommandUpgradeDependency.ID(),
6464
Arguments: upgradeDepArgs,
6565
},
6666
})
@@ -82,7 +82,7 @@ func upgradeLens(ctx context.Context, snapshot source.Snapshot, fh source.FileHa
8282
Range: moduleRng,
8383
Command: protocol.Command{
8484
Title: "Upgrade all dependencies",
85-
Command: source.CommandUpgradeDependency.Name,
85+
Command: source.CommandUpgradeDependency.ID(),
8686
Arguments: upgradeDepArgs,
8787
},
8888
})
@@ -117,7 +117,7 @@ func tidyLens(ctx context.Context, snapshot source.Snapshot, fh source.FileHandl
117117
Range: rng,
118118
Command: protocol.Command{
119119
Title: "Tidy module",
120-
Command: source.CommandTidy.Name,
120+
Command: source.CommandTidy.ID(),
121121
Arguments: goModArgs,
122122
},
123123
}}, err
@@ -150,7 +150,7 @@ func vendorLens(ctx context.Context, snapshot source.Snapshot, fh source.FileHan
150150
Range: rng,
151151
Command: protocol.Command{
152152
Title: title,
153-
Command: source.CommandVendor.Name,
153+
Command: source.CommandVendor.ID(),
154154
Arguments: goModArgs,
155155
},
156156
}}, nil

‎internal/lsp/source/api_json.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/lsp/source/code_lens.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func runTestCodeLens(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]p
6868
Range: rng,
6969
Command: protocol.Command{
7070
Title: "run test",
71-
Command: CommandTest.Name,
71+
Command: CommandTest.ID(),
7272
Arguments: jsonArgs,
7373
},
7474
})
@@ -83,7 +83,7 @@ func runTestCodeLens(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]p
8383
Range: rng,
8484
Command: protocol.Command{
8585
Title: "run benchmark",
86-
Command: CommandTest.Name,
86+
Command: CommandTest.ID(),
8787
Arguments: jsonArgs,
8888
},
8989
})
@@ -102,7 +102,7 @@ func runTestCodeLens(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]p
102102
Range: rng,
103103
Command: protocol.Command{
104104
Title: "run file benchmarks",
105-
Command: CommandTest.Name,
105+
Command: CommandTest.ID(),
106106
Arguments: args,
107107
},
108108
})
@@ -176,15 +176,15 @@ func goGenerateCodeLens(ctx context.Context, snapshot Snapshot, fh FileHandle) (
176176
Range: rng,
177177
Command: protocol.Command{
178178
Title: "run go generate",
179-
Command: CommandGenerate.Name,
179+
Command: CommandGenerate.ID(),
180180
Arguments: nonRecursiveArgs,
181181
},
182182
},
183183
{
184184
Range: rng,
185185
Command: protocol.Command{
186186
Title: "run go generate ./...",
187-
Command: CommandGenerate.Name,
187+
Command: CommandGenerate.ID(),
188188
Arguments: recursiveArgs,
189189
},
190190
},
@@ -222,7 +222,7 @@ func regenerateCgoLens(ctx context.Context, snapshot Snapshot, fh FileHandle) ([
222222
Range: rng,
223223
Command: protocol.Command{
224224
Title: "regenerate cgo definitions",
225-
Command: CommandRegenerateCgo.Name,
225+
Command: CommandRegenerateCgo.ID(),
226226
Arguments: jsonArgs,
227227
},
228228
},
@@ -246,7 +246,7 @@ func toggleDetailsCodeLens(ctx context.Context, snapshot Snapshot, fh FileHandle
246246
Range: rng,
247247
Command: protocol.Command{
248248
Title: "Toggle gc annotation details",
249-
Command: CommandToggleDetails.Name,
249+
Command: CommandToggleDetails.ID(),
250250
Arguments: jsonArgs,
251251
},
252252
}}, nil

‎internal/lsp/source/command.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ import (
2020
)
2121

2222
type Command struct {
23-
Name, Title string
23+
Title string
24+
Name string
2425

2526
// Synchronous controls whether the command executes synchronously within the
2627
// ExecuteCommand request (applying suggested fixes is always synchronous).
@@ -36,6 +37,12 @@ type Command struct {
3637
suggestedFixFn SuggestedFixFunc
3738
}
3839

40+
// ID adds the "gopls_" prefix to the command name, in order to avoid
41+
// collisions with other language servers.
42+
func (c Command) ID() string {
43+
return "gopls_" + c.Name
44+
}
45+
3946
type AppliesFunc func(fset *token.FileSet, rng span.Range, src []byte, file *ast.File, pkg *types.Package, info *types.Info) bool
4047

4148
// SuggestedFixFunc is a function used to get the suggested fixes for a given
@@ -109,13 +116,15 @@ var (
109116
// values.
110117
CommandFillStruct = &Command{
111118
Name: "fill_struct",
119+
Title: "Fill struct",
112120
suggestedFixFn: fillstruct.SuggestedFix,
113121
}
114122

115123
// CommandUndeclaredName adds a variable declaration for an undeclared
116124
// name.
117125
CommandUndeclaredName = &Command{
118126
Name: "undeclared_name",
127+
Title: "Undeclared name",
119128
suggestedFixFn: undeclaredname.SuggestedFix,
120129
}
121130

0 commit comments

Comments
 (0)