From 89a01ca1a8d2f8fa299a2cd5c2a568e8a0819b49 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Mon, 12 Aug 2019 16:59:23 -0400 Subject: [PATCH] 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 Reviewed-by: Ian Cottrell --- internal/lsp/general.go | 12 ++- internal/lsp/hover.go | 64 ++++++++++-- internal/lsp/lsp_test.go | 2 +- internal/lsp/server.go | 2 +- internal/lsp/source/completion_format.go | 4 +- internal/lsp/source/hover.go | 120 +++++++++-------------- internal/lsp/source/signature_help.go | 10 +- internal/lsp/source/source_test.go | 7 +- 8 files changed, 127 insertions(+), 94 deletions(-) diff --git a/internal/lsp/general.go b/internal/lsp/general.go index 207aa8e668..97c90c5b98 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -41,7 +41,7 @@ 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.hoverKind = synopsisDocumentation s.supportedCodeActions = map[source.FileKind]map[protocol.CodeActionKind]bool{ source.Go: { @@ -230,13 +230,15 @@ func (s *Server) processConfig(ctx context.Context, view source.View, config int 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 e6147e3791..a27455c8e3 100644 --- a/internal/lsp/hover.go +++ b/internal/lsp/hover.go @@ -6,12 +6,31 @@ package lsp 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 @@ func (s *Server) hover(ctx context.Context, params *protocol.TextDocumentPositio 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 @@ func (s *Server) hover(ctx context.Context, params *protocol.TextDocumentPositio 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 df629f6ace..800c5e905f 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -59,7 +59,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { }, 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 6ab758b6c0..5bcc844f0d 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -79,7 +79,7 @@ type Server struct { // 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 b724c77585..71869595a3 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -132,11 +132,11 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { 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 d0e6841f38..e7f3250701 100644 --- a/internal/lsp/source/hover.go +++ b/internal/lsp/source/hover.go @@ -16,92 +16,62 @@ import ( 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 - } - 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") + return nil, err } + // 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 @@ func (d declaration) hover(ctx context.Context) (*documentation, error) { 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 @@ func formatGenDecl(node *ast.GenDecl, obj types.Object, typ types.Type) (*docume 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 @@ func formatVar(node ast.Spec, obj types.Object) (*documentation, error) { 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 ffbb3d58b0..615e21b65b 100644 --- a/internal/lsp/source/signature_help.go +++ b/internal/lsp/source/signature_help.go @@ -7,6 +7,7 @@ package source import ( "context" "go/ast" + "go/doc" "go/token" "go/types" @@ -154,10 +155,13 @@ func signatureInformation(name string, comment *ast.CommentGroup, params, result 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 b6ce7d5b4d..b54684dd49 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -405,10 +405,15 @@ 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, 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