diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go index 77850e2609..7b330633b2 100644 --- a/internal/lsp/completion.go +++ b/internal/lsp/completion.go @@ -70,8 +70,9 @@ func (s *Server) toProtocolCompletionItems(candidates []source.CompletionItem, r } insertText := candidate.InsertText if options.InsertTextFormat == protocol.SnippetTextFormat { - insertText = candidate.Snippet(options.UsePlaceholders) + insertText = candidate.Snippet() } + item := protocol.CompletionItem{ Label: candidate.Label, Detail: candidate.Detail, diff --git a/internal/lsp/general.go b/internal/lsp/general.go index 0f49faa1b7..f48507090f 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -322,7 +322,7 @@ func (s *Server) processConfig(ctx context.Context, view source.View, options *s setBool(&options.Completion.Unimported, c, "wantUnimportedCompletions") // If the user wants placeholders for autocompletion results. - setBool(&options.UsePlaceholders, c, "usePlaceholders") + setBool(&options.Completion.Placeholders, c, "usePlaceholders") return nil } diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index fd9fbe7ec6..b756ebf745 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -153,12 +153,11 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests modified.InsertTextFormat = protocol.SnippetTextFormat for _, usePlaceholders := range []bool{true, false} { - modified.UsePlaceholders = usePlaceholders - for src, want := range snippets { modified.Completion.Deep = strings.Contains(string(src.URI()), "deepcomplete") modified.Completion.FuzzyMatching = strings.Contains(string(src.URI()), "fuzzymatch") modified.Completion.Unimported = strings.Contains(string(src.URI()), "unimported") + modified.Completion.Placeholders = usePlaceholders r.server.session.SetOptions(modified) list := r.runCompletion(t, src) diff --git a/internal/lsp/snippet/snippet_builder.go b/internal/lsp/snippet/snippet_builder.go index 14c5d712b3..a17091cab3 100644 --- a/internal/lsp/snippet/snippet_builder.go +++ b/internal/lsp/snippet/snippet_builder.go @@ -43,9 +43,8 @@ func (b *Builder) WriteText(s string) { // The callback style allows for creating nested placeholders. To write an // empty tab stop, provide a nil callback. func (b *Builder) WritePlaceholder(fn func(*Builder)) { - fmt.Fprintf(&b.sb, "${%d", b.nextTabStop()) + fmt.Fprintf(&b.sb, "${%d:", b.nextTabStop()) if fn != nil { - b.sb.WriteByte(':') fn(b) } b.sb.WriteByte('}') diff --git a/internal/lsp/snippet/snippet_builder_test.go b/internal/lsp/snippet/snippet_builder_test.go index 810a7fe195..bb47f52e47 100644 --- a/internal/lsp/snippet/snippet_builder_test.go +++ b/internal/lsp/snippet/snippet_builder_test.go @@ -10,6 +10,8 @@ import ( func TestSnippetBuilder(t *testing.T) { expect := func(expected string, fn func(*Builder)) { + t.Helper() + var b Builder fn(&b) if got := b.String(); got != expected { @@ -23,7 +25,7 @@ func TestSnippetBuilder(t *testing.T) { b.WriteText(`hi { } $ | " , / \`) }) - expect("${1}", func(b *Builder) { + expect("${1:}", func(b *Builder) { b.WritePlaceholder(nil) }) diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index faa2aaaff8..497ecbffec 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -55,21 +55,9 @@ type CompletionItem struct { // A higher score indicates that this completion item is more relevant. Score float64 - // Snippet is the LSP snippet for the completion item, without placeholders. - // The LSP specification contains details about LSP snippets. - // For example, a snippet for a function with the following signature: - // - // func foo(a, b, c int) - // - // would be: - // - // foo(${1:}) - // - plainSnippet *snippet.Builder - - // PlaceholderSnippet is the LSP snippet for the completion ite, containing - // placeholders. The LSP specification contains details about LSP snippets. - // For example, a placeholder snippet for a function with the following signature: + // snippet is the LSP snippet for the completion item. The LSP + // specification contains details about LSP snippets. For example, a + // snippet for a function with the following signature: // // func foo(a, b, c int) // @@ -77,22 +65,22 @@ type CompletionItem struct { // // foo(${1:a int}, ${2: b int}, ${3: c int}) // - placeholderSnippet *snippet.Builder + // If Placeholders is false in the CompletionOptions, the above + // snippet would instead be: + // + // foo(${1:}) + snippet *snippet.Builder // Documentation is the documentation for the completion item. Documentation string } -// Snippet is a convenience function that determines the snippet that should be +// Snippet is a convenience returns the snippet if available, otherwise +// the InsertText. // used for an item, depending on if the callee wants placeholders or not. -func (i *CompletionItem) Snippet(usePlaceholders bool) string { - if usePlaceholders { - if i.placeholderSnippet != nil { - return i.placeholderSnippet.String() - } - } - if i.plainSnippet != nil { - return i.plainSnippet.String() +func (i *CompletionItem) Snippet() string { + if i.snippet != nil { + return i.snippet.String() } return i.InsertText } diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index 587c5fc6a0..e86ab2911d 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -30,20 +30,19 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { } var ( - label = cand.name - detail = types.TypeString(obj.Type(), c.qf) - insert = label - kind CompletionItemKind - plainSnippet *snippet.Builder - placeholderSnippet *snippet.Builder - protocolEdits []protocol.TextEdit + label = cand.name + detail = types.TypeString(obj.Type(), c.qf) + insert = label + kind CompletionItemKind + snip *snippet.Builder + protocolEdits []protocol.TextEdit ) - // expandFuncCall mutates the completion label, detail, and snippets + // expandFuncCall mutates the completion label, detail, and snippet // to that of an invocation of sig. expandFuncCall := func(sig *types.Signature) { params := formatParams(sig.Params(), sig.Variadic(), c.qf) - plainSnippet, placeholderSnippet = c.functionCallSnippets(label, params) + snip = c.functionCallSnippet(label, params) results, writeParens := formatResults(sig.Results(), c.qf) detail = "func" + formatFunction(params, results, writeParens) } @@ -59,7 +58,7 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { } if obj.IsField() { kind = FieldCompletionItem - plainSnippet, placeholderSnippet = c.structFieldSnippets(label, detail) + snip = c.structFieldSnippet(label, detail) } else if c.isParameter(obj) { kind = ParameterCompletionItem } else { @@ -110,8 +109,7 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { Kind: kind, Score: cand.score, Depth: len(c.deepState.chain), - plainSnippet: plainSnippet, - placeholderSnippet: placeholderSnippet, + snippet: snip, } // If the user doesn't want documentation for completion items. if !c.opts.Documentation { @@ -200,7 +198,7 @@ func (c *completer) formatBuiltin(cand candidate) CompletionItem { results, writeResultParens := formatFieldList(c.ctx, c.view, decl.Type.Results) item.Label = obj.Name() item.Detail = "func" + formatFunction(params, results, writeResultParens) - item.plainSnippet, item.placeholderSnippet = c.functionCallSnippets(obj.Name(), params) + item.snippet = c.functionCallSnippet(obj.Name(), params) case *types.TypeName: if types.IsInterface(obj.Type()) { item.Kind = InterfaceCompletionItem diff --git a/internal/lsp/source/completion_snippet.go b/internal/lsp/source/completion_snippet.go index 90cf02a33a..02c56f50a8 100644 --- a/internal/lsp/source/completion_snippet.go +++ b/internal/lsp/source/completion_snippet.go @@ -5,57 +5,53 @@ package source import ( - "fmt" "go/ast" "golang.org/x/tools/internal/lsp/snippet" ) -// structFieldSnippets calculates the plain and placeholder snippets for struct literal field names. -func (c *completer) structFieldSnippets(label, detail string) (*snippet.Builder, *snippet.Builder) { +// structFieldSnippets calculates the snippet for struct literal field names. +func (c *completer) structFieldSnippet(label, detail string) *snippet.Builder { if !c.wantStructFieldCompletions() { - return nil, nil + return nil } // If we are in a deep completion then we can't be completing a field // name (e.g. "Foo{f<>}" completing to "Foo{f.Bar}" should not generate // a snippet). if c.inDeepCompletion() { - return nil, nil + return nil } clInfo := c.enclosingCompositeLiteral // If we are already in a key-value expression, we don't want a snippet. if clInfo.kv != nil { - return nil, nil + return nil } - plain, placeholder := &snippet.Builder{}, &snippet.Builder{} - label = fmt.Sprintf("%s: ", label) + snip := &snippet.Builder{} // A plain snippet turns "Foo{Ba<>" into "Foo{Bar: <>". - plain.WriteText(label) - plain.WritePlaceholder(nil) - - // A placeholder snippet turns "Foo{Ba<>" into "Foo{Bar: <*int*>". - placeholder.WriteText(label) - placeholder.WritePlaceholder(func(b *snippet.Builder) { - b.WriteText(detail) + snip.WriteText(label + ": ") + snip.WritePlaceholder(func(b *snippet.Builder) { + // A placeholder snippet turns "Foo{Ba<>" into "Foo{Bar: <*int*>". + if c.opts.Placeholders { + b.WriteText(detail) + } }) // If the cursor position is on a different line from the literal's opening brace, // we are in a multiline literal. if c.view.Session().Cache().FileSet().Position(c.pos).Line != c.view.Session().Cache().FileSet().Position(clInfo.cl.Lbrace).Line { - plain.WriteText(",") - placeholder.WriteText(",") + snip.WriteText(",") } - return plain, placeholder + return snip } -// functionCallSnippets calculates the plain and placeholder snippets for function calls. -func (c *completer) functionCallSnippets(name string, params []string) (*snippet.Builder, *snippet.Builder) { +// functionCallSnippets calculates the snippet for function calls. +func (c *completer) functionCallSnippet(name string, params []string) *snippet.Builder { // If we are the left side (i.e. "Fun") part of a call expression, // we don't want a snippet since there are already parens present. if len(c.path) > 1 { @@ -65,37 +61,37 @@ func (c *completer) functionCallSnippets(name string, params []string) (*snippet // inserted when fixing the AST. In this case, we do still need // to insert the calling "()" parens. if n.Fun == c.path[0] && n.Lparen != n.Rparen { - return nil, nil + return nil } case *ast.SelectorExpr: if len(c.path) > 2 { if call, ok := c.path[2].(*ast.CallExpr); ok && call.Fun == c.path[1] && call.Lparen != call.Rparen { - return nil, nil + return nil } } } } - plain, placeholder := &snippet.Builder{}, &snippet.Builder{} - label := fmt.Sprintf("%s(", name) + snip := &snippet.Builder{} + snip.WriteText(name + "(") - // A plain snippet turns "someFun<>" into "someFunc(<>)". - plain.WriteText(label) - if len(params) > 0 { - plain.WritePlaceholder(nil) - } - plain.WriteText(")") - - // A placeholder snippet turns "someFun<>" into "someFunc(<*i int*>, *s string*)". - placeholder.WriteText(label) - for i, p := range params { - if i > 0 { - placeholder.WriteText(", ") + if c.opts.Placeholders { + // A placeholder snippet turns "someFun<>" into "someFunc(<*i int*>, *s string*)". + for i, p := range params { + if i > 0 { + snip.WriteText(", ") + } + snip.WritePlaceholder(func(b *snippet.Builder) { + b.WriteText(p) + }) + } + } else { + // A plain snippet turns "someFun<>" into "someFunc(<>)". + if len(params) > 0 { + snip.WritePlaceholder(nil) } - placeholder.WritePlaceholder(func(b *snippet.Builder) { - b.WriteText(p) - }) } - placeholder.WriteText(")") - return plain, placeholder + snip.WriteText(")") + + return snip } diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 6770902031..81596f8284 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -31,7 +31,6 @@ var ( type SessionOptions struct { Env []string BuildFlags []string - UsePlaceholders bool HoverKind HoverKind DisabledAnalyses map[string]struct{} @@ -59,6 +58,7 @@ type CompletionOptions struct { Unimported bool Documentation bool FullDocumentation bool + Placeholders bool } type HoverKind int diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 51b220e2aa..f0ec5bdda6 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -167,6 +167,7 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests Documentation: true, Deep: strings.Contains(string(src.URI()), "deepcomplete"), FuzzyMatching: strings.Contains(string(src.URI()), "fuzzymatch"), + Placeholders: usePlaceholders, }) if err != nil { t.Fatalf("failed for %v: %v", src, err) @@ -186,8 +187,18 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests if usePlaceholders { expected = want.PlaceholderSnippet } - if actual := got.Snippet(usePlaceholders); expected != actual { - t.Errorf("%s: expected placeholder snippet %q, got %q", src, expected, actual) + if expected == "" { + if got != nil { + t.Fatalf("%s:%d: expected no matching snippet", src.URI(), src.Start().Line()) + } + } else { + if got == nil { + t.Fatalf("%s:%d: couldn't find completion matching %q", src.URI(), src.Start().Line(), wantItem.Label) + } + actual := got.Snippet() + if 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.in similarity index 64% rename from internal/lsp/testdata/snippets/snippets.go rename to internal/lsp/testdata/snippets/snippets.go.in index ee56eb3c86..872753b7b7 100644 --- a/internal/lsp/testdata/snippets/snippets.go +++ b/internal/lsp/testdata/snippets/snippets.go.in @@ -11,23 +11,23 @@ func (Foo) Baz() func() {} //@item(snipMethodBaz, "Baz", "func() func()", "metho func (Foo) BazBar() func() {} //@item(snipMethodBazBar, "BazBar", "func() func()", "method") func _() { - f //@snippet(" //", snipFoo, "foo(${1})", "foo(${1:i int}, ${2:b bool})") + f //@snippet(" //", snipFoo, "foo(${1:})", "foo(${1:i int}, ${2:b bool})") - bar //@snippet(" //", snipBar, "bar(${1})", "bar(${1:fn func()})") + bar //@snippet(" //", snipBar, "bar(${1:})", "bar(${1:fn func()})") bar(nil) //@snippet("(", snipBar, "bar", "bar") - bar(ba) //@snippet(")", snipBar, "bar(${1})", "bar(${1:fn func()})") + bar(ba) //@snippet(")", snipBar, "bar(${1:})", "bar(${1:fn func()})") var f Foo bar(f.Ba) //@snippet(")", snipMethodBaz, "Baz()", "Baz()") - (bar)(nil) //@snippet(")", snipBar, "bar(${1})", "bar(${1:fn func()})") + (bar)(nil) //@snippet(")", snipBar, "bar(${1:})", "bar(${1:fn func()})") (f.Ba)() //@snippet(")", snipMethodBaz, "Baz()", "Baz()") Foo{ - B //@snippet(" //", snipFieldBar, "Bar: ${1},", "Bar: ${1:int},") + B //@snippet(" //", snipFieldBar, "Bar: ${1:},", "Bar: ${1:int},") } - Foo{B} //@snippet("}", snipFieldBar, "Bar: ${1}", "Bar: ${1:int}") - Foo{} //@snippet("}", snipFieldBar, "Bar: ${1}", "Bar: ${1:int}") + Foo{B} //@snippet("}", snipFieldBar, "Bar: ${1:}", "Bar: ${1:int}") + Foo{} //@snippet("}", snipFieldBar, "Bar: ${1:}", "Bar: ${1:int}") Foo{Foo{}.B} //@snippet("} ", snipFieldBar, "Bar", "Bar")