From 4f9eeaf1bf48b6552059d5949ad1c86b5091ff21 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Tue, 2 Jul 2019 17:31:31 -0400 Subject: [PATCH] internal/lsp: add documentation to completion items This change adds documentation to the completion items. This normally should be done in completionItem/resolve, since it takes more time to compute documentation. However, I am not sure if that latency incurred by pre-computing documentation is actually significantly more than the latency incurred by an extra call to 'completionItem/resolve'. This needs to be investigated, so we begin by just precomputing all of the documentation for each item. Updates golang/go#29151 Change-Id: I148664d271cf3f1d089c1a871901e3ee404ffbe8 Reviewed-on: https://go-review.googlesource.com/c/tools/+/184721 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/completion.go | 75 +++++++++++------------- internal/lsp/general.go | 5 +- internal/lsp/server.go | 9 +-- internal/lsp/source/completion.go | 24 +++++--- internal/lsp/source/completion_format.go | 39 ++++++++++-- internal/lsp/source/hover.go | 12 +++- internal/lsp/source/signature_help.go | 2 +- 7 files changed, 104 insertions(+), 62 deletions(-) diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go index ecd3968f1eb..d1103ff4b40 100644 --- a/internal/lsp/completion.go +++ b/internal/lsp/completion.go @@ -31,34 +31,15 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara return nil, err } candidates, surrounding, err := source.Completion(ctx, view, f, rng.Start, source.CompletionOptions{ - DeepComplete: s.useDeepCompletions, + DeepComplete: s.useDeepCompletions, + WantDocumentaton: s.wantCompletionDocumentation, }) 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) } - // We might need to adjust the position to account for the prefix. - insertionRng := protocol.Range{ - Start: params.Position, - End: params.Position, - } - var prefix string - if surrounding != nil { - prefix = surrounding.Prefix() - spn, err := surrounding.Range.Span() - if err != nil { - s.session.Logger().Infof(ctx, "failed to get span for surrounding position: %s:%v:%v: %v", uri, int(params.Position.Line), int(params.Position.Character), err) - } else { - rng, err := m.Range(spn) - if err != nil { - s.session.Logger().Infof(ctx, "failed to convert surrounding position: %s:%v:%v: %v", uri, int(params.Position.Line), int(params.Position.Character), err) - } else { - insertionRng = rng - } - } - } return &protocol.CompletionList{ IsIncomplete: false, - Items: toProtocolCompletionItems(candidates, prefix, insertionRng, s.insertTextFormat, s.usePlaceholders, s.useDeepCompletions), + Items: s.toProtocolCompletionItems(ctx, view, m, candidates, params.Position, surrounding), }, nil } @@ -66,41 +47,54 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara // 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 { +func (s *Server) toProtocolCompletionItems(ctx context.Context, view source.View, m *protocol.ColumnMapper, candidates []source.CompletionItem, pos protocol.Position, surrounding *source.Selection) []protocol.CompletionItem { // Sort the candidates by score, since that is not supported by LSP yet. sort.SliceStable(candidates, func(i, j int) bool { return candidates[i].Score > candidates[j].Score }) + // We might need to adjust the position to account for the prefix. + insertionRange := protocol.Range{ + Start: pos, + End: pos, + } + var prefix string + if surrounding != nil { + prefix = strings.ToLower(surrounding.Prefix()) + spn, err := surrounding.Range.Span() + if err != nil { + s.session.Logger().Infof(ctx, "failed to get span for surrounding position: %s:%v:%v: %v", m.URI, int(pos.Line), int(pos.Character), err) + } else { + rng, err := m.Range(spn) + if err != nil { + s.session.Logger().Infof(ctx, "failed to convert surrounding position: %s:%v:%v: %v", m.URI, int(pos.Line), int(pos.Character), err) + } else { + insertionRange = rng + } + } + } - // Matching against the prefix should be case insensitive. - prefix = strings.ToLower(prefix) + var numDeepCompletionsSeen int - var ( - items = make([]protocol.CompletionItem, 0, len(candidates)) - numDeepCompletionsSeen int - ) + items := make([]protocol.CompletionItem, 0, len(candidates)) for i, candidate := range candidates { // Match against the label (case-insensitive). if !strings.HasPrefix(strings.ToLower(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 { + if !s.useDeepCompletions { continue } - if numDeepCompletionsSeen >= maxDeepCompletions { continue } numDeepCompletionsSeen++ } - insertText := candidate.InsertText - if insertTextFormat == protocol.SnippetTextFormat { - insertText = candidate.Snippet(usePlaceholders) + if s.insertTextFormat == protocol.SnippetTextFormat { + insertText = candidate.Snippet(s.usePlaceholders) } item := protocol.CompletionItem{ Label: candidate.Label, @@ -108,15 +102,16 @@ func toProtocolCompletionItems(candidates []source.CompletionItem, prefix string Kind: toProtocolCompletionItemKind(candidate.Kind), TextEdit: &protocol.TextEdit{ NewText: insertText, - Range: rng, + Range: insertionRange, }, - InsertTextFormat: insertTextFormat, + InsertTextFormat: s.insertTextFormat, // This is a hack so that the client sorts completion results in the order // according to their score. This can be removed upon the resolution of // https://github.com/Microsoft/language-server-protocol/issues/348. - SortText: fmt.Sprintf("%05d", i), - FilterText: candidate.InsertText, - Preselect: i == 0, + SortText: fmt.Sprintf("%05d", i), + FilterText: candidate.InsertText, + Preselect: i == 0, + Documentation: candidate.Documentation, } // Trigger signature help for any function or method completion. // This is helpful even if a function does not have parameters, diff --git a/internal/lsp/general.go b/internal/lsp/general.go index eb280631dc1..65ba6ea33d2 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -64,7 +64,6 @@ func (s *Server) initialize(ctx context.Context, params *protocol.InitializePara return nil, err } } - return &protocol.InitializeResult{ Capabilities: protocol.ServerCapabilities{ CodeActionProvider: true, @@ -192,6 +191,10 @@ func (s *Server) processConfig(ctx context.Context, view source.View, config int } view.SetBuildFlags(flags) } + // Check if the user wants documentation in completion items. + if wantCompletionDocumentation, ok := c["wantCompletionDocumentation"].(bool); ok { + s.wantCompletionDocumentation = wantCompletionDocumentation + } // Check if placeholders are enabled. if usePlaceholders, ok := c["usePlaceholders"].(bool); ok { s.usePlaceholders = usePlaceholders diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 2478d73771e..1e31cbf7150 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -73,6 +73,7 @@ type Server struct { usePlaceholders bool hoverKind source.HoverKind useDeepCompletions bool + wantCompletionDocumentation bool insertTextFormat protocol.InsertTextFormat configurationSupported bool dynamicConfigurationSupported bool @@ -164,8 +165,8 @@ func (s *Server) Completion(ctx context.Context, params *protocol.CompletionPara return s.completion(ctx, params) } -func (s *Server) CompletionResolve(context.Context, *protocol.CompletionItem) (*protocol.CompletionItem, error) { - return nil, notImplemented("CompletionResolve") +func (s *Server) Resolve(ctx context.Context, item *protocol.CompletionItem) (*protocol.CompletionItem, error) { + return nil, notImplemented("completionItem/resolve") } func (s *Server) Hover(ctx context.Context, params *protocol.TextDocumentPositionParams) (*protocol.Hover, error) { @@ -260,10 +261,6 @@ func (s *Server) PrepareRename(context.Context, *protocol.TextDocumentPositionPa return nil, notImplemented("PrepareRename") } -func (s *Server) Resolve(context.Context, *protocol.CompletionItem) (*protocol.CompletionItem, error) { - return nil, notImplemented("Resolve") -} - func (s *Server) SetTraceNotification(context.Context, *protocol.SetTraceParams) error { return notImplemented("SetTraceNotification") } diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 33980eb0c4e..e5e7d6a9d1a 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -65,6 +65,9 @@ type CompletionItem struct { // foo(${1:a int}, ${2: b int}, ${3: c int}) // placeholderSnippet *snippet.Builder + + // Documentation is the documentation for the completion item. + Documentation string } // Snippet is a convenience function that determines the snippet that should be @@ -115,6 +118,7 @@ type completer struct { types *types.Package info *types.Info qf types.Qualifier + opts CompletionOptions // view is the View associated with this completion request. view View @@ -206,9 +210,9 @@ func (c *completer) setSurrounding(ident *ast.Ident) { // 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) { +func (c *completer) found(obj types.Object, score float64) error { if obj.Pkg() != nil && obj.Pkg() != c.types && !obj.Exported() { - return // inaccessible + return fmt.Errorf("%s is inaccessible from %s", obj.Name(), c.types.Path()) } if c.inDeepCompletion() { @@ -217,13 +221,13 @@ func (c *completer) found(obj types.Object, score float64) { // "bar.Baz" even though "Baz" is represented the same types.Object in both. for _, seenObj := range c.deepState.chain { if seenObj == obj { - return + return nil } } } else { // At the top level, dedupe by object. if c.seen[obj] { - return + return nil } c.seen[obj] = true } @@ -239,10 +243,14 @@ func (c *completer) found(obj types.Object, score float64) { // 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)) + item, err := c.item(cand) + if err != nil { + return err + } + c.items = append(c.items, item) c.deepSearch(obj) + return nil } // candidate represents a completion candidate. @@ -259,7 +267,8 @@ type candidate struct { } type CompletionOptions struct { - DeepComplete bool + DeepComplete bool + WantDocumentaton bool } // Completion returns a list of possible candidates for completion, given a @@ -310,6 +319,7 @@ func Completion(ctx context.Context, view View, f GoFile, pos token.Pos, opts Co seen: make(map[types.Object]bool), enclosingFunction: enclosingFunction(path, pos, pkg.GetTypesInfo()), enclosingCompositeLiteral: clInfo, + opts: opts, } c.deepState.enabled = opts.DeepComplete diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index b9718f785eb..1b771427d4b 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -14,10 +14,11 @@ import ( "strings" "golang.org/x/tools/internal/lsp/snippet" + "golang.org/x/tools/internal/span" ) // formatCompletion creates a completion item for a given candidate. -func (c *completer) item(cand candidate) CompletionItem { +func (c *completer) item(cand candidate) (CompletionItem, error) { obj := cand.obj // Handle builtin types separately. @@ -83,8 +84,7 @@ func (c *completer) item(cand candidate) CompletionItem { } detail = strings.TrimPrefix(detail, "untyped ") - - return CompletionItem{ + item := CompletionItem{ Label: label, InsertText: insert, Detail: detail, @@ -94,6 +94,35 @@ func (c *completer) item(cand candidate) CompletionItem { plainSnippet: plainSnippet, placeholderSnippet: placeholderSnippet, } + if c.opts.WantDocumentaton { + declRange, err := objToRange(c.ctx, c.view.Session().Cache().FileSet(), obj) + if err != nil { + return CompletionItem{}, err + } + pos := declRange.FileSet.Position(declRange.Start) + if !pos.IsValid() { + return CompletionItem{}, fmt.Errorf("invalid declaration position for %v", item.Label) + } + uri := span.FileURI(pos.Filename) + f, err := c.view.GetFile(c.ctx, uri) + if err != nil { + return CompletionItem{}, err + } + gof, ok := f.(GoFile) + if !ok { + return CompletionItem{}, fmt.Errorf("declaration for %s not in a Go file: %s", item.Label, uri) + } + ident, err := Identifier(c.ctx, c.view, gof, declRange.Start) + if err != nil { + return CompletionItem{}, err + } + documentation, err := ident.Documentation(c.ctx, SynopsisDocumentation) + if err != nil { + return CompletionItem{}, err + } + item.Documentation = documentation + } + return item, nil } // isParameter returns true if the given *types.Var is a parameter @@ -110,7 +139,7 @@ func (c *completer) isParameter(v *types.Var) bool { return false } -func (c *completer) formatBuiltin(cand candidate) CompletionItem { +func (c *completer) formatBuiltin(cand candidate) (CompletionItem, error) { obj := cand.obj item := CompletionItem{ Label: obj.Name(), @@ -140,7 +169,7 @@ func (c *completer) formatBuiltin(cand candidate) CompletionItem { case *types.Nil: item.Kind = VariableCompletionItem } - return item + return item, nil } var replacer = strings.NewReplacer( diff --git a/internal/lsp/source/hover.go b/internal/lsp/source/hover.go index db412ef55a6..9fdfe1b980a 100644 --- a/internal/lsp/source/hover.go +++ b/internal/lsp/source/hover.go @@ -40,7 +40,7 @@ func (i *IdentifierInfo) Hover(ctx context.Context, markdownSupported bool, hove return "", err } var b strings.Builder - if comment := formatDocumentation(hoverKind, h.comment); comment != "" { + if comment := formatDocumentation(h.comment, hoverKind); comment != "" { b.WriteString(comment) b.WriteRune('\n') } @@ -61,7 +61,7 @@ func (i *IdentifierInfo) Hover(ctx context.Context, markdownSupported bool, hove return b.String(), nil } -func formatDocumentation(hoverKind HoverKind, c *ast.CommentGroup) string { +func formatDocumentation(c *ast.CommentGroup, hoverKind HoverKind) string { switch hoverKind { case SynopsisDocumentation: return doc.Synopsis((c.Text())) @@ -71,6 +71,14 @@ func formatDocumentation(hoverKind HoverKind, c *ast.CommentGroup) string { return "" } +func (i *IdentifierInfo) Documentation(ctx context.Context, hoverKind HoverKind) (string, error) { + h, err := i.decl.hover(ctx) + if err != nil { + return "", err + } + return formatDocumentation(h.comment, hoverKind), nil +} + func (d declaration) hover(ctx context.Context) (*documentation, error) { ctx, ts := trace.StartSpan(ctx, "source.hover") defer ts.End() diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go index c17b8f591f4..2d68b7cbee8 100644 --- a/internal/lsp/source/signature_help.go +++ b/internal/lsp/source/signature_help.go @@ -156,7 +156,7 @@ func signatureInformation(name string, comment *ast.CommentGroup, params, result return &SignatureInformation{ Label: label, // TODO: Should we have the HoverKind apply to signature information as well? - Documentation: formatDocumentation(SynopsisDocumentation, comment), + Documentation: formatDocumentation(comment, SynopsisDocumentation), Parameters: paramInfo, ActiveParameter: activeParam, }