internal/lsp: add some minimal validation for client capabilities
We've had a couple of breakages with changes to the protocol that are
bugs on our end. It's hard to review changes to the protocol, and it's
not reasonable to expect that we would remember the correct types for
everything, so we should have a test that validates some basic expectations
about the expected responses. We can add more here as issues come up.
Also, change RenameProvider back to an interface.
Updates golang/go#32703
Change-Id: Ic5f0b0ece40b05e4425cd98ab7bf18db3ad74601
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208272
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/internal/lsp/cmd/capabilities_test.go b/internal/lsp/cmd/capabilities_test.go
new file mode 100644
index 0000000..68683d2
--- /dev/null
+++ b/internal/lsp/cmd/capabilities_test.go
@@ -0,0 +1,101 @@
+package cmd
+
+import (
+ "context"
+ "io/ioutil"
+ "os"
+ "path/filepath"
+ "testing"
+
+ "golang.org/x/tools/internal/lsp"
+ "golang.org/x/tools/internal/lsp/cache"
+ "golang.org/x/tools/internal/lsp/protocol"
+ "golang.org/x/tools/internal/span"
+ errors "golang.org/x/xerrors"
+)
+
+// TestCapabilities does some minimal validation of the server's adherence to the LSP.
+// The checks in the test are added as changes are made and errors noticed.
+func TestCapabilities(t *testing.T) {
+ tmpDir, err := ioutil.TempDir("", "fake")
+ if err != nil {
+ t.Fatal(err)
+ }
+ tmpFile := filepath.Join(tmpDir, "fake.go")
+ if err := ioutil.WriteFile(tmpFile, []byte(""), 0775); err != nil {
+ t.Fatal(err)
+ }
+ if err := ioutil.WriteFile(filepath.Join(tmpDir, "go.mod"), []byte(`module fake`), 0775); err != nil {
+ t.Fatal(err)
+ }
+ defer os.RemoveAll(tmpDir)
+
+ app := New("gopls-test", tmpDir, os.Environ(), nil)
+ c := newConnection(app)
+ ctx := context.Background()
+ defer c.terminate(ctx)
+
+ params := &protocol.ParamInitialize{}
+ params.RootURI = string(span.FileURI(c.Client.app.wd))
+ params.Capabilities.Workspace.Configuration = true
+
+ // Send an initialize request to the server.
+ ctx, c.Server = lsp.NewClientServer(ctx, cache.New(app.options), c.Client)
+ result, err := c.Server.Initialize(ctx, params)
+ if err != nil {
+ t.Fatal(err)
+ }
+ // Validate initialization result.
+ if err := validateCapabilities(result); err != nil {
+ t.Error(err)
+ }
+ // Complete initialization of server.
+ if err := c.Server.Initialized(ctx, &protocol.InitializedParams{}); err != nil {
+ t.Fatal(err)
+ }
+
+ // Open the file on the server side.
+ uri := protocol.NewURI(span.FileURI(tmpFile))
+ if err := c.Server.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{
+ TextDocument: protocol.TextDocumentItem{
+ URI: uri,
+ LanguageID: "go",
+ Version: 1,
+ Text: `package main; func main() {};`,
+ },
+ }); err != nil {
+ t.Fatal(err)
+ }
+
+ // If we are sending a full text change, the change.Range must be nil.
+ // It is not enough for the Change to be empty, as that is ambiguous.
+ if err := c.Server.DidChange(ctx, &protocol.DidChangeTextDocumentParams{
+ TextDocument: protocol.VersionedTextDocumentIdentifier{
+ TextDocumentIdentifier: protocol.TextDocumentIdentifier{
+ URI: uri,
+ },
+ Version: 2,
+ },
+ ContentChanges: []protocol.TextDocumentContentChangeEvent{
+ {
+ Range: nil,
+ Text: `package main; func main() {}; func main2() {};`,
+ },
+ },
+ }); err != nil {
+ t.Fatal(err)
+ }
+}
+
+func validateCapabilities(result *protocol.InitializeResult) error {
+ // If the client sends "false" for RenameProvider.PrepareSupport,
+ // the server must respond with a boolean.
+ if v, ok := result.Capabilities.RenameProvider.(bool); !ok {
+ return errors.Errorf("RenameProvider must be a boolean if PrepareSupport is false (got %T)", v)
+ }
+ // The same goes for CodeActionKind.ValueSet.
+ if v, ok := result.Capabilities.CodeActionProvider.(bool); !ok {
+ return errors.Errorf("CodeActionSupport must be a boolean if CodeActionKind.ValueSet has length 0 (got %T)", v)
+ }
+ return nil
+}
diff --git a/internal/lsp/general.go b/internal/lsp/general.go
index 3b371bc..c8c11ae 100644
--- a/internal/lsp/general.go
+++ b/internal/lsp/general.go
@@ -52,7 +52,7 @@
}
}
- var codeActionProvider interface{}
+ var codeActionProvider interface{} = true
if ca := params.Capabilities.TextDocument.CodeAction; len(ca.CodeActionLiteralSupport.CodeActionKind.ValueSet) > 0 {
// If the client has specified CodeActionLiteralSupport,
// send the code actions we support.
@@ -61,14 +61,12 @@
codeActionProvider = &protocol.CodeActionOptions{
CodeActionKinds: s.getSupportedCodeActions(),
}
- } else {
- codeActionProvider = true
}
- // This used to be interface{}, when r could be nil
- var renameOpts protocol.RenameOptions
- r := params.Capabilities.TextDocument.Rename
- renameOpts = protocol.RenameOptions{
- PrepareProvider: r.PrepareSupport,
+ var renameOpts interface{} = true
+ if r := params.Capabilities.TextDocument.Rename; r.PrepareSupport {
+ renameOpts = protocol.RenameOptions{
+ PrepareProvider: r.PrepareSupport,
+ }
}
return &protocol.InitializeResult{
Capabilities: protocol.ServerCapabilities{
diff --git a/internal/lsp/protocol/tsprotocol.go b/internal/lsp/protocol/tsprotocol.go
index 5657378..209b3ea 100644
--- a/internal/lsp/protocol/tsprotocol.go
+++ b/internal/lsp/protocol/tsprotocol.go
@@ -2477,7 +2477,7 @@
* specified if the client states that it supports
* `prepareSupport` in its initial `initialize` request.
*/
- RenameProvider RenameOptions/*boolean | RenameOptions*/ `json:"renameProvider,omitempty"`
+ RenameProvider interface{}/*boolean | RenameOptions*/ `json:"renameProvider,omitempty"`
/**
* The server provides folding provider support.
*/
