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 <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go
index ecd3968..d1103ff 100644
--- a/internal/lsp/completion.go
+++ b/internal/lsp/completion.go
@@ -31,34 +31,15 @@
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 @@
// 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 @@
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 eb28063..65ba6ea 100644
--- a/internal/lsp/general.go
+++ b/internal/lsp/general.go
@@ -64,7 +64,6 @@
return nil, err
}
}
-
return &protocol.InitializeResult{
Capabilities: protocol.ServerCapabilities{
CodeActionProvider: true,
@@ -192,6 +191,10 @@
}
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 2478d73..1e31cbf 100644
--- a/internal/lsp/server.go
+++ b/internal/lsp/server.go
@@ -73,6 +73,7 @@
usePlaceholders bool
hoverKind source.HoverKind
useDeepCompletions bool
+ wantCompletionDocumentation bool
insertTextFormat protocol.InsertTextFormat
configurationSupported bool
dynamicConfigurationSupported bool
@@ -164,8 +165,8 @@
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 @@
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 33980eb..e5e7d6a 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -65,6 +65,9 @@
// 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 @@
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 @@
// 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 @@
// "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 @@
// 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 CompletionOptions struct {
- DeepComplete bool
+ DeepComplete bool
+ WantDocumentaton bool
}
// Completion returns a list of possible candidates for completion, given a
@@ -310,6 +319,7 @@
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 b9718f7..1b77142 100644
--- a/internal/lsp/source/completion_format.go
+++ b/internal/lsp/source/completion_format.go
@@ -14,10 +14,11 @@
"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 @@
}
detail = strings.TrimPrefix(detail, "untyped ")
-
- return CompletionItem{
+ item := CompletionItem{
Label: label,
InsertText: insert,
Detail: detail,
@@ -94,6 +94,35 @@
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 @@
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 @@
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 db412ef..9fdfe1b 100644
--- a/internal/lsp/source/hover.go
+++ b/internal/lsp/source/hover.go
@@ -40,7 +40,7 @@
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 @@
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 @@
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 c17b8f5..2d68b7c 100644
--- a/internal/lsp/source/signature_help.go
+++ b/internal/lsp/source/signature_help.go
@@ -156,7 +156,7 @@
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,
}
