From 1d943b090355e97ccc203c569f2c4fe171961c8f Mon Sep 17 00:00:00 2001
From: Rebecca Stambler <rstambler@golang.org>
Date: Wed, 4 Dec 2019 13:36:18 -0500
Subject: [PATCH] internal/lsp: change CompletionItem.{Command,TextEdit} to
 pointers

This is a continuation of the discussion on
https://github.com/microsoft/vscode-go/issues/2920. Add corresponding
checks to internal/lsp/cmd/capabilities_test.go.

Change-Id: I51af05dee9e7ecea0e40733dd4c5ca3dfb8f4dd8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209859
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
---
 internal/lsp/cmd/capabilities_test.go | 34 +++++++++++++++++++++++++++
 internal/lsp/completion.go            |  4 ++--
 internal/lsp/protocol/tsprotocol.go   |  4 ++--
 internal/lsp/tests/completion.go      |  2 +-
 4 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/internal/lsp/cmd/capabilities_test.go b/internal/lsp/cmd/capabilities_test.go
index 33ab47a93b..da906d83f7 100644
--- a/internal/lsp/cmd/capabilities_test.go
+++ b/internal/lsp/cmd/capabilities_test.go
@@ -114,6 +114,40 @@ func TestCapabilities(t *testing.T) {
 	}); err != nil {
 		t.Fatal(err)
 	}
+
+	// Send a completion request to validate expected types.
+	list, err := c.Server.Completion(ctx, &protocol.CompletionParams{
+		TextDocumentPositionParams: protocol.TextDocumentPositionParams{
+			TextDocument: protocol.TextDocumentIdentifier{
+				URI: uri,
+			},
+			Position: protocol.Position{
+				Line:      0,
+				Character: 28,
+			},
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, item := range list.Items {
+		// We expect the "editor.action.triggerParameterHints" command for functions and methods.
+		if item.Kind == protocol.MethodCompletion || item.Kind == protocol.FunctionCompletion {
+			continue
+		}
+		// All other completion items should have nil commands.
+		// An empty command will be treated as a command with the name '' by VS Code.
+		// This causes VS Code to report errors to users about invalid commands.
+		if item.Command != nil {
+			t.Errorf("unexpected command for non-function completion item")
+		}
+		// The item's TextEdit must be a pointer, as VS Code considers TextEdits
+		// that don't contain the cursor position to be invalid.
+		var textEdit interface{} = item.TextEdit
+		if _, ok := textEdit.(*protocol.TextEdit); !ok {
+			t.Errorf("textEdit is not a *protocol.TextEdit, instead it is %T", textEdit)
+		}
+	}
 }
 
 func validateCapabilities(result *protocol.InitializeResult) error {
diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go
index b4634dedf6..a10981f8de 100644
--- a/internal/lsp/completion.go
+++ b/internal/lsp/completion.go
@@ -114,7 +114,7 @@ func toProtocolCompletionItems(candidates []source.CompletionItem, rng protocol.
 			Label:  candidate.Label,
 			Detail: candidate.Detail,
 			Kind:   candidate.Kind,
-			TextEdit: protocol.TextEdit{
+			TextEdit: &protocol.TextEdit{
 				NewText: insertText,
 				Range:   rng,
 			},
@@ -133,7 +133,7 @@ func toProtocolCompletionItems(candidates []source.CompletionItem, rng protocol.
 		// since we show return types as well.
 		switch item.Kind {
 		case protocol.FunctionCompletion, protocol.MethodCompletion:
-			item.Command = protocol.Command{
+			item.Command = &protocol.Command{
 				Command: "editor.action.triggerParameterHints",
 			}
 		}
diff --git a/internal/lsp/protocol/tsprotocol.go b/internal/lsp/protocol/tsprotocol.go
index 1e1a306573..3243a84cd6 100644
--- a/internal/lsp/protocol/tsprotocol.go
+++ b/internal/lsp/protocol/tsprotocol.go
@@ -552,7 +552,7 @@ type CompletionItem struct {
 	 * *Note:* The text edit's range must be a [single line] and it must contain the position
 	 * at which completion has been requested.
 	 */
-	TextEdit TextEdit `json:"textEdit,omitempty"`
+	TextEdit *TextEdit `json:"textEdit,omitempty"`
 	/**
 	 * An optional array of additional [text edits](#TextEdit) that are applied when
 	 * selecting this completion. Edits must not overlap (including the same insert position)
@@ -574,7 +574,7 @@ type CompletionItem struct {
 	 * additional modifications to the current document should be described with the
 	 * [additionalTextEdits](#CompletionItem.additionalTextEdits)-property.
 	 */
-	Command Command `json:"command,omitempty"`
+	Command *Command `json:"command,omitempty"`
 	/**
 	 * An data entry field that is preserved on a completion item between
 	 * a [CompletionRequest](#CompletionRequest) and a [CompletionResolveRequest]
diff --git a/internal/lsp/tests/completion.go b/internal/lsp/tests/completion.go
index 316e20cd51..ba5ec70f24 100644
--- a/internal/lsp/tests/completion.go
+++ b/internal/lsp/tests/completion.go
@@ -26,7 +26,7 @@ func ToProtocolCompletionItem(item source.CompletionItem) protocol.CompletionIte
 		Detail:        item.Detail,
 		Documentation: item.Documentation,
 		InsertText:    item.InsertText,
-		TextEdit: protocol.TextEdit{
+		TextEdit: &protocol.TextEdit{
 			NewText: item.Snippet(),
 		},
 		// Negate score so best score has lowest sort text like real API.