internal/lsp: simplify snippet config/generation
I moved the "usePlaceholders" config field on to CompletionOptions.
This way the completion code generates a single snippet with a little
conditional logic based on the "WantPlaceholders" option instead of
juggling the generation of two almost identical "plain" and
"placeholder" snippets at the same time. It also reduces the work done
generating completion candidates a little.
I also made a minor tweak to the snippet builder where empty
placeholders are now always represented as e.g "${1:}" instead of
"${1}" or "${1:}", depending on if you passed a callback to
WritePlaceholder() or not.
Change-Id: Ib84cc0cd729a11b9e13ad3ac4b6fd2d82460acd5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193697
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go
index 77850e2..7b33063 100644
--- a/internal/lsp/completion.go
+++ b/internal/lsp/completion.go
@@ -70,8 +70,9 @@
}
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 0f49faa..f485070 100644
--- a/internal/lsp/general.go
+++ b/internal/lsp/general.go
@@ -322,7 +322,7 @@
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 fd9fbe7..b756ebf 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -153,12 +153,11 @@
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 14c5d71..a17091c 100644
--- a/internal/lsp/snippet/snippet_builder.go
+++ b/internal/lsp/snippet/snippet_builder.go
@@ -43,9 +43,8 @@
// 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 810a7fe1..bb47f52 100644
--- a/internal/lsp/snippet/snippet_builder_test.go
+++ b/internal/lsp/snippet/snippet_builder_test.go
@@ -10,6 +10,8 @@
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 @@
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 faa2aaa..497ecbf 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -55,21 +55,9 @@
// 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 @@
//
// 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 587c5fc..e86ab29 100644
--- a/internal/lsp/source/completion_format.go
+++ b/internal/lsp/source/completion_format.go
@@ -30,20 +30,19 @@
}
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 @@
}
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 @@
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 @@
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 90cf02a..02c56f5 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 @@
// 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)
+ })
}
- placeholder.WritePlaceholder(func(b *snippet.Builder) {
- b.WriteText(p)
- })
+ } else {
+ // A plain snippet turns "someFun<>" into "someFunc(<>)".
+ if len(params) > 0 {
+ snip.WritePlaceholder(nil)
+ }
}
- placeholder.WriteText(")")
- return plain, placeholder
+ snip.WriteText(")")
+
+ return snip
}
diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go
index 6770902..81596f8 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -31,7 +31,6 @@
type SessionOptions struct {
Env []string
BuildFlags []string
- UsePlaceholders bool
HoverKind HoverKind
DisabledAnalyses map[string]struct{}
@@ -59,6 +58,7 @@
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 51b220e..f0ec5bd 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -167,6 +167,7 @@
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 @@
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 ee56eb3..872753b 100644
--- a/internal/lsp/testdata/snippets/snippets.go
+++ b/internal/lsp/testdata/snippets/snippets.go.in
@@ -11,23 +11,23 @@
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")
