From 473d3dc1d7ebfe600ef76607c3e200794eaf947a Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Tue, 14 May 2019 22:02:51 -0400 Subject: [PATCH] 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 Reviewed-by: Ian Cottrell --- internal/lsp/lsp_test.go | 31 +++++++--------------- internal/lsp/source/completion_format.go | 3 +++ internal/lsp/source/completion_snippet.go | 1 - internal/lsp/source/source_test.go | 17 +++++------- internal/lsp/testdata/snippets/snippets.go | 12 +++++++-- internal/lsp/tests/tests.go | 2 +- 6 files changed, 29 insertions(+), 37 deletions(-) diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index dd3c9c19..caa9b4be 100644 --- a/internal/lsp/lsp_test.go +++ b/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] - var gotItem *protocol.CompletionItem + wantItem := items[want.CompletionItem] + var got *protocol.CompletionItem for _, item := range list.Items { - if item.Label == wantCompletion.Label { - gotItem = &item + if item.Label == wantItem.Label { + got = &item break } } - - if gotItem == nil { - t.Fatalf("%s: couldn't find completion matching %q", src.URI(), wantCompletion.Label) + if got == nil { + t.Fatalf("%s: couldn't find completion matching %q", src.URI(), wantItem.Label) } - var expected string if usePlaceholders { expected = want.PlaceholderSnippet } else { expected = want.PlainSnippet } - - if expected != gotItem.TextEdit.NewText { - t.Errorf("%s: expected snippet %q, got %q", src, expected, gotItem.TextEdit.NewText) + if expected != got.TextEdit.NewText { + t.Errorf("%s: expected snippet %q, got %q", src, expected, got.TextEdit.NewText) } } } diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index da3774e8..3657c230 100644 --- a/internal/lsp/source/completion_format.go +++ b/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 diff --git a/internal/lsp/source/completion_snippet.go b/internal/lsp/source/completion_snippet.go index 2e8b0e93..8ac4fab3 100644 --- a/internal/lsp/source/completion_snippet.go +++ b/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) diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 577d3254..69005c80 100644 --- a/internal/lsp/source/source_test.go +++ b/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) } - - wantCompletion := items[want.CompletionItem] + wantItem := 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 { - t.Errorf("%s: expected snippet %q, got %q", src, expected, insertText) + if actual := got.Snippet(usePlaceholders); expected != actual { + t.Errorf("%s: expected placeholder snippet %q, got %q", src, expected, actual) } } } diff --git a/internal/lsp/testdata/snippets/snippets.go b/internal/lsp/testdata/snippets/snippets.go index d9c9580b..af10c885 100644 --- a/internal/lsp/testdata/snippets/snippets.go +++ b/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") } diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index a2631601..88cc983c 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -36,7 +36,7 @@ const ( ExpectedHighlightsCount = 2 ExpectedSymbolsCount = 1 ExpectedSignaturesCount = 20 - ExpectedCompletionSnippetCount = 11 + ExpectedCompletionSnippetCount = 13 ExpectedLinksCount = 2 )