x/tools/gopls: render doc links properly in hover

[ian-h-chamberlain]

gopls version

Build info
----------
golang.org/x/tools/gopls v0.11.0
    golang.org/x/tools/gopls@v0.11.0 h1:/nvKHdTtePQmrv9XN3gIUN9MOdUrKzO/dcqgbG6x8EY=
    github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
    github.com/google/go-cmp@v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/exp@v0.0.0-20221031165847-c99f073a8326 h1:QfTh0HpN6hlw6D3vu8DAwC8pBIwikq0AI1evdm+FksE=
    golang.org/x/exp/typeparams@v0.0.0-20221031165847-c99f073a8326 h1:fl8k2zg28yA23264d82M4dp+YlJ3ngDcpuB1bewkQi4=
    golang.org/x/mod@v0.7.0 h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA=
    golang.org/x/sync@v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=
    golang.org/x/sys@v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A=
    golang.org/x/text@v0.4.0 h1:BrVqGRd7+k1DiOgtnFvAkoQEWQvBc25ouMJM6429SFg=
    golang.org/x/tools@v0.3.1-0.20221213193459-ca17b2c27ca8 h1:7/HkGkN/2ktghBCSRRgp31wAww4syfsW52tj7yirjWk=
    golang.org/x/vuln@v0.0.0-20221109205719-3af8368ee4fe h1:qptQiQwEpETwDiz85LKtChqif9xhVkAm8Nhxs0xnTww=
    honnef.co/go/tools@v0.3.3 h1:oDx7VAwstgpYpb3wv0oxiZlxY+foCpRAwY7Vk6XpAgA=
    mvdan.cc/gofumpt@v0.4.0 h1:JVf4NN1mIpHogBj7ABpgOyZc65/UUOkKQFkoURsz4MM=
    mvdan.cc/xurls/v2@v2.4.0 h1:tzxjVAj+wSBmDcF6zBB7/myTy3gX9xvi8Tyr28AuQgc=
go: go1.19.1

go env

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/ichamberlain/Library/Caches/go-build"
GOENV="/Users/ichamberlain/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/ichamberlain/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/ichamberlain/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org"
GOROOT="/usr/local/Cellar/go/1.19.5/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.19.5/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.19.5"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/ichamberlain/Documents/go-sandbox/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/wb/7n13y1cx0v1d28zb304bw8bm0000gn/T/go-build1610262310=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

The new go1.19 doc comments are rendered to markdown, as mentioned in #54260, but the implementation seems to be incomplete to me. Mostly, linking to anything (package-local or otherwise) seems broken, and markdown links don't render as expected.

I believe all of these formats should be supported per https://go.dev/doc/comment#links

Sample program:

package main

import (
	"fmt"
)

// Struct is a struct.
//
// # Unexpected link behavior
//
//   - [fmt.Println] is linked to, but the link doesn't work (leads to non-existent "fmt")
//   - [fmt] same as above
//   - [Struct] isn't linked to
//   - [absolute markdown links] don't seem to work
//   - [relative markdown links] don't seem to work, although I'm
//     not entirely sure what the expected behavior here is
//
// [absolute markdown links]: https://example.com
// [relative markdown links]: #Example
//
//nolint:ignored
type Struct struct{}

func main() {
	fmt.Println()
}

What did you expect to see?

I would expect a hover comment to render something like this (using GitHub to render the expected markdown):

Struct is a struct.

Unexpected link behavior

• fmt.Println is linked to, but the link doesn't work (leads to non-existent "fmt")
• fmt same as above
• Struct isn't linked to
• absolute markdown links don't seem to work
• relative markdown links don't seem to work, although I'm
  not entirely sure what the expected behavior here is

Whether fmt.Println should link to pkg.go.dev or be a "go to definition" link is up for debate, but maybe making it configurable would be nice?

For local links like Struct I think the same question applies but I would lean towards "go to definition", it seems there has been some tangentially related discussion in #44890.

What did you see instead?

Clicking the fmt links leads to this:

Editor and settings

Version: 1.75.0
Commit: e2816fe719a4026ffa1ee0189dc89bdfdbafb164
Date: 2023-02-01T15:24:42.903Z
Electron: 19.1.9
Chromium: 102.0.5005.194
Node.js: 16.14.2
V8: 10.2.154.23-electron.0
OS: Darwin x64 21.6.0
Sandboxed: No

Relevant config:

{
    "go.buildOnSave": "off",
    "go.formatTool": "gofumpt",
    "go.lintOnSave": "file",
    "go.terminal.activateEnvironment": false,
    "go.toolsManagement.autoUpdate": true,
    "go.vetOnSave": "off",
    "go.testExplorer.enable": true,
    "go.useLanguageServer": true,
    "go.inlayHints.assignVariableTypes": false,
    "go.inlayHints.compositeLiteralFields": true,
    "go.inlayHints.compositeLiteralTypes": true,
    "go.inlayHints.constantValues": true,
    "go.inlayHints.functionTypeParameters": true,
    "go.inlayHints.parameterNames": true,
    "go.inlayHints.rangeVariableTypes": false,
    "gopls": {
        "ui.completion.usePlaceholders": true,
        "ui.navigation.importShortcut": "Definition",
        "ui.semanticTokens": true,
        "noSemanticString": true,
        "noSemanticNumber": true,
    },
}

Logs

N/A, I think — but I can capture some if necessary.

[findleyr]

CC @pjweinb

Thank you for the report. I agree we should fix those links.

[pjweinb]

There are several different things going on in the example, some of which we can fix.

1. The reason [absolute markdown links] doesn't work is that the block containing its definition is not recognized as a link definition block, because #Example is not recognized as a URL. Removing that line produces links for [absolute markdown links]
2. We can generate pkg.go.dev links for fmt.Println and fmt.
3. go doc does not seem to generate any kind of link for [Block]. What link might be generated, remembering that links are to URLs? ('go to definition' isn't implemented by a URL)

I don't think it matters, but vscode does not implement the same version of markdown as GitHub.

[gopherbot]

Change https://go.dev/cl/469715 mentions this issue: gopls/hove: add documentation links to hover

[ian-h-chamberlain]

1. The reason [absolute markdown links] doesn't work is that the block containing its definition is not recognized as a link definition block, because #Example is not recognized as a URL. Removing that line produces links for [absolute markdown links]

Is this expected behavior? I would think it should be possible to link to another anchor on the same page in the generated docs for https://pkg.go.dev (for example, something in fmt might link to #hdr-Printing or #example-Stringer)? I guess maybe this deserves a separate issue for go doc and/or pkgsite, since that behavior would determine how gopls should behave?

3. go doc does not seem to generate any kind of link for [Block]. What link might be generated, remembering that links are to URLs? ('go to definition' isn't implemented by a URL)

This is a good point — in the context of the editor, "go to definition" would be convenient, but linking to https://pkg.go.dev for the current package would probably also work for packages that have docs published there. I would expect that https://pkg.go.dev normally generates these as URL fragments like mentioned above, but in the editor that wouldn't work as expected.

I looked into how another language server I use implemented "go to definition" for this particular use case, and it looks like they use a nonstandard LSP extension to provide the client with a clickable "go to definition" command 😞. So I guess short of implementing something like that, just using https://pkg.go.dev is probably the most reasonable option.

[pjweinb]

It is expected behavior. from https://tip.golang.org/doc/comment A span of unindented non-blank lines defines link targets when every line is of the form “[Text]: URL”.

…

On Tue, Feb 21, 2023 at 9:59 AM Ian Chamberlain ***@***.***> wrote: 1. The reason [absolute markdown links] doesn't work is that the block containing its definition is not recognized as a link definition block, because #Example is not recognized as a URL. Removing that line produces links for [absolute markdown links] Is this expected behavior? I would think it should be possible to link to another anchor on the same page in the generated docs for https://pkg.go.dev (for example, something in fmt might link to #hdr-Printing or #example-Stringer)? I guess maybe this deserves a separate issue for go doc and/or pkgsite, since that behavior would determine how gopls should behave? 1. go doc does not seem to generate any kind of link for [Block]. What link might be generated, remembering that links are to URLs? ('go to definition' isn't implemented by a URL) This is a good point — in the context of the editor, "go to definition" would be convenient, but linking to https://pkg.go.dev for the current package would probably also work for packages that have docs published there. I would expect that https://pkg.go.dev normally generates these as URL fragments like mentioned above, but in the editor that wouldn't work as expected. I looked into how another language server I use implemented "go to definition" for this particular use case, and it looks like they use a nonstandard LSP extension to provide the client with a clickable "go to definition" command 😞. So I guess short of implementing something like that, just using https://pkg.go.dev is probably the most reasonable option. — Reply to this email directly, view it on GitHub <#58352 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJIAI4IV6MKVUZJOVV4J23WYTJ53ANCNFSM6AAAAAAUS4GCMM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[ian-h-chamberlain]

It is expected behavior. from tip.golang.org/doc/comment A span of unindented non-blank lines defines link targets when every line is of the form “[Text]: URL”.

Hmm, I guess more precisely what I meant was: is it expected that #Example is not recognized as a URL?

I guess technically it is a URI but not a URL (according to RFCs 3986 and 1738), but I think common usage would indicate that fragments can be considered as URLs (in fact, in the docs you linked the href attribute is used as an example of the ways these URLs can be used. <a href="#frag"> is a fairly common usage of this attribute).

All that to say — if fragments like this are never intended to be supported, then perhaps the documentation should be updated to explicitly indicate this (something like relative URLs without a scheme are not supported, or just an explicit mention of what definition is being used for "URL").

Or, if these kinds of links should be supported, then perhaps the behavior of go doc and gopls should be adjusted to support them.