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>

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
+}

internal/lsp/general.go

@@ -52,7 +52,7 @@ func (s *Server) initialize(ctx context.Context, params *protocol.ParamInitializ
 		}
 	}
 
-	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 @@ func (s *Server) initialize(ctx context.Context, params *protocol.ParamInitializ
 		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{

internal/lsp/protocol/tsprotocol.go

@@ -2477,7 +2477,7 @@ type ServerCapabilities = struct {
 	 * 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.
 	 */