internal/lsp: switch to the new command API
Fully switch to the new generated command API, and remove the old
dynamic command configuration.
This involved several steps:
+ Switch the command dispatch in internal/lsp/command.go to go through
the command package. This means that all commands must now use the new
signature.
+ Update commandHandler to use the new command signatures.
+ Fix some errors discovered in the command interface now that we're
actually using it.
+ Regenerate bindings.
+ Update all code lens and suggested fixes to new the new command
constructors.
+ Generate values in the command package to hold command names and the
full set of commands, so that they may be referenced by name.
+ Update any references to command names to use the command package.
+ Delete command metadata from the source package. Rename command.go to
fix.go.
+ Update lsp tests to execute commands directly rather than use an
internal API. This involved a bit of hackery to collect the edits.
+ Update document generation to use command metadata. Documenting the
arguments is left to a later CL.
+ Various small fixes related to the above.
This change is intended to be invisible to users. We have changed the
command signatures, but have not (previously) committed to backwards
compatibility for commands. Notably, the gopls.test and gopls.gc_details
signatures are preserved, as these are the two cases where we are aware
of LSP clients calling them directly, not from a code lens or
diagnostic.
For golang/go#40438
Change-Id: Ie1b92c95d6ce7e2fc25fc029d1f85b942f40e851
Reviewed-on: https://go-review.googlesource.com/c/tools/+/290111
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go
index b796433..1ab91fb 100644
--- a/internal/lsp/code_action.go
+++ b/internal/lsp/code_action.go
@@ -13,6 +13,7 @@
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/imports"
+ "golang.org/x/tools/internal/lsp/command"
"golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/mod"
"golang.org/x/tools/internal/lsp/protocol"
@@ -264,7 +265,7 @@
}
// If the suggested fix for the diagnostic is expected to be separate,
// see if there are any supported commands available.
- if analyzer.Command != nil {
+ if analyzer.Fix != "" {
action, err := diagnosticToCommandCodeAction(ctx, snapshot, srcErr, &diag, protocol.QuickFix)
if err != nil {
return nil, nil, err
@@ -362,7 +363,7 @@
if !a.IsEnabled(snapshot.View()) {
continue
}
- if a.Command == nil {
+ if a.Fix == "" {
event.Error(ctx, "convenienceFixes", fmt.Errorf("no suggested fixes for convenience analyzer %s", a.Analyzer.Name))
continue
}
@@ -399,10 +400,14 @@
if analyzer == nil {
return nil, fmt.Errorf("no convenience analyzer for source %s", sd.Source)
}
- if analyzer.Command == nil {
- return nil, fmt.Errorf("no command for convenience analyzer %s", analyzer.Analyzer.Name)
+ if analyzer.Fix == "" {
+ return nil, fmt.Errorf("no fix for convenience analyzer %s", analyzer.Analyzer.Name)
}
- jsonArgs, err := source.MarshalArgs(sd.URI, sd.Range)
+ cmd, err := command.NewApplyFixCommand(sd.Message, command.ApplyFixArgs{
+ URI: protocol.URIFromSpanURI(sd.URI),
+ Range: sd.Range,
+ Fix: analyzer.Fix,
+ })
if err != nil {
return nil, err
}
@@ -414,11 +419,7 @@
Title: sd.Message,
Kind: kind,
Diagnostics: diagnostics,
- Command: &protocol.Command{
- Command: analyzer.Command.ID(),
- Title: sd.Message,
- Arguments: jsonArgs,
- },
+ Command: &cmd,
}, nil
}
@@ -430,10 +431,6 @@
if err != nil {
return nil, err
}
- jsonArgs, err := source.MarshalArgs(uri, rng)
- if err != nil {
- return nil, err
- }
_, pgf, err := source.GetParsedFile(ctx, snapshot, fh, source.NarrowestPackage)
if err != nil {
return nil, errors.Errorf("getting file for Identifier: %w", err)
@@ -442,22 +439,36 @@
if err != nil {
return nil, err
}
- var commands []*source.Command
+ puri := protocol.URIFromSpanURI(uri)
+ var commands []protocol.Command
if _, ok, _ := source.CanExtractFunction(snapshot.FileSet(), srng, pgf.Src, pgf.File); ok {
- commands = append(commands, source.CommandExtractFunction)
+ cmd, err := command.NewApplyFixCommand("Extract to function", command.ApplyFixArgs{
+ URI: puri,
+ Fix: source.ExtractFunction,
+ Range: rng,
+ })
+ if err != nil {
+ return nil, err
+ }
+ commands = append(commands, cmd)
}
if _, _, ok, _ := source.CanExtractVariable(srng, pgf.File); ok {
- commands = append(commands, source.CommandExtractVariable)
+ cmd, err := command.NewApplyFixCommand("Extract variable", command.ApplyFixArgs{
+ URI: puri,
+ Fix: source.ExtractVariable,
+ Range: rng,
+ })
+ if err != nil {
+ return nil, err
+ }
+ commands = append(commands, cmd)
}
var actions []protocol.CodeAction
- for _, command := range commands {
+ for _, cmd := range commands {
actions = append(actions, protocol.CodeAction{
- Title: command.Title,
- Kind: protocol.RefactorExtract,
- Command: &protocol.Command{
- Command: command.ID(),
- Arguments: jsonArgs,
- },
+ Title: cmd.Title,
+ Kind: protocol.RefactorExtract,
+ Command: &cmd,
})
}
return actions, nil
@@ -574,17 +585,13 @@
return nil, nil
}
- jsonArgs, err := source.MarshalArgs(uri, tests, benchmarks)
+ cmd, err := command.NewTestCommand("Run tests and benchmarks", protocol.URIFromSpanURI(uri), tests, benchmarks)
if err != nil {
return nil, err
}
return []protocol.CodeAction{{
- Title: source.CommandTest.Name,
- Kind: protocol.GoTest,
- Command: &protocol.Command{
- Title: source.CommandTest.Title,
- Command: source.CommandTest.ID(),
- Arguments: jsonArgs,
- },
+ Title: cmd.Title,
+ Kind: protocol.GoTest,
+ Command: &cmd,
}}, nil
}
