internal/lsp: handle additional snippet cases

This change handles the case when a function that has already been
written out is being completed.

Change-Id: I0c4e9ec9bb5a8428526f00a4e62e020bcc30f0bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/176923
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>

internal/lsp/lsp_test.go

@@ -151,17 +151,7 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests
 			t.Errorf("%s: %s", src, diff)
 		}
 	}
-	// Make sure we don't crash completing the first position in file set.
-	firstPos, err := span.NewRange(r.data.Exported.ExpectFileSet, 1, 2).Span()
-	if err != nil {
-		t.Fatal(err)
-	}
-	_ = r.runCompletion(t, firstPos)
 
-	r.checkCompletionSnippets(t, snippets, items)
-}
-
-func (r *runner) checkCompletionSnippets(t *testing.T, data tests.CompletionSnippets, items tests.CompletionItems) {
 	origPlaceHolders := r.server.usePlaceholders
 	origTextFormat := r.server.insertTextFormat
 	defer func() {
@@ -173,31 +163,28 @@ func (r *runner) checkCompletionSnippets(t *testing.T, data tests.CompletionSnip
 	for _, usePlaceholders := range []bool{true, false} {
 		r.server.usePlaceholders = usePlaceholders
 
-		for src, want := range data {
+		for src, want := range snippets {
 			list := r.runCompletion(t, src)
 
-			wantCompletion := items[want.CompletionItem]
+			wantItem := items[want.CompletionItem]
-			var gotItem *protocol.CompletionItem
+			var got *protocol.CompletionItem
 			for _, item := range list.Items {
-				if item.Label == wantCompletion.Label {
+				if item.Label == wantItem.Label {
-					gotItem = &item
+					got = &item
 					break
 				}
 			}
-
+			if got == nil {
-			if gotItem == nil {
+				t.Fatalf("%s: couldn't find completion matching %q", src.URI(), wantItem.Label)
-				t.Fatalf("%s: couldn't find completion matching %q", src.URI(), wantCompletion.Label)
 			}
-
 			var expected string
 			if usePlaceholders {
 				expected = want.PlaceholderSnippet
 			} else {
 				expected = want.PlainSnippet
 			}
-
+			if expected != got.TextEdit.NewText {
-			if expected != gotItem.TextEdit.NewText {
+				t.Errorf("%s: expected snippet %q, got %q", src, expected, got.TextEdit.NewText)
-				t.Errorf("%s: expected snippet %q, got %q", src, expected, gotItem.TextEdit.NewText)
 			}
 		}
 	}

internal/lsp/source/completion_format.go

@@ -66,6 +66,9 @@ func (c *completer) item(obj types.Object, score float64) CompletionItem {
 		results, writeParens := formatResults(sig.Results(), c.qf)
 		label, detail = formatFunction(obj.Name(), params, results, writeParens)
 		plainSnippet, placeholderSnippet = c.functionCallSnippets(obj.Name(), params)
+		if plainSnippet == nil && placeholderSnippet == nil {
+			insert = ""
+		}
 		kind = FunctionCompletionItem
 		if sig.Recv() != nil {
 			kind = MethodCompletionItem

internal/lsp/source/completion_snippet.go

@@ -72,7 +72,6 @@ func (c *completer) functionCallSnippets(name string, params []string) (*snippet
 			}
 		}
 	}
-
 	plain, placeholder := &snippet.Builder{}, &snippet.Builder{}
 	label := fmt.Sprintf("%s(", name)
 

internal/lsp/source/source_test.go

@@ -156,11 +156,8 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests
 			t.Errorf("%s: %s", src, diff)
 		}
 	}
-}
-
-func (r *runner) checkCompletionSnippets(ctx context.Context, t *testing.T, data tests.CompletionSnippets, items tests.CompletionItems) {
 	for _, usePlaceholders := range []bool{true, false} {
-		for src, want := range data {
+		for src, want := range snippets {
 			f, err := r.view.GetFile(ctx, src.URI())
 			if err != nil {
 				t.Fatalf("failed for %v: %v", src, err)
@@ -171,25 +168,23 @@ func (r *runner) checkCompletionSnippets(ctx context.Context, t *testing.T, data
 			if err != nil {
 				t.Fatalf("failed for %v: %v", src, err)
 			}
-
+			wantItem := items[want.CompletionItem]
-			wantCompletion := items[want.CompletionItem]
 			var got *source.CompletionItem
 			for _, item := range list {
-				if item.Label == wantCompletion.Label {
+				if item.Label == wantItem.Label {
 					got = &item
 					break
 				}
 			}
 			if got == nil {
-				t.Fatalf("%s: couldn't find completion matching %q", src.URI(), wantCompletion.Label)
+				t.Fatalf("%s: couldn't find completion matching %q", src.URI(), wantItem.Label)
 			}
-
 			expected := want.PlainSnippet
 			if usePlaceholders {
 				expected = want.PlaceholderSnippet
 			}
-			if insertText := got.Snippet(usePlaceholders); expected != insertText {
+			if actual := got.Snippet(usePlaceholders); expected != actual {
-				t.Errorf("%s: expected snippet %q, got %q", src, expected, insertText)
+				t.Errorf("%s: expected placeholder snippet %q, got %q", src, expected, actual)
 			}
 		}
 	}

internal/lsp/testdata/snippets/snippets.go

@@ -7,14 +7,15 @@ type Foo struct {
 	Bar int //@item(snipFieldBar, "Bar", "int", "field")
 }
 
-func (Foo) Baz() func() {} //@item(snipMethodBaz, "Baz()", "func()", "field")
+func (Foo) Baz() func() {} //@item(snipMethodBaz, "Baz()", "func()", "method")
+func (Foo) BazBar() func() {} //@item(snipMethodBazBar, "BazBar()", "func()", "method")
 
 func _() {
 	f //@snippet(" //", snipFoo, "foo(${1})", "foo(${1:i int}, ${2:b bool})")
 
 	bar //@snippet(" //", snipBar, "bar(${1})", "bar(${1:fn func()})")
 
-	bar(nil) //@snippet("(", snipBar, "bar", "bar")
+	bar(nil) //@snippet("(", snipBar, "", "")
 	bar(ba) //@snippet(")", snipBar, "bar(${1})", "bar(${1:fn func()})")
 	var f Foo
 	bar(f.Ba) //@snippet(")", snipMethodBaz, "Baz()", "Baz()")
@@ -29,4 +30,11 @@ func _() {
 	Foo{} //@snippet("}", snipFieldBar, "Bar: ${1}", "Bar: ${1:int}")
 
 	Foo{Foo{}.B} //@snippet("} ", snipFieldBar, "Bar", "Bar")
+
+	var err error
+	err.Error() //@snippet("E", Error, "", "")
+	f.Baz()     //@snippet("B", snipMethodBaz, "", "")
+
+	// TODO(rstambler): Handle this case correctly.
+	f.Baz()     //@fails("(", snipMethodBazBar, "Bar", "Bar")
 }

internal/lsp/tests/tests.go

@@ -36,7 +36,7 @@ const (
 	ExpectedHighlightsCount        = 2
 	ExpectedSymbolsCount           = 1
 	ExpectedSignaturesCount        = 20
-	ExpectedCompletionSnippetCount = 11
+	ExpectedCompletionSnippetCount = 13
 	ExpectedLinksCount             = 2
 )