Skip to content

Commit 411d040

Browse files
committed
internal/lsp/source: clean up the interface to hover information
Clean up the relationship between HoverContext and HoverJSON so that HoverContext is more clearly responsible for collecting comment and signature nodes, and HoverJSON is more clearly a DTO for the hover RPC. Change-Id: Ib32d4151a53505d227b4225be4f87754a542e980 Reviewed-on: https://go-review.googlesource.com/c/tools/+/385017 Trust: Robert Findley <[email protected]> Run-TryBot: Robert Findley <[email protected]> Reviewed-by: Heschi Kreinick <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent 45aeaf7 commit 411d040

File tree

3 files changed

+37
-38
lines changed

3 files changed

+37
-38
lines changed

internal/lsp/source/completion/format.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"context"
1010
"fmt"
1111
"go/ast"
12+
"go/doc"
1213
"go/types"
1314
"strings"
1415

@@ -251,12 +252,13 @@ Suffixes:
251252
event.Error(ctx, "failed to find Hover", err, tag.URI.Of(uri))
252253
return item, nil
253254
}
254-
item.Documentation = hover.Synopsis
255255
if c.opts.fullDocumentation {
256-
item.Documentation = hover.FullDocumentation
256+
item.Documentation = hover.Comment.Text()
257+
} else {
258+
item.Documentation = doc.Synopsis(hover.Comment.Text())
257259
}
258260
// The desired pattern is `^// Deprecated`, but the prefix has been removed
259-
if strings.HasPrefix(hover.FullDocumentation, "Deprecated") {
261+
if strings.HasPrefix(hover.Comment.Text(), "Deprecated") {
260262
if c.snapshot.View().Options().CompletionTags {
261263
item.Tags = []protocol.CompletionItemTag{protocol.ComplDeprecated}
262264
} else if c.snapshot.View().Options().CompletionDeprecated {

internal/lsp/source/hover.go

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -30,43 +30,41 @@ import (
3030
// of a given node, for use in various summaries (hover, autocomplete,
3131
// signature help).
3232
type HoverContext struct {
33-
// Synopsis is a single sentence synopsis of the symbol's documentation.
34-
Synopsis string `json:"synopsis"`
35-
36-
// FullDocumentation is the symbol's full documentation.
37-
FullDocumentation string `json:"fullDocumentation"`
38-
3933
// signatureSource is the object or node use to derive the hover signature.
4034
//
4135
// TODO(rfindley): can we pre-compute the signature, to avoid this indirection?
4236
signatureSource interface{}
4337

4438
// comment is the most relevant comment group associated with the hovered object.
45-
comment *ast.CommentGroup
39+
Comment *ast.CommentGroup
4640
}
4741

4842
// HoverJSON contains information used by hover. It is also the JSON returned
4943
// for the "structured" hover format
5044
type HoverJSON struct {
45+
// Synopsis is a single sentence synopsis of the symbol's documentation.
46+
Synopsis string `json:"synopsis"`
47+
48+
// FullDocumentation is the symbol's full documentation.
49+
FullDocumentation string `json:"fullDocumentation"`
50+
5151
// Signature is the symbol's signature.
5252
Signature string `json:"signature"`
5353

5454
// SingleLine is a single line describing the symbol.
5555
// This is recommended only for use in clients that show a single line for hover.
5656
SingleLine string `json:"singleLine"`
5757

58+
// SymbolName is the types.Object.Name for the given symbol.
59+
SymbolName string `json:"symbolName"`
60+
5861
// LinkPath is the pkg.go.dev link for the given symbol.
5962
// For example, the "go/ast" part of "pkg.go.dev/go/ast#Node".
6063
LinkPath string `json:"linkPath"`
6164

6265
// LinkAnchor is the pkg.go.dev link anchor for the given symbol.
6366
// For example, the "Node" part of "pkg.go.dev/go/ast#Node".
6467
LinkAnchor string `json:"linkAnchor"`
65-
66-
// symbolName is the types.Object.Name for the given symbol.
67-
symbolName string
68-
69-
HoverContext
7068
}
7169

7270
func Hover(ctx context.Context, snapshot Snapshot, fh FileHandle, position protocol.Position) (*protocol.Hover, error) {
@@ -251,15 +249,19 @@ func HoverIdentifier(ctx context.Context, i *IdentifierInfo) (*HoverJSON, error)
251249
ctx, done := event.Start(ctx, "source.Hover")
252250
defer done()
253251

254-
fset := i.Snapshot.FileSet()
255252
hoverCtx, err := FindHoverContext(ctx, i.Snapshot, i.pkg, i.Declaration.obj, i.Declaration.node, i.Declaration.fullDecl)
256253
if err != nil {
257254
return nil, err
258255
}
259256

260-
h := &HoverJSON{HoverContext: *hoverCtx}
257+
h := &HoverJSON{
258+
FullDocumentation: hoverCtx.Comment.Text(),
259+
Synopsis: doc.Synopsis(hoverCtx.Comment.Text()),
260+
}
261+
262+
fset := i.Snapshot.FileSet()
261263
// Determine the symbol's signature.
262-
switch x := h.signatureSource.(type) {
264+
switch x := hoverCtx.signatureSource.(type) {
263265
case *ast.TypeSpec:
264266
x2 := *x
265267
// Don't duplicate comments when formatting type specs.
@@ -316,7 +318,7 @@ func HoverIdentifier(ctx context.Context, i *IdentifierInfo) (*HoverJSON, error)
316318
}
317319
}
318320

319-
h.symbolName, h.LinkPath, h.LinkAnchor = linkData(obj, i.enclosing)
321+
h.SymbolName, h.LinkPath, h.LinkAnchor = linkData(obj, i.enclosing)
320322

321323
// See golang/go#36998: don't link to modules matching GOPRIVATE.
322324
//
@@ -492,7 +494,7 @@ func FindHoverContext(ctx context.Context, s Snapshot, pkg Package, obj types.Ob
492494
// The package declaration.
493495
for _, f := range pkg.GetSyntax() {
494496
if f.Name == pkgNode {
495-
info = &HoverContext{comment: f.Doc}
497+
info = &HoverContext{Comment: f.Doc}
496498
}
497499
}
498500
case *ast.ImportSpec:
@@ -506,7 +508,7 @@ func FindHoverContext(ctx context.Context, s Snapshot, pkg Package, obj types.Ob
506508
// so pick the first file that has a doc comment.
507509
for _, file := range imp.GetSyntax() {
508510
if file.Doc != nil {
509-
info = &HoverContext{signatureSource: obj, comment: file.Doc}
511+
info = &HoverContext{signatureSource: obj, Comment: file.Doc}
510512
break
511513
}
512514
}
@@ -567,9 +569,9 @@ func FindHoverContext(ctx context.Context, s Snapshot, pkg Package, obj types.Ob
567569
case *ast.FuncDecl:
568570
switch obj.(type) {
569571
case *types.Func:
570-
info = &HoverContext{signatureSource: obj, comment: node.Doc}
572+
info = &HoverContext{signatureSource: obj, Comment: node.Doc}
571573
case *types.Builtin:
572-
info = &HoverContext{signatureSource: node.Type, comment: node.Doc}
574+
info = &HoverContext{signatureSource: node.Type, Comment: node.Doc}
573575
case *types.Var:
574576
// Object is a function param or the field of an anonymous struct
575577
// declared with ':='. Skip the first one because only fields
@@ -588,7 +590,7 @@ func FindHoverContext(ctx context.Context, s Snapshot, pkg Package, obj types.Ob
588590
if comment.Text() == "" {
589591
comment = field.Comment
590592
}
591-
info = &HoverContext{signatureSource: obj, comment: comment}
593+
info = &HoverContext{signatureSource: obj, Comment: comment}
592594
}
593595
}
594596
}
@@ -597,11 +599,6 @@ func FindHoverContext(ctx context.Context, s Snapshot, pkg Package, obj types.Ob
597599
info = &HoverContext{signatureSource: obj}
598600
}
599601

600-
if info.comment != nil {
601-
info.FullDocumentation = info.comment.Text()
602-
info.Synopsis = doc.Synopsis(info.FullDocumentation)
603-
}
604-
605602
return info, nil
606603
}
607604

@@ -642,9 +639,9 @@ func formatGenDecl(node *ast.GenDecl, spec ast.Spec, fullPos token.Pos, obj type
642639
case *ast.TypeSpec:
643640
return formatTypeSpec(spec, node), nil
644641
case *ast.ValueSpec:
645-
return &HoverContext{signatureSource: spec, comment: spec.Doc}, nil
642+
return &HoverContext{signatureSource: spec, Comment: spec.Doc}, nil
646643
case *ast.ImportSpec:
647-
return &HoverContext{signatureSource: spec, comment: spec.Doc}, nil
644+
return &HoverContext{signatureSource: spec, Comment: spec.Doc}, nil
648645
}
649646
return nil, errors.Errorf("unable to format spec %v (%T)", spec, spec)
650647
}
@@ -659,7 +656,7 @@ func formatTypeSpec(spec *ast.TypeSpec, decl *ast.GenDecl) *HoverContext {
659656
}
660657
return &HoverContext{
661658
signatureSource: spec,
662-
comment: comment,
659+
Comment: comment,
663660
}
664661
}
665662

@@ -691,18 +688,18 @@ func formatVar(node ast.Spec, fullPos token.Pos, obj types.Object, decl *ast.Gen
691688
// associated values so that we can augment their hover with more information.
692689
if _, ok := obj.(*types.Var); ok && spec.Type == nil && len(spec.Values) > 0 {
693690
if _, ok := spec.Values[0].(*ast.BasicLit); ok {
694-
return &HoverContext{signatureSource: spec, comment: comment}
691+
return &HoverContext{signatureSource: spec, Comment: comment}
695692
}
696693
}
697694

698-
return &HoverContext{signatureSource: obj, comment: comment}
695+
return &HoverContext{signatureSource: obj, Comment: comment}
699696
}
700697

701698
if fieldList != nil {
702699
comment := findFieldComment(fullPos, fieldList)
703-
return &HoverContext{signatureSource: obj, comment: comment}
700+
return &HoverContext{signatureSource: obj, Comment: comment}
704701
}
705-
return &HoverContext{signatureSource: obj, comment: decl.Doc}
702+
return &HoverContext{signatureSource: obj, Comment: decl.Doc}
706703
}
707704

708705
// extractFieldList recursively tries to extract a field list.
@@ -806,7 +803,7 @@ func formatLink(h *HoverJSON, options *Options) string {
806803
plainLink := BuildLink(options.LinkTarget, h.LinkPath, h.LinkAnchor)
807804
switch options.PreferredContentFormat {
808805
case protocol.Markdown:
809-
return fmt.Sprintf("[`%s` on %s](%s)", h.symbolName, options.LinkTarget, plainLink)
806+
return fmt.Sprintf("[`%s` on %s](%s)", h.SymbolName, options.LinkTarget, plainLink)
810807
case protocol.PlainText:
811808
return ""
812809
default:

internal/lsp/source/signature_help.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ FindCall:
120120
return nil, 0, err
121121
}
122122
name = obj.Name()
123-
comment = d.comment
123+
comment = d.Comment
124124
} else {
125125
name = "func"
126126
}

0 commit comments

Comments
 (0)