gopls/internal/lsp/source: simplify extracting object hover doc
Add a new helper HoverDocForObject, which finds the relevant object
documentation without needing to type-check. This eliminates the last
use of FindPackageFromPos.
For golang/go#57987
Change-Id: Ic9deec78d68156e9ead3831a8247f8c30259a3c6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/464455
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/gopls/internal/lsp/source/completion/format.go b/gopls/internal/lsp/source/completion/format.go
index dfa61a0..738879b 100644
--- a/gopls/internal/lsp/source/completion/format.go
+++ b/gopls/internal/lsp/source/completion/format.go
@@ -19,7 +19,6 @@
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/span"
"golang.org/x/tools/internal/event"
- "golang.org/x/tools/internal/event/tag"
"golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/typeparams"
)
@@ -242,27 +241,21 @@
if !pos.IsValid() {
return item, nil
}
- uri := span.URIFromPath(pos.Filename)
- // Find the source file of the candidate.
- pkg, err := source.FindPackageFromPos(ctx, c.snapshot, c.pkg, obj.Pos())
+ comment, err := source.HoverDocForObject(ctx, c.snapshot, c.pkg, obj)
if err != nil {
- return item, nil
- }
-
- decl, _, _ := source.FindDeclInfo(pkg.GetSyntax(), obj.Pos()) // may be nil
- hover, err := source.FindHoverContext(ctx, c.snapshot, pkg, obj, decl, nil)
- if err != nil {
- event.Error(ctx, "failed to find Hover", err, tag.URI.Of(uri))
+ event.Error(ctx, fmt.Sprintf("failed to find Hover for %q", obj.Name()), err)
return item, nil
}
if c.opts.fullDocumentation {
- item.Documentation = hover.Comment.Text()
+ item.Documentation = comment.Text()
} else {
- item.Documentation = doc.Synopsis(hover.Comment.Text())
+ item.Documentation = doc.Synopsis(comment.Text())
}
// The desired pattern is `^// Deprecated`, but the prefix has been removed
- if strings.HasPrefix(hover.Comment.Text(), "Deprecated") {
+ // TODO(rfindley): It doesn't look like this does the right thing for
+ // multi-line comments.
+ if strings.HasPrefix(comment.Text(), "Deprecated") {
if c.snapshot.View().Options().CompletionTags {
item.Tags = []protocol.CompletionItemTag{protocol.ComplDeprecated}
} else if c.snapshot.View().Options().CompletionDeprecated {
diff --git a/gopls/internal/lsp/source/hover.go b/gopls/internal/lsp/source/hover.go
index 1d48152..85d66de 100644
--- a/gopls/internal/lsp/source/hover.go
+++ b/gopls/internal/lsp/source/hover.go
@@ -499,6 +499,51 @@
return str
}
+// HoverDocForObject returns the best doc comment for obj, referenced by srcpkg.
+//
+// TODO(rfindley): there appears to be zero(!) tests for this functionality.
+func HoverDocForObject(ctx context.Context, snapshot Snapshot, srcpkg Package, obj types.Object) (*ast.CommentGroup, error) {
+ if _, isTypeName := obj.(*types.TypeName); isTypeName {
+ if _, isTypeParam := obj.Type().(*typeparams.TypeParam); isTypeParam {
+ return nil, nil
+ }
+ }
+
+ pgf, pos, err := parseFull(ctx, snapshot, srcpkg, obj.Pos())
+ if err != nil {
+ return nil, fmt.Errorf("re-parsing: %v", err)
+ }
+
+ decl, spec, field := FindDeclInfo([]*ast.File{pgf.File}, pos)
+ if field != nil && field.Doc != nil {
+ return field.Doc, nil
+ }
+ switch decl := decl.(type) {
+ case *ast.FuncDecl:
+ return decl.Doc, nil
+ case *ast.GenDecl:
+ switch spec := spec.(type) {
+ case *ast.ValueSpec:
+ if spec.Doc != nil {
+ return spec.Doc, nil
+ }
+ if decl.Doc != nil {
+ return decl.Doc, nil
+ }
+ return spec.Comment, nil
+ case *ast.TypeSpec:
+ if spec.Doc != nil {
+ return spec.Doc, nil
+ }
+ if decl.Doc != nil {
+ return decl.Doc, nil
+ }
+ return spec.Comment, nil
+ }
+ }
+ return nil, nil
+}
+
// parseFull fully parses the file corresponding to position pos, referenced
// from the given srcpkg.
//
diff --git a/gopls/internal/lsp/source/signature_help.go b/gopls/internal/lsp/source/signature_help.go
index 17549b8..0f81b85 100644
--- a/gopls/internal/lsp/source/signature_help.go
+++ b/gopls/internal/lsp/source/signature_help.go
@@ -97,17 +97,12 @@
comment *ast.CommentGroup
)
if obj != nil {
- declPkg, err := FindPackageFromPos(ctx, snapshot, pkg, obj.Pos())
- if err != nil {
- return nil, 0, err
- }
- node, _, _ := FindDeclInfo(declPkg.GetSyntax(), obj.Pos()) // may be nil
- d, err := FindHoverContext(ctx, snapshot, pkg, obj, node, nil)
+ d, err := HoverDocForObject(ctx, snapshot, pkg, obj)
if err != nil {
return nil, 0, err
}
name = obj.Name()
- comment = d.Comment
+ comment = d
} else {
name = "func"
}
diff --git a/gopls/internal/lsp/source/util.go b/gopls/internal/lsp/source/util.go
index 6a8dc26..8340c5e 100644
--- a/gopls/internal/lsp/source/util.go
+++ b/gopls/internal/lsp/source/util.go
@@ -100,22 +100,6 @@
return pgf.PosMappedRange(pos, end)
}
-// FindPackageFromPos returns the Package for the given position, which must be
-// among the transitive dependencies of pkg.
-//
-// TODO(rfindley): is this the best factoring of this API? This function is
-// really a trivial wrapper around findFileInDeps, which may be a more useful
-// function to expose.
-func FindPackageFromPos(ctx context.Context, snapshot Snapshot, pkg Package, pos token.Pos) (Package, error) {
- if !pos.IsValid() {
- return nil, fmt.Errorf("invalid position")
- }
- fileName := pkg.FileSet().File(pos).Name()
- uri := span.URIFromPath(fileName)
- _, pkg, err := findFileInDeps(ctx, snapshot, pkg, uri)
- return pkg, err
-}
-
// Matches cgo generated comment as well as the proposed standard:
//
// https://golang.org/s/generatedcode
