From 4298585011179b3b5cf23d121bc1461cb604d87e Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Thu, 27 Jun 2019 17:50:01 +0000 Subject: [PATCH] internal/lsp: provide deep completion candidates Deep completion refers to searching through an object's fields and methods for more completion candidates. For example: func wantsInt(int) { } var s struct { i int } wantsInt(<>) Will now give a candidate for "s.i" since its type matches the expected type. We limit to three deep completion results. In some cases there are many useless deep completion matches. Showing too many options defeats the purpose of "smart" completions. We also lower a completion item's score according to its depth so that we favor shallower options. For now we do not continue searching past function calls to limit our search scope. In other words, we are not able to suggest results with any chained fields/methods after the first method call. Deep completions are behind the "useDeepCompletions" LSP config flag for now. Change-Id: I1b888c82e5c4b882f9718177ce07811e2bccbf22 GitHub-Last-Rev: 26522363730036e0b382a7bcd10aa1ed825f6866 GitHub-Pull-Request: golang/tools#100 Reviewed-on: https://go-review.googlesource.com/c/tools/+/177622 Reviewed-by: Rebecca Stambler Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot --- internal/lsp/completion.go | 31 ++++++- internal/lsp/general.go | 4 + internal/lsp/lsp_test.go | 6 ++ internal/lsp/server.go | 1 + internal/lsp/source/completion.go | 84 ++++++++++++++----- internal/lsp/source/completion_format.go | 8 +- internal/lsp/source/completion_snippet.go | 20 ++--- internal/lsp/source/deep_completion.go | 84 +++++++++++++++++++ internal/lsp/source/source_test.go | 30 +++++-- internal/lsp/source/util.go | 22 ++--- .../testdata/deepcomplete/deep_complete.go | 61 ++++++++++++++ internal/lsp/tests/tests.go | 4 +- 12 files changed, 297 insertions(+), 58 deletions(-) create mode 100644 internal/lsp/source/deep_completion.go create mode 100644 internal/lsp/testdata/deepcomplete/deep_complete.go diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go index aa2c672c..3fc75b53 100644 --- a/internal/lsp/completion.go +++ b/internal/lsp/completion.go @@ -30,7 +30,9 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara if err != nil { return nil, err } - items, surrounding, err := source.Completion(ctx, view, f, rng.Start) + items, surrounding, err := source.Completion(ctx, view, f, rng.Start, source.CompletionOptions{ + DeepComplete: s.useDeepCompletions, + }) if err != nil { s.session.Logger().Infof(ctx, "no completions found for %s:%v:%v: %v", uri, int(params.Position.Line), int(params.Position.Character), err) } @@ -56,20 +58,41 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara } return &protocol.CompletionList{ IsIncomplete: false, - Items: toProtocolCompletionItems(items, prefix, insertionRng, s.insertTextFormat, s.usePlaceholders), + Items: toProtocolCompletionItems(items, prefix, insertionRng, s.insertTextFormat, s.usePlaceholders, s.useDeepCompletions), }, nil } -func toProtocolCompletionItems(candidates []source.CompletionItem, prefix string, rng protocol.Range, insertTextFormat protocol.InsertTextFormat, usePlaceholders bool) []protocol.CompletionItem { +// Limit deep completion results because in some cases there are too many +// to be useful. +const maxDeepCompletions = 3 + +func toProtocolCompletionItems(candidates []source.CompletionItem, prefix string, rng protocol.Range, insertTextFormat protocol.InsertTextFormat, usePlaceholders bool, useDeepCompletions bool) []protocol.CompletionItem { sort.SliceStable(candidates, func(i, j int) bool { return candidates[i].Score > candidates[j].Score }) - items := make([]protocol.CompletionItem, 0, len(candidates)) + var ( + items = make([]protocol.CompletionItem, 0, len(candidates)) + numDeepCompletionsSeen int + ) for i, candidate := range candidates { // Match against the label. if !strings.HasPrefix(candidate.Label, prefix) { continue } + + // Limit the number of deep completions to not overwhelm the user in cases + // with dozens of deep completion matches. + if candidate.Depth > 0 { + if !useDeepCompletions { + continue + } + + if numDeepCompletionsSeen >= maxDeepCompletions { + continue + } + numDeepCompletionsSeen++ + } + insertText := candidate.InsertText if insertTextFormat == protocol.SnippetTextFormat { insertText = candidate.Snippet(usePlaceholders) diff --git a/internal/lsp/general.go b/internal/lsp/general.go index 1e1af991..c9767609 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -201,6 +201,10 @@ func (s *Server) processConfig(view source.View, config interface{}) error { } } } + // Check if deep completions are enabled. + if useDeepCompletions, ok := c["useDeepCompletions"].(bool); ok { + s.useDeepCompletions = useDeepCompletions + } return nil } diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 263f17d3..8f3eda4a 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -145,12 +145,16 @@ func summarizeDiagnostics(i int, want []source.Diagnostic, got []source.Diagnost } func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests.CompletionSnippets, items tests.CompletionItems) { + defer func() { r.server.useDeepCompletions = false }() + for src, itemList := range data { var want []source.CompletionItem for _, pos := range itemList { want = append(want, *items[pos]) } + r.server.useDeepCompletions = strings.Contains(string(src.URI()), "deepcomplete") + list := r.runCompletion(t, src) wantBuiltins := strings.Contains(string(src.URI()), "builtins") @@ -178,6 +182,8 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests r.server.usePlaceholders = usePlaceholders for src, want := range snippets { + r.server.useDeepCompletions = strings.Contains(string(src.URI()), "deepcomplete") + list := r.runCompletion(t, src) wantItem := items[want.CompletionItem] diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 771e14a3..97f8b5b7 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -72,6 +72,7 @@ type Server struct { // TODO(rstambler): Separate these into their own struct? usePlaceholders bool noDocsOnHover bool + useDeepCompletions bool insertTextFormat protocol.InsertTextFormat configurationSupported bool dynamicConfigurationSupported bool diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index fb8fb55d..d06f9c6b 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -31,6 +31,11 @@ type CompletionItem struct { Kind CompletionItemKind + // Depth is how many levels were searched to find this completion. + // For example when completing "foo<>", "fooBar" is depth 0, and + // "fooBar.Baz" is depth 1. + Depth int + // Score is the internal relevance score. // A higher score indicates that this completion item is more relevant. Score float64 @@ -140,6 +145,9 @@ type completer struct { // enclosingCompositeLiteral contains information about the composite literal // enclosing the position. enclosingCompositeLiteral *compLitInfo + + // deepState contains the current state of our deep completion search. + deepState deepCompletionState } type compLitInfo struct { @@ -190,17 +198,29 @@ func (c *completer) setSurrounding(ident *ast.Ident) { } } -// found adds a candidate completion. -// -// Only the first candidate of a given name is considered. +// found adds a candidate completion. We will also search through the object's +// members for more candidates. func (c *completer) found(obj types.Object, score float64) { if obj.Pkg() != nil && obj.Pkg() != c.types && !obj.Exported() { return // inaccessible } - if c.seen[obj] { - return + + if c.inDeepCompletion() { + // When searching deep, just make sure we don't have a cycle in our chain. + // We don't dedupe by object because we want to allow both "foo.Baz" and + // "bar.Baz" even though "Baz" is represented the same types.Object in both. + for _, seenObj := range c.deepState.chain { + if seenObj == obj { + return + } + } + } else { + // At the top level, dedupe by object. + if c.seen[obj] { + return + } + c.seen[obj] = true } - c.seen[obj] = true cand := candidate{ obj: obj, @@ -211,7 +231,12 @@ func (c *completer) found(obj types.Object, score float64) { cand.score *= highScore } + // Favor shallow matches by lowering weight according to depth. + cand.score -= stdScore * float64(len(c.deepState.chain)) + c.items = append(c.items, c.item(cand)) + + c.deepSearch(obj) } // candidate represents a completion candidate. @@ -227,13 +252,17 @@ type candidate struct { expandFuncCall bool } +type CompletionOptions struct { + DeepComplete bool +} + // Completion returns a list of possible candidates for completion, given a // a file and a position. // // The selection is computed based on the preceding identifier and can be used by // the client to score the quality of the completion. For instance, some clients // may tolerate imperfect matches as valid completion results, since users may make typos. -func Completion(ctx context.Context, view View, f GoFile, pos token.Pos) ([]CompletionItem, *Selection, error) { +func Completion(ctx context.Context, view View, f GoFile, pos token.Pos, opts CompletionOptions) ([]CompletionItem, *Selection, error) { file := f.GetAST(ctx) if file == nil { return nil, nil, fmt.Errorf("no AST for %s", f.URI()) @@ -275,6 +304,8 @@ func Completion(ctx context.Context, view View, f GoFile, pos token.Pos) ([]Comp enclosingCompositeLiteral: clInfo, } + c.deepState.enabled = opts.DeepComplete + // Set the filter surrounding. if ident, ok := path[0].(*ast.Ident); ok { c.setSurrounding(ident) @@ -366,11 +397,7 @@ func (c *completer) selector(sel *ast.SelectorExpr) error { // Is sel a qualified identifier? if id, ok := sel.X.(*ast.Ident); ok { if pkgname, ok := c.info.Uses[id].(*types.PkgName); ok { - // Enumerate package members. - scope := pkgname.Imported().Scope() - for _, name := range scope.Names() { - c.found(scope.Lookup(name), stdScore) - } + c.packageMembers(pkgname) return nil } } @@ -381,22 +408,33 @@ func (c *completer) selector(sel *ast.SelectorExpr) error { return fmt.Errorf("cannot resolve %s", sel.X) } - // Add methods of T. - mset := types.NewMethodSet(tv.Type) + return c.methodsAndFields(tv.Type, tv.Addressable()) +} + +func (c *completer) packageMembers(pkg *types.PkgName) { + scope := pkg.Imported().Scope() + for _, name := range scope.Names() { + c.found(scope.Lookup(name), stdScore) + } +} + +func (c *completer) methodsAndFields(typ types.Type, addressable bool) error { + var mset *types.MethodSet + + if addressable && !types.IsInterface(typ) && !isPointer(typ) { + // Add methods of *T, which includes methods with receiver T. + mset = types.NewMethodSet(types.NewPointer(typ)) + } else { + // Add methods of T. + mset = types.NewMethodSet(typ) + } + for i := 0; i < mset.Len(); i++ { c.found(mset.At(i).Obj(), stdScore) } - // Add methods of *T. - if tv.Addressable() && !types.IsInterface(tv.Type) && !isPointer(tv.Type) { - mset := types.NewMethodSet(types.NewPointer(tv.Type)) - for i := 0; i < mset.Len(); i++ { - c.found(mset.At(i).Obj(), stdScore) - } - } - // Add fields of T. - for _, f := range fieldSelections(tv.Type) { + for _, f := range fieldSelections(typ) { c.found(f, stdScore) } return nil diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index bdb8cd1b..abad1f9f 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -26,7 +26,7 @@ func (c *completer) item(cand candidate) CompletionItem { } var ( - label = obj.Name() + label = c.deepState.chainString(obj.Name()) detail = types.TypeString(obj.Type(), c.qf) insert = label kind CompletionItemKind @@ -38,9 +38,9 @@ func (c *completer) item(cand candidate) CompletionItem { // 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) results, writeParens := formatResults(sig.Results(), c.qf) - label, detail = formatFunction(obj.Name(), params, results, writeParens) - plainSnippet, placeholderSnippet = c.functionCallSnippets(obj.Name(), params) + label, detail = formatFunction(label, params, results, writeParens) } switch obj := obj.(type) { @@ -69,7 +69,6 @@ func (c *completer) item(cand candidate) CompletionItem { if !ok { break } - kind = FunctionCompletionItem if sig != nil && sig.Recv() != nil { kind = MethodCompletionItem @@ -91,6 +90,7 @@ func (c *completer) item(cand candidate) CompletionItem { Detail: detail, Kind: kind, Score: cand.score, + Depth: len(c.deepState.chain), plainSnippet: plainSnippet, placeholderSnippet: placeholderSnippet, } diff --git a/internal/lsp/source/completion_snippet.go b/internal/lsp/source/completion_snippet.go index 9b1841fe..fd21acac 100644 --- a/internal/lsp/source/completion_snippet.go +++ b/internal/lsp/source/completion_snippet.go @@ -13,24 +13,24 @@ import ( // structFieldSnippets calculates the plain and placeholder snippets for struct literal field names. func (c *completer) structFieldSnippets(label, detail string) (*snippet.Builder, *snippet.Builder) { - clInfo := c.enclosingCompositeLiteral - - if clInfo == nil || !clInfo.isStruct() { + if !c.wantStructFieldCompletions() { return nil, 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 + } + + 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 } - // We don't want snippet unless we are completing a field name. maybeInFieldName - // means we _might_ not be a struct field name, but this method is only called for - // struct fields, so we can ignore that possibility. - if !clInfo.inKey && !clInfo.maybeInFieldName { - return nil, nil - } - plain, placeholder := &snippet.Builder{}, &snippet.Builder{} label = fmt.Sprintf("%s: ", label) diff --git a/internal/lsp/source/deep_completion.go b/internal/lsp/source/deep_completion.go new file mode 100644 index 00000000..bb1fbbff --- /dev/null +++ b/internal/lsp/source/deep_completion.go @@ -0,0 +1,84 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package source + +import ( + "go/types" + "strings" +) + +// deepCompletionState stores our state as we search for deep completions. +// "deep completion" refers to searching into objects' fields and methods to +// find more completion candidates. +type deepCompletionState struct { + // enabled is true if deep completions are enabled. + enabled bool + + // chain holds the traversal path as we do a depth-first search through + // objects' members looking for exact type matches. + chain []types.Object + + // chainNames holds the names of the chain objects. This allows us to + // save allocations as we build many deep completion items. + chainNames []string +} + +// push pushes obj onto our search stack. +func (s *deepCompletionState) push(obj types.Object) { + s.chain = append(s.chain, obj) + s.chainNames = append(s.chainNames, obj.Name()) +} + +// pop pops the last object off our search stack. +func (s *deepCompletionState) pop() { + s.chain = s.chain[:len(s.chain)-1] + s.chainNames = s.chainNames[:len(s.chainNames)-1] +} + +// chainString joins the chain of objects' names together on ".". +func (s *deepCompletionState) chainString(finalName string) string { + s.chainNames = append(s.chainNames, finalName) + chainStr := strings.Join(s.chainNames, ".") + s.chainNames = s.chainNames[:len(s.chainNames)-1] + return chainStr +} + +func (c *completer) inDeepCompletion() bool { + return len(c.deepState.chain) > 0 +} + +// deepSearch searches through obj's subordinate objects for more +// completion items. +func (c *completer) deepSearch(obj types.Object) { + if !c.deepState.enabled { + return + } + + // Don't search into type names. + if isTypeName(obj) { + return + } + + // Don't search embedded fields because they were already included in their + // parent's fields. + if v, ok := obj.(*types.Var); ok && v.Embedded() { + return + } + + // Push this object onto our search stack. + c.deepState.push(obj) + + switch obj := obj.(type) { + case *types.PkgName: + c.packageMembers(obj) + default: + // For now it is okay to assume obj is addressable since we don't search beyond + // function calls. + c.methodsAndFields(obj.Type(), true) + } + + // Pop the object off our search stack. + c.deepState.pop() +} diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 4877614b..9d212021 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -146,7 +146,9 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests t.Fatalf("failed to get token for %v", src) } pos := tok.Pos(src.Start().Offset()) - list, surrounding, err := source.Completion(ctx, r.view, f.(source.GoFile), pos) + list, surrounding, err := source.Completion(ctx, r.view, f.(source.GoFile), pos, source.CompletionOptions{ + DeepComplete: strings.Contains(string(src.URI()), "deepcomplete"), + }) if err != nil { t.Fatalf("failed for %v: %v", src, err) } @@ -179,7 +181,9 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests } tok := f.GetToken(ctx) pos := tok.Pos(src.Start().Offset()) - list, _, err := source.Completion(ctx, r.view, f.(source.GoFile), pos) + list, _, err := source.Completion(ctx, r.view, f.(source.GoFile), pos, source.CompletionOptions{ + DeepComplete: strings.Contains(string(src.URI()), "deepcomplete"), + }) if err != nil { t.Fatalf("failed for %v: %v", src, err) } @@ -227,12 +231,28 @@ func isBuiltin(item source.CompletionItem) bool { // diffCompletionItems prints the diff between expected and actual completion // test results. func diffCompletionItems(t *testing.T, spn span.Span, want []source.CompletionItem, got []source.CompletionItem) string { - if len(got) != len(want) { - return summarizeCompletionItems(-1, want, got, "different lengths got %v want %v", len(got), len(want)) - } sort.SliceStable(got, func(i, j int) bool { return got[i].Score > got[j].Score }) + + // duplicate the lsp/completion logic to limit deep candidates to keep expected + // list short + var idx, seenDeepCompletions int + for _, item := range got { + if item.Depth > 0 { + if seenDeepCompletions >= 3 { + continue + } + seenDeepCompletions++ + } + got[idx] = item + idx++ + } + got = got[:idx] + + if len(got) != len(want) { + return summarizeCompletionItems(-1, want, got, "different lengths got %v want %v", len(got), len(want)) + } for i, w := range want { g := got[i] if w.Label != g.Label { diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index a5aad79a..ef7d7828 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -38,18 +38,20 @@ func fieldSelections(T types.Type) (fields []*types.Var) { // selections that match more than one field/method. // types.NewSelectionSet should do that for us. - seen := make(map[types.Type]bool) // for termination on recursive types + seen := make(map[*types.Var]bool) // for termination on recursive types + var visit func(T types.Type) visit = func(T types.Type) { - if !seen[T] { - seen[T] = true - if T, ok := deref(T).Underlying().(*types.Struct); ok { - for i := 0; i < T.NumFields(); i++ { - f := T.Field(i) - fields = append(fields, f) - if f.Anonymous() { - visit(f.Type()) - } + if T, ok := deref(T).Underlying().(*types.Struct); ok { + for i := 0; i < T.NumFields(); i++ { + f := T.Field(i) + if seen[f] { + continue + } + seen[f] = true + fields = append(fields, f) + if f.Anonymous() { + visit(f.Type()) } } } diff --git a/internal/lsp/testdata/deepcomplete/deep_complete.go b/internal/lsp/testdata/deepcomplete/deep_complete.go new file mode 100644 index 00000000..1a56fbaa --- /dev/null +++ b/internal/lsp/testdata/deepcomplete/deep_complete.go @@ -0,0 +1,61 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package deepcomplete + +import "context" //@item(ctxPackage, "context", "\"context\"", "package") + +type deepA struct { + b deepB //@item(deepBField, "b", "deepB", "field") +} + +type deepB struct { +} + +func wantsDeepB(deepB) {} + +func _() { + var a deepA //@item(deepAVar, "a", "deepA", "var") + a.b //@item(deepABField, "a.b", "deepB", "field") + wantsDeepB(a) //@complete(")", deepABField, deepAVar) + + deepA{a} //@snippet("}", deepABField, "a.b", "a.b") +} + +func wantsContext(context.Context) {} + +func _() { + context.Background() //@item(ctxBackground, "context.Background()", "context.Context", "func") + context.TODO() //@item(ctxTODO, "context.TODO()", "context.Context", "func") + /* context.WithValue(parent context.Context, key interface{}, val interface{}) */ //@item(ctxWithValue, "context.WithValue(parent context.Context, key interface{}, val interface{})", "context.Context", "func") + + wantsContext(c) //@complete(")", ctxBackground, ctxTODO, ctxWithValue, ctxPackage) +} + +func _() { + type deepCircle struct { + *deepCircle + } + var circle deepCircle //@item(deepCircle, "circle", "deepCircle", "var") + circle.deepCircle //@item(deepCircleField, "circle.deepCircle", "*deepCircle", "field") + var _ deepCircle = ci //@complete(" //", deepCircle, deepCircleField) +} + +func _() { + type deepEmbedC struct { + } + type deepEmbedB struct { + deepEmbedC + } + type deepEmbedA struct { + deepEmbedB + } + + wantsC := func(deepEmbedC) {} + + var a deepEmbedA //@item(deepEmbedA, "a", "deepEmbedA", "var") + a.deepEmbedB //@item(deepEmbedB, "a.deepEmbedB", "deepEmbedB", "field") + a.deepEmbedC //@item(deepEmbedC, "a.deepEmbedC", "deepEmbedC", "field") + wantsC(a) //@complete(")", deepEmbedC, deepEmbedA, deepEmbedB) +} diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index 24b026d4..da06a384 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -25,8 +25,8 @@ import ( // We hardcode the expected number of test cases to ensure that all tests // are being executed. If a test is added, this number must be changed. const ( - ExpectedCompletionsCount = 132 - ExpectedCompletionSnippetCount = 14 + ExpectedCompletionsCount = 136 + ExpectedCompletionSnippetCount = 15 ExpectedDiagnosticsCount = 17 ExpectedFormatCount = 5 ExpectedImportCount = 2