internal/lsp: support an experimental structured hover format

Updates golang/go#33352

Change-Id: Ibf18e2529c9ba8c94c66942ea6f2c27f047ed285
Reviewed-on: https://go-review.googlesource.com/c/tools/+/189977
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/internal/lsp/general.go b/internal/lsp/general.go
index 207aa8e..97c90c5 100644
--- a/internal/lsp/general.go
+++ b/internal/lsp/general.go
@@ -41,7 +41,7 @@
 	}
 
 	// Default to using synopsis as a default for hover information.
-	s.hoverKind = source.SynopsisDocumentation
+	s.hoverKind = synopsisDocumentation
 
 	s.supportedCodeActions = map[source.FileKind]map[protocol.CodeActionKind]bool{
 		source.Go: {
@@ -230,13 +230,15 @@
 	if hoverKind, ok := c["hoverKind"].(string); ok {
 		switch hoverKind {
 		case "NoDocumentation":
-			s.hoverKind = source.NoDocumentation
+			s.hoverKind = noDocumentation
 		case "SingleLine":
-			s.hoverKind = source.SingleLine
+			s.hoverKind = singleLine
 		case "SynopsisDocumentation":
-			s.hoverKind = source.SynopsisDocumentation
+			s.hoverKind = synopsisDocumentation
 		case "FullDocumentation":
-			s.hoverKind = source.FullDocumentation
+			s.hoverKind = fullDocumentation
+		case "Structured":
+			s.hoverKind = structured
 		default:
 			log.Error(ctx, "unsupported hover kind", nil, tag.Of("HoverKind", hoverKind))
 			// The default value is already be set to synopsis.
diff --git a/internal/lsp/hover.go b/internal/lsp/hover.go
index e6147e3..a27455c 100644
--- a/internal/lsp/hover.go
+++ b/internal/lsp/hover.go
@@ -6,12 +6,31 @@
 
 import (
 	"context"
+	"encoding/json"
+	"fmt"
 
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
+	"golang.org/x/tools/internal/lsp/telemetry/log"
 	"golang.org/x/tools/internal/span"
 )
 
+type hoverKind int
+
+const (
+	singleLine = hoverKind(iota)
+	noDocumentation
+	synopsisDocumentation
+	fullDocumentation
+
+	// structured is an experimental setting that returns a structured hover format.
+	// This format separates the signature from the documentation, so that the client
+	// can do more manipulation of these fields.
+	//
+	// This should only be used by clients that support this behavior.
+	structured
+)
+
 func (s *Server) hover(ctx context.Context, params *protocol.TextDocumentPositionParams) (*protocol.Hover, error) {
 	uri := span.NewURI(params.TextDocument.URI)
 	view := s.session.ViewOf(uri)
@@ -31,7 +50,7 @@
 	if err != nil {
 		return nil, nil
 	}
-	hover, err := ident.Hover(ctx, s.preferredContentFormat == protocol.Markdown, s.hoverKind)
+	hover, err := ident.Hover(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -43,11 +62,44 @@
 	if err != nil {
 		return nil, err
 	}
+	contents := s.toProtocolHoverContents(ctx, hover)
 	return &protocol.Hover{
-		Contents: protocol.MarkupContent{
-			Kind:  s.preferredContentFormat,
-			Value: hover,
-		},
-		Range: &rng,
+		Contents: contents,
+		Range:    &rng,
 	}, nil
 }
+
+func (s *Server) toProtocolHoverContents(ctx context.Context, h *source.HoverInformation) protocol.MarkupContent {
+	content := protocol.MarkupContent{
+		Kind: s.preferredContentFormat,
+	}
+	signature := h.Signature
+	if content.Kind == protocol.Markdown {
+		signature = fmt.Sprintf("```go\n%s\n```", h.Signature)
+	}
+	switch s.hoverKind {
+	case singleLine:
+		content.Value = h.SingleLine
+	case noDocumentation:
+		content.Value = signature
+	case synopsisDocumentation:
+		if h.Synopsis != "" {
+			content.Value = fmt.Sprintf("%s\n%s", h.Synopsis, signature)
+		} else {
+			content.Value = signature
+		}
+	case fullDocumentation:
+		if h.FullDocumentation != "" {
+			content.Value = fmt.Sprintf("%s\n%s", signature, h.FullDocumentation)
+		} else {
+			content.Value = signature
+		}
+	case structured:
+		b, err := json.Marshal(h)
+		if err != nil {
+			log.Error(ctx, "failed to marshal structured hover", err)
+		}
+		content.Value = string(b)
+	}
+	return content
+}
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index df629f6..800c5e9 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -59,7 +59,7 @@
 				},
 				source.Mod: {},
 				source.Sum: {}},
-			hoverKind: source.SynopsisDocumentation,
+			hoverKind: synopsisDocumentation,
 		},
 		data: data,
 		ctx:  ctx,
diff --git a/internal/lsp/server.go b/internal/lsp/server.go
index 6ab758b..5bcc844 100644
--- a/internal/lsp/server.go
+++ b/internal/lsp/server.go
@@ -79,7 +79,7 @@
 	// Configurations.
 	// TODO(rstambler): Separate these into their own struct?
 	usePlaceholders               bool
-	hoverKind                     source.HoverKind
+	hoverKind                     hoverKind
 	useDeepCompletions            bool
 	wantCompletionDocumentation   bool
 	insertTextFormat              protocol.InsertTextFormat
diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go
index b724c77..7186959 100644
--- a/internal/lsp/source/completion_format.go
+++ b/internal/lsp/source/completion_format.go
@@ -132,11 +132,11 @@
 		if err != nil {
 			goto Return
 		}
-		documentation, err := ident.Documentation(c.ctx, SynopsisDocumentation)
+		hover, err := ident.Hover(c.ctx)
 		if err != nil {
 			goto Return
 		}
-		item.Documentation = documentation
+		item.Documentation = hover.Synopsis
 	}
 Return:
 	return item, nil
diff --git a/internal/lsp/source/hover.go b/internal/lsp/source/hover.go
index d0e6841..e7f3250 100644
--- a/internal/lsp/source/hover.go
+++ b/internal/lsp/source/hover.go
@@ -16,92 +16,62 @@
 	errors "golang.org/x/xerrors"
 )
 
-type documentation struct {
+type HoverInformation struct {
+	// Signature is the symbol's signature.
+	Signature string `json:"signature"`
+
+	// SingleLine is a single line describing the symbol.
+	// This is recommended only for use in clients that show a single line for hover.
+	SingleLine string `json:"singleLine"`
+
+	// Synopsis is a single sentence synopsis of the symbol's documentation.
+	Synopsis string `json:"synopsis"`
+
+	// FullDocumentation is the symbol's full documentation.
+	FullDocumentation string `json:"fullDocumentation"`
+
 	source  interface{}
 	comment *ast.CommentGroup
 }
 
-type HoverKind int
-
-const (
-	NoDocumentation = HoverKind(iota)
-	SingleLine
-	SynopsisDocumentation
-	FullDocumentation
-)
-
-func (i *IdentifierInfo) Hover(ctx context.Context, markdownSupported bool, hoverKind HoverKind) (string, error) {
+func (i *IdentifierInfo) Hover(ctx context.Context) (*HoverInformation, error) {
 	ctx, done := trace.StartSpan(ctx, "source.Hover")
 	defer done()
 
-	// If the user has explicitly requested a single line of hover information,
-	// fall back to using types.ObjectString.
-	if hoverKind == SingleLine {
-		return types.ObjectString(i.decl.obj, i.qf), nil
-	}
-
 	h, err := i.decl.hover(ctx)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
-	var b strings.Builder
-
-	// Add documentation to the top if the HoverKind is anything other than full documentation.
-	if hoverKind != FullDocumentation {
-		if comment := formatDocumentation(h.comment, hoverKind); comment != "" {
-			b.WriteString(comment)
-			b.WriteRune('\n')
-		}
-	}
-	if markdownSupported {
-		b.WriteString("```go\n")
-	}
+	// Determine the symbol's signature.
 	switch x := h.source.(type) {
 	case ast.Node:
+		var b strings.Builder
 		if err := format.Node(&b, i.File.FileSet(), x); err != nil {
-			return "", err
+			return nil, err
 		}
+		h.Signature = b.String()
 	case types.Object:
-		b.WriteString(types.ObjectString(x, i.qf))
+		h.Signature = types.ObjectString(x, i.qf)
 	}
-	if markdownSupported {
-		b.WriteString("\n```")
+
+	// Set the documentation.
+	if i.decl.obj != nil {
+		h.SingleLine = types.ObjectString(i.decl.obj, i.qf)
 	}
-	// Add documentation to the bottom if the HoverKind is full documentation.
-	if hoverKind == FullDocumentation {
-		if comment := formatDocumentation(h.comment, hoverKind); comment != "" {
-			b.WriteRune('\n')
-			b.WriteString(comment)
-		}
+	if h.comment != nil {
+		h.FullDocumentation = h.comment.Text()
+		h.Synopsis = doc.Synopsis(h.FullDocumentation)
 	}
-	return b.String(), nil
+	return h, nil
 }
 
-func formatDocumentation(c *ast.CommentGroup, hoverKind HoverKind) string {
-	switch hoverKind {
-	case SynopsisDocumentation:
-		return doc.Synopsis((c.Text()))
-	case FullDocumentation:
-		return c.Text()
-	}
-	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) {
+func (d declaration) hover(ctx context.Context) (*HoverInformation, error) {
 	ctx, done := trace.StartSpan(ctx, "source.hover")
 	defer done()
 	obj := d.obj
 	switch node := d.node.(type) {
 	case *ast.ImportSpec:
-		return &documentation{node, nil}, nil
+		return &HoverInformation{source: node}, nil
 	case *ast.GenDecl:
 		switch obj := obj.(type) {
 		case *types.TypeName, *types.Var, *types.Const, *types.Func:
@@ -110,22 +80,22 @@
 	case *ast.TypeSpec:
 		if obj.Parent() == types.Universe {
 			if obj.Name() == "error" {
-				return &documentation{node, nil}, nil
+				return &HoverInformation{source: node}, nil
 			}
-			return &documentation{node.Name, nil}, nil // comments not needed for builtins
+			return &HoverInformation{source: node.Name}, nil // comments not needed for builtins
 		}
 	case *ast.FuncDecl:
 		switch obj.(type) {
 		case *types.Func:
-			return &documentation{obj, node.Doc}, nil
+			return &HoverInformation{source: obj, comment: node.Doc}, nil
 		case *types.Builtin:
-			return &documentation{node.Type, node.Doc}, nil
+			return &HoverInformation{source: node.Type, comment: node.Doc}, nil
 		}
 	}
-	return &documentation{obj, nil}, nil
+	return &HoverInformation{source: obj}, nil
 }
 
-func formatGenDecl(node *ast.GenDecl, obj types.Object, typ types.Type) (*documentation, error) {
+func formatGenDecl(node *ast.GenDecl, obj types.Object, typ types.Type) (*HoverInformation, error) {
 	if _, ok := typ.(*types.Named); ok {
 		switch typ.Underlying().(type) {
 		case *types.Interface, *types.Struct:
@@ -152,19 +122,19 @@
 	case *ast.TypeSpec:
 		if len(node.Specs) > 1 {
 			// If multiple types are declared in the same block.
-			return &documentation{spec.Type, spec.Doc}, nil
+			return &HoverInformation{source: spec.Type, comment: spec.Doc}, nil
 		} else {
-			return &documentation{spec, node.Doc}, nil
+			return &HoverInformation{source: spec, comment: node.Doc}, nil
 		}
 	case *ast.ValueSpec:
-		return &documentation{spec, spec.Doc}, nil
+		return &HoverInformation{source: spec, comment: spec.Doc}, nil
 	case *ast.ImportSpec:
-		return &documentation{spec, spec.Doc}, nil
+		return &HoverInformation{source: spec, comment: spec.Doc}, nil
 	}
 	return nil, errors.Errorf("unable to format spec %v (%T)", spec, spec)
 }
 
-func formatVar(node ast.Spec, obj types.Object) (*documentation, error) {
+func formatVar(node ast.Spec, obj types.Object) (*HoverInformation, error) {
 	var fieldList *ast.FieldList
 	if spec, ok := node.(*ast.TypeSpec); ok {
 		switch t := spec.Type.(type) {
@@ -181,13 +151,13 @@
 			field := fieldList.List[i]
 			if field.Pos() <= obj.Pos() && obj.Pos() <= field.End() {
 				if field.Doc.Text() != "" {
-					return &documentation{obj, field.Doc}, nil
+					return &HoverInformation{source: obj, comment: field.Doc}, nil
 				} else if field.Comment.Text() != "" {
-					return &documentation{obj, field.Comment}, nil
+					return &HoverInformation{source: obj, comment: field.Comment}, nil
 				}
 			}
 		}
 	}
 	// If we weren't able to find documentation for the object.
-	return &documentation{obj, nil}, nil
+	return &HoverInformation{source: obj}, nil
 }
diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go
index ffbb3d5..615e21b 100644
--- a/internal/lsp/source/signature_help.go
+++ b/internal/lsp/source/signature_help.go
@@ -7,6 +7,7 @@
 import (
 	"context"
 	"go/ast"
+	"go/doc"
 	"go/token"
 	"go/types"
 
@@ -154,10 +155,13 @@
 		paramInfo = append(paramInfo, ParameterInformation{Label: p})
 	}
 	label := name + formatFunction(params, results, writeResultParens)
+	var c string
+	if comment != nil {
+		c = doc.Synopsis(comment.Text())
+	}
 	return &SignatureInformation{
-		Label: label,
-		// TODO: Should we have the HoverKind apply to signature information as well?
-		Documentation:   formatDocumentation(comment, SynopsisDocumentation),
+		Label:           label,
+		Documentation:   c,
 		Parameters:      paramInfo,
 		ActiveParameter: activeParam,
 	}
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index b6ce7d5..b54684d 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -405,10 +405,15 @@
 		if err != nil {
 			t.Fatalf("failed for %v: %v", d.Src, err)
 		}
-		hover, err := ident.Hover(ctx, false, source.SynopsisDocumentation)
+		h, err := ident.Hover(ctx)
 		if err != nil {
 			t.Fatalf("failed for %v: %v", d.Src, err)
 		}
+		var hover string
+		if h.Synopsis != "" {
+			hover += h.Synopsis + "\n"
+		}
+		hover += h.Signature
 		rng := ident.DeclarationRange()
 		if d.IsType {
 			rng = ident.Type.Range