internal/lsp: add configuration for hover levels

Instead of defaulting to a one sentence synopsis for documentation on
hover, allow the user to configure the amount of documentation they want
to see. Right now, the options are none, some (using go/doc.Synopsis),
or all. We should add a 4th, single-line, mode, which will allow clients
like vim-go to stop stripping off documentation on hover.

Updates golang/go#32561

Change-Id: I529242da84b794636984d5ef2918b7252886f0ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/184797
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>

internal/lsp/general.go

@@ -34,6 +34,9 @@ func (s *Server) initialize(ctx context.Context, params *protocol.InitializePara
 		}
 	}
 
+	// Default to using synopsis as a default for hover information.
+	s.hoverKind = source.SynopsisDocumentation
+
 	s.supportedCodeActions = map[protocol.CodeActionKind]bool{
 		protocol.SourceOrganizeImports: true,
 		protocol.QuickFix:              true,
@@ -145,7 +148,7 @@ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedPa
 			if err != nil {
 				return err
 			}
-			if err := s.processConfig(view, config[0]); err != nil {
+			if err := s.processConfig(ctx, view, config[0]); err != nil {
 				return err
 			}
 		}
@@ -156,7 +159,7 @@ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedPa
 	return nil
 }
 
-func (s *Server) processConfig(view source.View, config interface{}) error {
+func (s *Server) processConfig(ctx context.Context, view source.View, config interface{}) error {
 	// TODO: We should probably store and process more of the config.
 	if config == nil {
 		return nil // ignore error if you don't have a config
@@ -193,9 +196,19 @@ func (s *Server) processConfig(view source.View, config interface{}) error {
 	if usePlaceholders, ok := c["usePlaceholders"].(bool); ok {
 		s.usePlaceholders = usePlaceholders
 	}
-	// Check if the user has disabled documentation on hover.
-	if noDocsOnHover, ok := c["noDocsOnHover"].(bool); ok {
-		s.noDocsOnHover = noDocsOnHover
+	// Set the hover kind.
+	if hoverKind, ok := c["hoverKind"].(string); ok {
+		switch hoverKind {
+		case "NoDocumentation":
+			s.hoverKind = source.NoDocumentation
+		case "SynopsisDocumentation":
+			s.hoverKind = source.SynopsisDocumentation
+		case "FullDocumentation":
+			s.hoverKind = source.FullDocumentation
+		default:
+			view.Session().Logger().Errorf(ctx, "unsupported hover kind %s", hoverKind)
+			// The default value is already be set to synopsis.
+		}
 	}
 	// Check if the user wants to see suggested fixes from go/analysis.
 	if wantSuggestedFixes, ok := c["wantSuggestedFixes"].(bool); ok {

internal/lsp/hover.go

@@ -32,7 +32,7 @@ func (s *Server) hover(ctx context.Context, params *protocol.TextDocumentPositio
 	if err != nil {
 		return nil, err
 	}
-	hover, err := ident.Hover(ctx, s.preferredContentFormat == protocol.Markdown, !s.noDocsOnHover)
+	hover, err := ident.Hover(ctx, s.preferredContentFormat == protocol.Markdown, s.hoverKind)
 	if err != nil {
 		return nil, err
 	}

internal/lsp/lsp_test.go

@@ -55,6 +55,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 				protocol.SourceOrganizeImports: true,
 				protocol.QuickFix:              true,
 			},
+			hoverKind: source.SynopsisDocumentation,
 		},
 		data: data,
 	}

internal/lsp/server.go

@@ -71,7 +71,7 @@ type Server struct {
 	// Configurations.
 	// TODO(rstambler): Separate these into their own struct?
 	usePlaceholders               bool
-	noDocsOnHover                 bool
+	hoverKind                     source.HoverKind
 	useDeepCompletions            bool
 	insertTextFormat              protocol.InsertTextFormat
 	configurationSupported        bool

internal/lsp/source/hover.go

@@ -10,7 +10,6 @@ import (
 	"go/ast"
 	"go/doc"
 	"go/format"
-	"go/token"
 	"go/types"
 	"strings"
 )
@@ -20,17 +19,52 @@ type documentation struct {
 	comment *ast.CommentGroup
 }
 
-func (i *IdentifierInfo) Hover(ctx context.Context, markdownSupported, wantComments bool) (string, error) {
+type HoverKind int
+
+const (
+	NoDocumentation = HoverKind(iota)
+	SynopsisDocumentation
+	FullDocumentation
+
+	// TODO: Support a single-line hover mode for clients like Vim.
+	singleLine
+)
+
+func (i *IdentifierInfo) Hover(ctx context.Context, markdownSupported bool, hoverKind HoverKind) (string, error) {
 	h, err := i.decl.hover(ctx)
 	if err != nil {
 		return "", err
 	}
-	c := h.comment
-	if !wantComments {
-		c = nil
-	}
 	var b strings.Builder
-	return writeHover(h.source, i.File.FileSet(), &b, c, markdownSupported, i.qf)
+	if comment := formatDocumentation(hoverKind, h.comment); comment != "" {
+		b.WriteString(comment)
+		b.WriteRune('\n')
+	}
+	if markdownSupported {
+		b.WriteString("```go\n")
+	}
+	switch x := h.source.(type) {
+	case ast.Node:
+		if err := format.Node(&b, i.File.FileSet(), x); err != nil {
+			return "", err
+		}
+	case types.Object:
+		b.WriteString(types.ObjectString(x, i.qf))
+	}
+	if markdownSupported {
+		b.WriteString("\n```")
+	}
+	return b.String(), nil
+}
+
+func formatDocumentation(hoverKind HoverKind, c *ast.CommentGroup) string {
+	switch hoverKind {
+	case SynopsisDocumentation:
+		return doc.Synopsis((c.Text()))
+	case FullDocumentation:
+		return c.Text()
+	}
+	return ""
 }
 
 func (d declaration) hover(ctx context.Context) (*documentation, error) {
@@ -125,34 +159,3 @@ func formatVar(node ast.Spec, obj types.Object) (*documentation, error) {
 	// If we weren't able to find documentation for the object.
 	return &documentation{obj, nil}, nil
 }
-
-// writeHover writes the hover for a given node and its documentation.
-func writeHover(x interface{}, fset *token.FileSet, b *strings.Builder, c *ast.CommentGroup, markdownSupported bool, qf types.Qualifier) (string, error) {
-	if c != nil {
-		// TODO(rstambler): Improve conversion from Go docs to markdown.
-		b.WriteString(formatDocumentation(c))
-		b.WriteRune('\n')
-	}
-	if markdownSupported {
-		b.WriteString("```go\n")
-	}
-	switch x := x.(type) {
-	case ast.Node:
-		if err := format.Node(b, fset, x); err != nil {
-			return "", err
-		}
-	case types.Object:
-		b.WriteString(types.ObjectString(x, qf))
-	}
-	if markdownSupported {
-		b.WriteString("\n```")
-	}
-	return b.String(), nil
-}
-
-func formatDocumentation(c *ast.CommentGroup) string {
-	if c == nil {
-		return ""
-	}
-	return doc.Synopsis(c.Text())
-}

internal/lsp/source/signature_help.go

@@ -155,8 +155,9 @@ func signatureInformation(name string, comment *ast.CommentGroup, params, result
 		label += " " + detail
 	}
 	return &SignatureInformation{
-		Label:           label,
-		Documentation:   formatDocumentation(comment),
+		Label: label,
+		// TODO: Should we have the HoverKind apply to signature information as well?
+		Documentation:   formatDocumentation(SynopsisDocumentation, comment),
 		Parameters:      paramInfo,
 		ActiveParameter: activeParam,
 	}

internal/lsp/source/source_test.go

@@ -376,7 +376,7 @@ func (r *runner) Definition(t *testing.T, data tests.Definitions) {
 		if err != nil {
 			t.Fatalf("failed for %v: %v", d.Src, err)
 		}
-		hover, err := ident.Hover(ctx, false, true)
+		hover, err := ident.Hover(ctx, false, source.SynopsisDocumentation)
 		if err != nil {
 			t.Fatalf("failed for %v: %v", d.Src, err)
 		}