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>

internal/lsp/completion.go

@@ -32,33 +32,14 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara
 	}
 	candidates, surrounding, err := source.Completion(ctx, view, f, rng.Start, source.CompletionOptions{
 		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.
+	var numDeepCompletionsSeen int
-	prefix = strings.ToLower(prefix)
 
-	var (
+	items := make([]protocol.CompletionItem, 0, len(candidates))
-		items                  = make([]protocol.CompletionItem, 0, len(candidates))
-		numDeepCompletionsSeen int
-	)
 	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 {
+		if s.insertTextFormat == protocol.SnippetTextFormat {
-			insertText = candidate.Snippet(usePlaceholders)
+			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,
+			Documentation: candidate.Documentation,
 		}
 		// Trigger signature help for any function or method completion.
 		// This is helpful even if a function does not have parameters,

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

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) {
+func (s *Server) Resolve(ctx context.Context, item *protocol.CompletionItem) (*protocol.CompletionItem, error) {
-	return nil, notImplemented("CompletionResolve")
+	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")
 }

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))
-
+	item, err := c.item(cand)
-	c.items = append(c.items, c.item(cand))
+	if err != nil {
+		return err
+	}
+	c.items = append(c.items, item)
 
 	c.deepSearch(obj)
+	return nil
 }
 
 // candidate represents a completion candidate.
@@ -260,6 +268,7 @@ type candidate struct {
 
 type CompletionOptions struct {
 	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

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 ")
-
+	item := CompletionItem{
-	return 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(

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()

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,
 	}