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 <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

gopls/internal/lsp/source/completion/format.go

@@ -19,7 +19,6 @@ import (
 	"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 @@ Suffixes:
 	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 {

gopls/internal/lsp/source/hover.go

@@ -499,6 +499,51 @@ func objectString(obj types.Object, qf types.Qualifier, inferred *types.Signatur
 	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.
 //

gopls/internal/lsp/source/signature_help.go

@@ -97,17 +97,12 @@ FindCall:
 		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"
 	}

gopls/internal/lsp/source/util.go

@@ -100,22 +100,6 @@ func posToMappedRange(ctx context.Context, snapshot Snapshot, pkg Package, pos,
 	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