gopls/internal/golang: RenderPackageDoc: emit anchors for var/const
This CL causes the package rendering to emit an <a id=...> anchor
for each name defined by a var or const declaration.
Also, choose the initial fragment id (e.g. "#Buffer.Len") based
on the selection.
+ Test that anchors are emitted.
Change-Id: Idb4838a6d2741a26cd9dbb5ad31a76d6f811ff26
Reviewed-on: https://go-review.googlesource.com/c/tools/+/575697
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/gopls/doc/release/v0.16.0.md b/gopls/doc/release/v0.16.0.md
index 0fc67ed..04306e8 100644
--- a/gopls/doc/release/v0.16.0.md
+++ b/gopls/doc/release/v0.16.0.md
@@ -10,11 +10,13 @@
Gopls now offers a "View package documentation" code action that opens
a local web page displaying the generated documentation for the
-current Go package in a form similar to https://pkg.go.dev. Use it to
-preview the marked-up documentation as you prepare API changes, or to
-read the documentation for locally edited packages, even ones that
-have not yet been saved. Reload the page after an edit to see updated
-documentation.
+current Go package in a form similar to https://pkg.go.dev.
+The page will be initially scrolled to the documentation for the
+declaration containing the cursor.
+Use this feature to preview the marked-up documentation as you prepare API
+changes, or to read the documentation for locally edited packages,
+even ones that have not yet been saved. Reload the page after an edit
+to see updated documentation.
TODO: demo in VS Code.
diff --git a/gopls/internal/golang/pkgdoc.go b/gopls/internal/golang/pkgdoc.go
index ff76e4a..b3525f8 100644
--- a/gopls/internal/golang/pkgdoc.go
+++ b/gopls/internal/golang/pkgdoc.go
@@ -20,6 +20,8 @@
// - abbreviate long signatures by replacing parameters 4 onwards with "...".
// - style the <li> bullets in the index as invisible.
// - add push notifications such as didChange -> reload.
+// - there appears to be a maximum file size beyond which the
+// "source.doc" code action is not offered. Remove that.
import (
"bytes"
@@ -254,8 +256,6 @@
emit(n.Pos())
pos = n.End()
if url := linkify(n); url != "" {
- // TODO(adonovan): emit anchors (not hrefs)
- // for package-level defining idents.
fmt.Fprintf(&buf, "<a class='id' href='%s'>%s</a>", url, escape(n.Name))
} else {
buf.WriteString(escape(n.Name)) // plain
@@ -355,6 +355,11 @@
// constants and variables
values := func(vals []*doc.Value) {
for _, v := range vals {
+ // anchors
+ for _, name := range v.Names {
+ fmt.Fprintf(&buf, "<a id='%s'></a>\n", escape(name))
+ }
+
// declaration
decl2 := *v.Decl // shallow copy
decl2.Doc = nil
diff --git a/gopls/internal/server/command.go b/gopls/internal/server/command.go
index 079c795..a6a3cdd 100644
--- a/gopls/internal/server/command.go
+++ b/gopls/internal/server/command.go
@@ -10,6 +10,7 @@
"encoding/json"
"errors"
"fmt"
+ "go/ast"
"io"
"os"
"path/filepath"
@@ -507,16 +508,16 @@
progress: "", // the operation should be fast
forURI: loc.URI,
}, func(ctx context.Context, deps commandDeps) error {
- pkg, _, err := golang.NarrowestPackageForFile(ctx, deps.snapshot, loc.URI)
+ pkg, pgf, err := golang.NarrowestPackageForFile(ctx, deps.snapshot, loc.URI)
if err != nil {
return err
}
// When invoked from a _test.go file, show the
// documentation of the package under test.
- path := pkg.Metadata().PkgPath
+ pkgpath := pkg.Metadata().PkgPath
if pkg.Metadata().ForTest != "" {
- path = pkg.Metadata().ForTest
+ pkgpath = pkg.Metadata().ForTest
}
// Start web server.
@@ -525,12 +526,59 @@
return err
}
+ // Compute fragment (e.g. "#Buffer.Len") based on
+ // enclosing top-level declaration, if exported.
+ var fragment string
+ pos, err := pgf.PositionPos(loc.Range.Start)
+ if err != nil {
+ return err
+ }
+ path, _ := astutil.PathEnclosingInterval(pgf.File, pos, pos)
+ if n := len(path); n > 1 {
+ switch decl := path[n-2].(type) {
+ case *ast.FuncDecl:
+ if decl.Name.IsExported() {
+ // e.g. "#Println"
+ fragment = decl.Name.Name
+
+ // method?
+ if decl.Recv != nil && len(decl.Recv.List) > 0 {
+ recv := decl.Recv.List[0].Type
+ if star, ok := recv.(*ast.StarExpr); ok {
+ recv = star.X // *N -> N
+ }
+ if id, ok := recv.(*ast.Ident); ok && id.IsExported() {
+ // e.g. "#Buffer.Len"
+ fragment = id.Name + "." + fragment
+ } else {
+ fragment = ""
+ }
+ }
+ }
+
+ case *ast.GenDecl:
+ // path=[... Spec? GenDecl File]
+ for _, spec := range decl.Specs {
+ if n > 2 && spec == path[n-3] {
+ var name *ast.Ident
+ switch spec := spec.(type) {
+ case *ast.ValueSpec:
+ // var, const: use first name
+ name = spec.Names[0]
+ case *ast.TypeSpec:
+ name = spec.Name
+ }
+ if name != nil && name.IsExported() {
+ fragment = name.Name
+ }
+ break
+ }
+ }
+ }
+ }
+
// Direct the client to open the /pkg page.
- //
- // TODO(adonovan): compute fragment (e.g. "#fmt.Println") based on loc.Range.
- // (Should it document the selected symbol, or the enclosing decl?)
- fragment := ""
- url := web.pkgURL(deps.snapshot.View(), path, fragment)
+ url := web.pkgURL(deps.snapshot.View(), pkgpath, fragment)
openClientBrowser(ctx, c.s.client, url)
return nil
diff --git a/gopls/internal/test/integration/misc/webserver_test.go b/gopls/internal/test/integration/misc/webserver_test.go
index 20fdba4..66cf52a 100644
--- a/gopls/internal/test/integration/misc/webserver_test.go
+++ b/gopls/internal/test/integration/misc/webserver_test.go
@@ -81,6 +81,8 @@
// that would corrupt the AST while filtering out unexported
// symbols such as b, causing nodeHTML to panic.
// Now it doesn't crash.
+ //
+ // We also check cross-reference anchors for all symbols.
const files = `
-- go.mod --
module example.com
@@ -88,27 +90,52 @@
-- a/a.go --
package a
-var A, b = 0, 0
+// The '1' suffix is to reduce risk of spurious matches with other HTML substrings.
-func ExportedFunc()
-func unexportedFunc()
-type ExportedType int
-type unexportedType int
+var V1, v1 = 0, 0
+const C1, c1 = 0, 0
+
+func F1()
+func f1()
+
+type T1 int
+type t1 int
+
+func (T1) M1() {}
+func (T1) m1() {}
+
+func (t1) M1() {}
+func (t1) m1() {}
`
Run(t, files, func(t *testing.T, env *Env) {
uri1 := viewPkgDoc(t, env, "a/a.go")
- doc1 := get(t, uri1)
+ doc := get(t, uri1)
// (Ideally our code rendering would also
// eliminate unexported symbols...)
- // TODO(adonovan): when we emit var anchors,
- // check that #A exists and #b does not.
- checkMatch(t, true, doc1, "var A, b = .*0.*0")
+ checkMatch(t, true, doc, "var V1, v1 = .*0.*0")
+ checkMatch(t, true, doc, "const C1, c1 = .*0.*0")
// Unexported funcs/types/... must still be discarded.
- checkMatch(t, true, doc1, "ExportedFunc")
- checkMatch(t, false, doc1, "unexportedFunc")
- checkMatch(t, true, doc1, "ExportedType")
- checkMatch(t, false, doc1, "unexportedType")
+ checkMatch(t, true, doc, "F1")
+ checkMatch(t, false, doc, "f1")
+ checkMatch(t, true, doc, "T1")
+ checkMatch(t, false, doc, "t1")
+
+ // Also, check that anchors exist (only) for exported symbols.
+ // exported:
+ checkMatch(t, true, doc, "<a id='V1'")
+ checkMatch(t, true, doc, "<a id='C1'")
+ checkMatch(t, true, doc, "<h3 id='T1'")
+ checkMatch(t, true, doc, "<h3 id='F1'")
+ checkMatch(t, true, doc, "<h4 id='T1.M1'")
+ // unexported:
+ checkMatch(t, false, doc, "<a id='v1'")
+ checkMatch(t, false, doc, "<a id='c1'")
+ checkMatch(t, false, doc, "<h3 id='t1'")
+ checkMatch(t, false, doc, "<h3 id='f1'")
+ checkMatch(t, false, doc, "<h4 id='T1.m1'")
+ checkMatch(t, false, doc, "<h4 id='t1.M1'")
+ checkMatch(t, false, doc, "<h4 id='t1.m1'")
})
}
