x/tools/gopls: hover: doc link markup ignores import mapping of current file, omits links

[nikos-atwise]

Is your feature request related to a problem? Please describe.
When hovering over a symbol in the editor, currently some go doc links do not render as links in the tooltip.

If the link is to some URL, then it will appear correctly. If the link is to a fully-qualified symbol eg [encoding/json.Marshal] then it will also appear correctly. However links to symbols imported in the current package but not fully-qualified will not appear as links.

Describe the solution you'd like
For builtins or standard library symbols that are accessible, they should link to their pkg.go.dev pages.

For symbols imported in the current package that are not rooted in the same module as the symbol being hovered on, they should also link to their pkg.go.dev pages. (Of course, they may not actually exist on pkg.go.dev, but I think this is good enough, and I believe GoLand does the same thing - it's been a while since I've used it however.)

For symbols imported in the current package that are rooted in the same module as the symbol being hovered on, I think the tooltip should change to showing the documentation for that symbol, since the full comment text should be available in the current workspace.

Describe alternatives you've considered
None

Additional context

https://tip.golang.org/doc/comment#doclinks describes the rules for links.

Given the code sample:

package sample

import "encoding/json"

// This is a sample go doc comment.
//   - [A regular link] works fine.
//   - So does a fully-qualified link to a builtin like [encoding/json.Marshal].
//   - And even non-builtins like [github.com/vmihailenco/msgpack/v5.Marshal].
//
// However...
//   - A link to a symbol accessible through an alias does not: [json.Marshal]
//   - Neither does a link to a symbol in the current package: [Bar]
//
// [A regular link]: http://example.com
type Foo struct{}
type Bar struct{}

func (Foo) F() {
	json.Marshal(nil)
}

This tooltip is shown when I hover over Foo in the editor:

[adonovan]

A related bug: clicking on a doc link such as [fmt.Println] only works if the package fmt is imported under its usual name; for a package like "path" that is usually imported under the name pkgpath, the doc link must read [pkgpath.Join], which is not what a user would ever write in a doc comment.

(Fixed by https://go.dev/cl/612045.)

[gopherbot]

Change https://go.dev/cl/612045 mentions this issue: gopls/internal/golang: Definitions: support renaming imports in doc links

[adonovan]

The problem here is that golang.CommentToMarkdown does not provide the doc.Parser with the necessary hooks. This little hack demonstrates what the implementation must do:

func CommentToMarkdown(text string, options *settings.Options) string {
	var p comment.Parser
+	p.LookupPackage = func(name string) (importPath string, ok bool) {
+		if name == "json" {
+			return "encoding/json", true
+		}
+		return "", false
+	}
+	p.LookupSym = func(recv, name string) (ok bool) {
+		return name == "Marshal"
+	}

We already have an implementation of the necessary hooks, in PackageDocHTML (pkgdoc.go). However the parsing part needs access to a syntax package (*cache.Package), so some factoring is required to plumb the necessary information.

[mvdan]

(If you were replying to my now-deleted comment, my bad, I actually meant to reply to a different but related issue: #69379 (comment))

[adonovan]

In fact my comment explains the work needed to address this issue. No worries though.

[adonovan]

I thought this might be an easy fix, but it's not. Some thoughts:

1. The doc.Parser needs context from the caller: it needs a mapping from PkgName.Name to PkgName.Imported.Path for each non-blank import in the particular file whose comment it is marking up. (This mapping can be derived from types.Info or from the metadata graph.) It also needs the PackagePath of the package containing the doc comment.
2. The doc parser may be called on fragments of doc comment from a wide variety of places in both the Hover logic and in PackageDocHTML. In each case, we need to record the import mapping for the file from which the doc comment came.
3. The HoverJSON would need to plumb all this context information, which is kind of a sign that it has failed as an abstraction.
4. It is not clear what syntax the Synopsis and FullDocumentation fields of HoverJSON use. They are (usually) produced by doc.Package.Synopsis, which returns rendered "Text", but they are (usually) consumed by CommentToMarkdown, which consumes raw doc comment form.

[gopherbot]

Change https://go.dev/cl/623637 mentions this issue: gopls/internal/golang: use correct imports in HTML pkg doc links

[mvdan]

@adonovan you might have meant to leave this issue open with your "Updates #X" line, but since you wrote "attempt to fix #X", it still got caught as a "fixes" commit :)

[adonovan]

Indeed--good catch!

[mvdan]

I have fallen for this silly regular expression a number of times before, you're definitely not alone.