From 47024e57924e39055497be0b727f7da4239bd2b6 Mon Sep 17 00:00:00 2001 From: Jonathan Amsterdam Date: Wed, 21 Aug 2024 20:28:41 -0400 Subject: [PATCH] internal/godoc/dochtml/internal/render: unique headings redux This CL takes an alternative approach to generate unique headings. It builds on the work of https://go.dev/cl/573595, which was a fix for https://go.dev/issue/64582. - It takes a more direct approach to constructing a unique string from an ast.Decl. - The function that does that is tested separately, reducing the test cases needed for formatDocHTML. - It saves the generated headings, so the HTML doesn't have to be reparsed. NOTE: This will break all links to headings that are not in the package comment. Happily, such headings are rare: only 40 of the top thousand packages have one. It might seem we could avoid any breakage by only applying a suffix to duplicate headings. But then at any time in the future, a unique heading could become a duplicate, causing a break. Better one break now than an unending stream of them. Change-Id: I379712b54c6bc9c6a9343d0006639085a40e23d9 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/608035 kokoro-CI: kokoro LUCI-TryBot-Result: Go LUCI Reviewed-by: Hyang-Ah Hana Kim --- .../godoc/dochtml/internal/render/linkify.go | 254 ++++++++----- .../dochtml/internal/render/linkify_test.go | 350 ++++++------------ .../godoc/dochtml/internal/render/render.go | 2 - .../render/testdata/formatDocHTML/README.md | 5 +- .../testdata/formatDocHTML/headinglink.txt | 6 +- .../render/testdata/formatDocHTML/links.txt | 24 +- .../testdata/formatDocHTML/unique_func.txt | 25 ++ .../testdata/formatDocHTML/unique_method.txt | 25 ++ .../formatDocHTML/unique_overview.txt | 20 +- .../testdata/formatDocHTML/unique_var.txt | 25 ++ .../testdata/formatDocHTML/unlinked_group.txt | 25 ++ .../godoc/dochtml/testdata/comments.golden | 6 +- 12 files changed, 383 insertions(+), 384 deletions(-) create mode 100644 internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_func.txt create mode 100644 internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_method.txt create mode 100644 internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_var.txt create mode 100644 internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unlinked_group.txt diff --git a/internal/godoc/dochtml/internal/render/linkify.go b/internal/godoc/dochtml/internal/render/linkify.go index c237ceda0..c8b5b8c08 100644 --- a/internal/godoc/dochtml/internal/render/linkify.go +++ b/internal/godoc/dochtml/internal/render/linkify.go @@ -23,7 +23,6 @@ import ( safe "github.com/google/safehtml" "github.com/google/safehtml/legacyconversions" "github.com/google/safehtml/template" - "golang.org/x/net/html" "golang.org/x/pkgsite/internal/log" ) @@ -53,10 +52,7 @@ const ( rfcRx = `RFC\s+(\d{3,5})(,?\s+[Ss]ection\s+(\d+(\.\d+)*))?` ) -var ( - matchRx = regexp.MustCompile(urlRx + `|` + rfcRx) - badAnchorRx = regexp.MustCompile(`[^a-zA-Z0-9]`) -) +var matchRx = regexp.MustCompile(urlRx + `|` + rfcRx) type link struct { Class string @@ -65,7 +61,7 @@ type link struct { } type heading struct { - ID safe.Identifier + ID safe.Identifier // if empty, the title is not linked Title safe.HTML } @@ -73,11 +69,15 @@ var ( // tocTemplate expects a []heading. tocTemplate = template.Must(template.New("toc").Parse(`
    - {{range . -}} + {{- range .}}
  • - {{.Title}} + {{- if .ID.String -}} + {{.Title}} + {{- else -}} + {{.Title}} + {{- end -}}
    • - {{end -}} + {{- end}}
`)) @@ -88,8 +88,13 @@ var ( paraTemplate = template.Must(template.New("para").Parse("

{{.}}\n

")) - headingTemplate = template.Must(template.New("heading").Parse( - `

{{.Title}}

`)) + // expects a heading + headingTemplate = template.Must(template.New("heading").Parse(` + {{- if .ID.String -}} +

{{.Title}}

+ {{- else -}} +

{{.Title}}

+ {{- end}}`)) linkTemplate = template.Must(template.New("link").Parse( `{{.Text}}`)) @@ -119,57 +124,14 @@ func (r *Renderer) formatDocHTML(text string, decl ast.Decl, extractLinks bool) if extractLinks { r.removeLinks(doc) } - - h := r.blocksToHTML(doc.Content, decl, true, extractLinks) - if headings := extractHeadings(h); len(headings) > 0 { - h = safe.HTMLConcat(ExecuteToHTML(tocTemplate, headings), h) + hscope := newHeadingScope(headingIDSuffix(decl)) + h := r.blocksToHTML(doc.Content, true, hscope) + if len(hscope.headings) > 0 { + h = safe.HTMLConcat(ExecuteToHTML(tocTemplate, hscope.headings), h) } - return h } -func extractHeadings(h safe.HTML) []heading { - var headings []heading - - doc, err := html.Parse(strings.NewReader(h.String())) - if err != nil { - return nil - } - - var f func(*html.Node) - f = func(n *html.Node) { - for _, a := range n.Attr { - if a.Key == "id" && strings.HasPrefix(a.Val, "hdr-") { - if tn := firstTextNode(n); tn != nil { - title := strings.TrimSpace(tn.Data) - hdrName := strings.TrimPrefix(a.Val, "hdr-") - id := safe.IdentifierFromConstantPrefix("hdr", hdrName) - headings = append(headings, heading{id, safe.HTMLEscaped(title)}) - } - break - } - } - for c := n.FirstChild; c != nil; c = c.NextSibling { - f(c) - } - } - f(doc) - - return headings -} - -func firstTextNode(n *html.Node) *html.Node { - if n.Type == html.TextNode { - return n - } - for c := n.FirstChild; c != nil; c = c.NextSibling { - if r := firstTextNode(c); r != nil { - return r - } - } - return nil -} - // removeLinks removes the "Links" section from doc. // Pkgsite has a convention where a "Links" heading in a doc comment provides links // that are rendered in a separate place in the UI. @@ -263,13 +225,13 @@ func parseLink(line string) *Link { } } -func (r *Renderer) blocksToHTML(bs []comment.Block, decl ast.Decl, useParagraph, extractLinks bool) safe.HTML { +func (r *Renderer) blocksToHTML(bs []comment.Block, useParagraph bool, hscope *headingScope) safe.HTML { return concatHTML(bs, func(b comment.Block) safe.HTML { - return r.blockToHTML(b, decl, useParagraph, extractLinks) + return r.blockToHTML(b, useParagraph, hscope) }) } -func (r *Renderer) blockToHTML(b comment.Block, decl ast.Decl, useParagraph, extractLinks bool) safe.HTML { +func (r *Renderer) blockToHTML(b comment.Block, useParagraph bool, hscope *headingScope) safe.HTML { switch b := b.(type) { case *comment.Paragraph: th := r.textsToHTML(b.Text) @@ -282,7 +244,7 @@ func (r *Renderer) blockToHTML(b comment.Block, decl ast.Decl, useParagraph, ext return ExecuteToHTML(codeTemplate, b.Text) case *comment.Heading: - return ExecuteToHTML(headingTemplate, r.newHeading(b, decl)) + return ExecuteToHTML(headingTemplate, r.newHeading(b, hscope)) case *comment.List: var items []safe.HTML @@ -291,7 +253,7 @@ func (r *Renderer) blockToHTML(b comment.Block, decl ast.Decl, useParagraph, ext items = append(items, ExecuteToHTML(listItemTemplate, struct { Number string Content safe.HTML - }{item.Number, r.blocksToHTML(item.Content, decl, useParagraph, false)})) + }{item.Number, r.blocksToHTML(item.Content, useParagraph, hscope)})) } t := oListTemplate if b.Items[0].Number == "" { @@ -303,53 +265,143 @@ func (r *Renderer) blockToHTML(b comment.Block, decl ast.Decl, useParagraph, ext } } -// headingIDs keeps track of used heading ids to prevent duplicates. -type headingIDs map[safe.Identifier]bool +/* +We want to convert headings in doc comments to unique and stable HTML IDs. -// headingID returns a unique identifier for a *comment.Heading & ast.Decl pair. -func (r *Renderer) headingID(h *comment.Heading, decl ast.Decl) safe.Identifier { - s := textsToString(h.Text) - hdrTitle := badAnchorRx.ReplaceAllString(s, "_") - id := safe.IdentifierFromConstantPrefix("hdr", hdrTitle) - if !r.headingIDs[id] { - r.headingIDs[id] = true - return id - } +The IDs in an HTML page should be unique, so URLs with fragments can link to +them. The headings in the doc comments of a Go package (which corresponds to an +HTML page for us) need not be unique. The same heading can be used in the doc +comment of several symbols, and in the package comment as well. - // The id is not unique. Attempt to generate an identifier using the decl. - for _, v := range generateAnchorPoints(decl) { - if v.Kind == "field" { - continue - } - hdrTitle = badAnchorRx.ReplaceAllString(v.ID.String()+"_"+s, "_") - if v.Kind == "constant" || v.Kind == "variable" { - if specs := decl.(*ast.GenDecl).Specs; len(specs) > 1 { - // Grouped consts and vars cannot be deterministically identified, - // so treat them as a single section. e.g. "hdr-constant-Title" - hdrTitle = badAnchorRx.ReplaceAllString(v.Kind+"_"+s, "_") - } - } - if v.Kind != "method" { - // Continue iterating until we find a higher precedence kind. - break - } +If uniqueness were the only criterion, we could just append a number. But we +also want the IDs to be stable. The page for the latest version of a package +has no version in the URL, so that a saved link will always reference the latest +version. We do the same for other links on the page, like links to symbols. We +want to do so for heading links too. That means that a heading link, which uses +the ID as a fragment, may visit pages with different content at different times. +We want that saved link to return to the same heading every time, if the heading +still exists. + +If we numbered the IDs from top to bottom, a heading ID would change whenever headings +were added above it. Instead, we try to construct a unique suffix from the declaration that the +doc comment refers to. (The suffix is empty for the package comment.) For ungrouped declarations, +we use the symbol name, which must be unique in the package. For grouped declarations, like + + const ( + A = 1 + B = 2 + ) + +or + + var x, Y int + +there is no good way to pick a suffix, so we don't link headings at all. Evidence suggests +that such headings are very rare. + +After constructing the unique suffix, we may still have duplicate IDs because the same doc comment +can repeat headings. So we append a number that is unique for the given suffix. + +This approach will always produce unique, stable IDs when it produces IDs at all. +*/ + +// A headingScope is a collection of headings that arise from the documentation of +// a single element, either the package itself or a symbol within it. +// A headingScope ensures that heading IDs are unique, and keeps a list of headings as they appear in the scope. +type headingScope struct { + suffix string // ID suffix; unique for the package; empty for package doc + createIDs bool // create IDs for the headings + headings []heading // headings for this scope, in order + ids map[string]int // count of seen IDs +} + +func newHeadingScope(suffix string, createIDs bool) *headingScope { + return &headingScope{ + suffix: badAnchorRx.ReplaceAllString(suffix, "_"), + createIDs: createIDs, + ids: map[string]int{}, } +} - for { - id = safe.IdentifierFromConstantPrefix("hdr", hdrTitle) - if !r.headingIDs[id] { - r.headingIDs[id] = true - break - } - // The id is still not unique. Append _ until unique. - hdrTitle = hdrTitle + "_" +// newHeading constructs a heading from the comment.Heading and remembers it. +func (r *Renderer) newHeading(h *comment.Heading, hs *headingScope) heading { + return hs.addHeading(h, r.textsToHTML(h.Text)) +} + +// addHeading constructs a heading and adds it to hs. +func (hs *headingScope) addHeading(ch *comment.Heading, html safe.HTML) heading { + h := heading{Title: html} + if hs.createIDs { + h.ID = safe.IdentifierFromConstantPrefix("hdr", hs.newHeadingID(ch)) } + hs.headings = append(hs.headings, h) + return h +} +var badAnchorRx = regexp.MustCompile(`[^a-zA-Z0-9-]`) + +// newHeadingID constructs a unique heading ID from the argument, which is the text +// of a heading. +func (hs *headingScope) newHeadingID(ch *comment.Heading) string { + // Start with the heading's default ID. + // We can only construct a [safe.Identifier] by adding a prefix, so remove the + // default's prefix. + id := strings.TrimPrefix(ch.DefaultID(), "hdr-") + if hs.suffix != "" { + id += "-" + hs.suffix + } + n := hs.ids[id] + hs.ids[id]++ + if n > 0 { + id += "-" + strconv.Itoa(n) + } return id } -func (r *Renderer) newHeading(h *comment.Heading, decl ast.Decl) heading { - return heading{r.headingID(h, decl), r.textsToHTML(h.Text)} +// headingIDSuffix returns a unique string from the decl, to be +// used as the suffix in heading IDs. +// If a unique string can be created, the second return value is true. +// If not (the declaration is a group), headingIDSuffix returns "", false. +func headingIDSuffix(decl ast.Decl) (string, bool) { + if decl == nil { + return "", true + } + switch decl := decl.(type) { + case *ast.FuncDecl: + // functionName, or recvTypeName_methodName + if decl.Recv == nil || len(decl.Recv.List) == 0 { + return decl.Name.Name, true + } + return recvTypeString(decl.Recv.List[0].Type) + "_" + decl.Name.Name, true + case *ast.GenDecl: + // We can only pick a stable name when there is a single decl. + if len(decl.Specs) == 1 { + switch spec := decl.Specs[0].(type) { + case *ast.TypeSpec: + return spec.Name.Name, true + case *ast.ValueSpec: + if len(spec.Names) == 1 { + return spec.Names[0].Name, true + } + } + } + } + return "", false +} + +// recvTypeString returns a string for the symbol in a type expression +// for a method receiver. +func recvTypeString(t ast.Expr) string { + switch t := t.(type) { + case *ast.Ident: + return t.Name + case *ast.IndexExpr: + return t.X.(*ast.Ident).Name + case *ast.StarExpr: + return recvTypeString(t.X) + default: // should never happen + return "unknown" + } } func (r *Renderer) textsToHTML(ts []comment.Text) safe.HTML { diff --git a/internal/godoc/dochtml/internal/render/linkify_test.go b/internal/godoc/dochtml/internal/render/linkify_test.go index 20e788416..bbcf5e767 100644 --- a/internal/godoc/dochtml/internal/render/linkify_test.go +++ b/internal/godoc/dochtml/internal/render/linkify_test.go @@ -9,14 +9,17 @@ import ( "fmt" "go/ast" "go/doc" + "go/doc/comment" "go/parser" "go/token" "path/filepath" + "slices" "strings" "testing" "github.com/google/go-cmp/cmp" "github.com/google/safehtml" + safe "github.com/google/safehtml" "github.com/google/safehtml/testconversions" "golang.org/x/tools/txtar" ) @@ -56,10 +59,14 @@ func TestFormatDocHTML(t *testing.T) { doc := string(mustContent(t, "doc")) wantNoExtract := mustContent(t, "want") + var decl ast.Decl + if d := getContent("decl"); d != "" { + decl = parseDecl(t, d) + } for _, extractLinks := range []bool{false, true} { t.Run(fmt.Sprintf("extractLinks=%t", extractLinks), func(t *testing.T) { r := New(context.Background(), nil, pkgTime, nil) - got := r.formatDocHTML(doc, nil, extractLinks).String() + got := r.formatDocHTML(doc, decl, extractLinks).String() want := wantNoExtract wantLinks := "" if extractLinks { @@ -71,6 +78,8 @@ func TestFormatDocHTML(t *testing.T) { } if diff := cmp.Diff(want, got); diff != "" { t.Errorf("doc mismatch (-want +got)\n%s", diff) + t.Logf("want: %s", want) + t.Logf("got: %s", got) } var b strings.Builder for _, l := range r.Links() { @@ -85,240 +94,6 @@ func TestFormatDocHTML(t *testing.T) { } } -func TestFormatDocHTMLDecl(t *testing.T) { - duplicateHeadersDoc := `Documentation. - -Information - -This is some information. - -Information - -This is some other information. -` - // typeWithFieldsDecl is declared as: - // type I2 interface { - // I1 - // M2() - // } - typeWithFieldsDecl := &ast.GenDecl{ - Tok: token.TYPE, - Specs: []ast.Spec{ - &ast.TypeSpec{ - Name: ast.NewIdent("I2"), - Type: &ast.InterfaceType{ - Methods: &ast.FieldList{ - List: []*ast.Field{ - {Type: ast.NewIdent("I1")}, - {Type: &ast.FuncType{}, Names: []*ast.Ident{ast.NewIdent("M2")}}, - }, - }, - }, - }, - }, - } - - for _, test := range []struct { - name string - doc string - decl ast.Decl - extractLinks []bool // nil means both - want string - wantLinks []Link - }{ - { - name: "unique header ids in constants section for grouped constants", - doc: duplicateHeadersDoc, - decl: &ast.GenDecl{ - Tok: token.CONST, - Specs: []ast.Spec{&ast.ValueSpec{Names: []*ast.Ident{{}}}, &ast.ValueSpec{Names: []*ast.Ident{{}}}}, - }, - want: `
- -
-

Documentation. -

Information

This is some information. -

Information

This is some other information. -

`, - }, - { - name: "unique header ids in variables section for grouped variables", - doc: duplicateHeadersDoc, - decl: &ast.GenDecl{ - Tok: token.VAR, - Specs: []ast.Spec{&ast.ValueSpec{Names: []*ast.Ident{{}}}, &ast.ValueSpec{Names: []*ast.Ident{{}}}}, - }, - want: `
- -
-

Documentation. -

Information

This is some information. -

Information

This is some other information. -

`, - }, - { - name: "unique header ids in functions section", - doc: duplicateHeadersDoc, - decl: &ast.FuncDecl{Name: ast.NewIdent("FooFunc")}, - want: `
- -
-

Documentation. -

Information

This is some information. -

Information

This is some other information. -

`, - }, - { - name: "unique header ids in functions section for method", - doc: duplicateHeadersDoc, - decl: &ast.FuncDecl{ - Recv: &ast.FieldList{List: []*ast.Field{{Type: ast.NewIdent("Bar")}}}, - Name: ast.NewIdent("Func"), - }, - want: `
- -
-

Documentation. -

Information

This is some information. -

Information

This is some other information. -

`, - }, - { - name: "unique header ids in types section", - doc: duplicateHeadersDoc, - decl: &ast.GenDecl{ - Tok: token.TYPE, - Specs: []ast.Spec{&ast.TypeSpec{Name: ast.NewIdent("Duration")}}, - }, - want: `
- -
-

Documentation. -

Information

This is some information. -

Information

This is some other information. -

`, - }, - { - name: "unique header ids in types section for types with fields", - doc: duplicateHeadersDoc, - decl: typeWithFieldsDecl, - want: `
- -
-

Documentation. -

Information

This is some information. -

Information

This is some other information. -

`, - }, - { - name: "unique header ids in types section for typed variable", - doc: duplicateHeadersDoc, - decl: &ast.GenDecl{ - Tok: token.VAR, - Specs: []ast.Spec{&ast.ValueSpec{Type: &ast.StarExpr{X: ast.NewIdent("Location")}, Names: []*ast.Ident{ast.NewIdent("UTC")}}}, - }, - want: `
- -
-

Documentation. -

Information

This is some information. -

Information

This is some other information. -

`, - }, - { - name: "unique header ids in types section for typed constant", - doc: duplicateHeadersDoc, - decl: &ast.GenDecl{ - Tok: token.CONST, - Specs: []ast.Spec{&ast.ValueSpec{Type: ast.NewIdent("T"), Names: []*ast.Ident{ast.NewIdent("C")}}}, - }, - want: `
- -
-

Documentation. -

Information

This is some information. -

Information

This is some other information. -

`, - }, - } { - t.Run(test.name, func(t *testing.T) { - extractLinks := test.extractLinks - if extractLinks == nil { - extractLinks = []bool{false, true} - } - for _, el := range extractLinks { - t.Run(fmt.Sprintf("extractLinks=%t", el), func(t *testing.T) { - r := New(context.Background(), nil, pkgTime, nil) - got := r.formatDocHTML(test.doc, test.decl, el) - want := testconversions.MakeHTMLForTest(test.want) - if diff := cmp.Diff(want, got, cmp.AllowUnexported(safehtml.HTML{})); diff != "" { - t.Errorf("doc mismatch (-want +got)\n%s", diff) - } - if diff := cmp.Diff(test.wantLinks, r.Links()); diff != "" { - t.Errorf("r.Links() mismatch (-want +got)\n%s", diff) - } - }) - } - }) - } -} - func TestDeclHTML(t *testing.T) { for _, test := range []struct { name string @@ -711,13 +486,9 @@ More text.` want := testconversions.MakeHTMLForTest(`

Documentation.

The Go Project

Go is an open source project. @@ -730,3 +501,98 @@ More text.` t.Errorf("r.declHTML() mismatch (-want +got)\n%s", diff) } } + +func TestHeadingIDSuffix(t *testing.T) { + for _, test := range []struct { + decl string + want string + wantIDs bool + }{ + {"", "", true}, + {"func Foo(){}", "Foo", true}, + {"func (x T) Run(){}", "T_Run", true}, + {"func (x *T) Run(){}", "T_Run", true}, + {"func (x T[A]) Run(){}", "T_Run", true}, + {"func (x *T[A]) Run(){}", "T_Run", true}, + {"const C = 1", "C", true}, + {"var V int", "V", true}, + {"var V, W int", "", false}, + {"var x, y, V int", "", false}, + {"type T int", "T", true}, + {"type T_Run[X any] int", "T_Run", true}, + {"const (a = 1; b = 2; C = 3)", "", false}, + {"var (a = 1; b = 2; C = 3)", "", false}, + {"type (a int; b int; C int)", "", false}, + } { + var decl ast.Decl + if test.decl != "" { + decl = parseDecl(t, test.decl) + } + got, gotIDs := headingIDSuffix(decl) + if got != test.want { + t.Errorf("%q: got %q, want %q", test.decl, got, test.want) + } + if gotIDs != test.wantIDs { + t.Errorf("%q createIDs: got %t, want %t", test.decl, gotIDs, test.wantIDs) + } + } +} + +func parseDecl(t *testing.T, decl string) ast.Decl { + prog := "package p\n" + decl + f, err := parser.ParseFile(token.NewFileSet(), "", prog, 0) + if err != nil { + t.Fatal(err) + } + return f.Decls[0] +} + +func TestAddHeading(t *testing.T) { + // This test checks that the generated IDs are unique and the headings are saved. + // It doesn't care about the HTML. + var html safe.HTML + + check := func(hs *headingScope, ids ...string) { + t.Helper() + var want []heading + for _, id := range ids { + want = append(want, heading{safe.IdentifierFromConstantPrefix("hdr", id), html}) + } + if !slices.Equal(hs.headings, want) { + t.Errorf("\ngot %v\nwant %v", hs.headings, want) + } + } + + addHeading := func(hs *headingScope, heading string) { + hs.addHeading(&comment.Heading{ + Text: []comment.Text{comment.Plain(heading)}, + }, html) + } + + hs := newHeadingScope("T", true) + addHeading(hs, "heading") + addHeading(hs, "heading 2") + addHeading(hs, "heading") + addHeading(hs, "heading") + addHeading(hs, "heading.2") + check(hs, "heading-T", "heading_2-T", "heading-T-1", "heading-T-2", "heading_2-T-1") + + // Check empty suffix. + hs = newHeadingScope("", true) + addHeading(hs, "h") + addHeading(hs, "h") + check(hs, "h", "h-1") + + // Check that invalid ID characters are removed from both suffix and input. + hs = newHeadingScope("a.b𝜽", true) + addHeading(hs, "h.i𝜽") + check(hs, "h_i_-a_b_") + + // Check no link (empty ID). + hs = newHeadingScope("", false) + addHeading(hs, "h") + want := []heading{{Title: html}} + if !slices.Equal(hs.headings, want) { + t.Errorf("got %v, want %v", hs.headings, want) + } +} diff --git a/internal/godoc/dochtml/internal/render/render.go b/internal/godoc/dochtml/internal/render/render.go index a5c3e17f4..dc133d9fa 100644 --- a/internal/godoc/dochtml/internal/render/render.go +++ b/internal/godoc/dochtml/internal/render/render.go @@ -26,7 +26,6 @@ var ( type Renderer struct { fset *token.FileSet - headingIDs headingIDs pids *packageIDs packageURL func(string) string ctx context.Context @@ -98,7 +97,6 @@ func New(ctx context.Context, fset *token.FileSet, pkg *doc.Package, opts *Optio return &Renderer{ fset: fset, - headingIDs: headingIDs{}, pids: pids, packageURL: packageURL, docTmpl: docDataTmpl, diff --git a/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/README.md b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/README.md index 0fa611671..26024d034 100644 --- a/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/README.md +++ b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/README.md @@ -12,5 +12,6 @@ By default, "want" is tested with extractLinks set to both true and false. The following sections are optional: - want:links: the output when extractLinks = true -- links: must be present if want:links is present; the extracted - links, one per line, each line has text and href separated by a single space. +- links: the extracted links, one per line + Each line has text and href separated by a single space. +- decl: A Go declaration to be passed to formatDocHTML. Default is nil. diff --git a/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/headinglink.txt b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/headinglink.txt index ef62eb31f..7774ab53d 100644 --- a/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/headinglink.txt +++ b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/headinglink.txt @@ -12,10 +12,8 @@ Go is an open source project. -- want --

The Go Project1

Go is an open source project. diff --git a/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/links.txt b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/links.txt index e25618345..177325e5e 100644 --- a/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/links.txt +++ b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/links.txt @@ -20,16 +20,10 @@ More doc. -- want --

+
  • The Go Project
    • +
  • Links
    • +
  • Header
    • +

    Documentation.

    The Go Project

    Go is an open source project. @@ -41,13 +35,9 @@ More doc. -- want:links --

    Documentation.

    The Go Project

    Go is an open source project. diff --git a/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_func.txt b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_func.txt new file mode 100644 index 000000000..2ffb360fc --- /dev/null +++ b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_func.txt @@ -0,0 +1,25 @@ +Unique heading IDs in function declaration. +-- doc -- +Documentation. + +Info + +This is some information. + +Info + +This is some other information. +-- decl -- +func Run() {} +-- want -- +

    + +
    +

    Documentation. +

    Info

    This is some information. +

    Info

    This is some other information. +

    + diff --git a/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_method.txt b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_method.txt new file mode 100644 index 000000000..735182c1d --- /dev/null +++ b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_method.txt @@ -0,0 +1,25 @@ +Unique heading IDs in method declaration. +-- doc -- +Documentation. + +Info + +This is some information. + +Info + +This is some other information. +-- decl -- +func (T) M() {} +-- want -- +
    + +
    +

    Documentation. +

    Info

    This is some information. +

    Info

    This is some other information. +

    + diff --git a/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_overview.txt b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_overview.txt index d70f8d855..be62b4573 100644 --- a/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_overview.txt +++ b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_overview.txt @@ -1,27 +1,23 @@ -Unique header IDs in overview. +Unique heading IDs in overview. -- doc -- Documentation. -Information +Info This is some information. -Information +Info This is some other information. -- want --
    +
  • Info
    • +
  • Info
    • +

    Documentation. -

    Information

    This is some information. -

    Information

    This is some other information. +

    Info

    This is some information. +

    Info

    This is some other information.

    diff --git a/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_var.txt b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_var.txt new file mode 100644 index 000000000..689114fa3 --- /dev/null +++ b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unique_var.txt @@ -0,0 +1,25 @@ +Unique heading IDs in variable declaration. +-- doc -- +Documentation. + +Info + +This is some information. + +Info + +This is some other information. +-- decl -- +var Global int +-- want -- +
    + +
    +

    Documentation. +

    Info

    This is some information. +

    Info

    This is some other information. +

    + diff --git a/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unlinked_group.txt b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unlinked_group.txt new file mode 100644 index 000000000..0b60d6a59 --- /dev/null +++ b/internal/godoc/dochtml/internal/render/testdata/formatDocHTML/unlinked_group.txt @@ -0,0 +1,25 @@ +Unlinked heading IDs in grouped declaration. +-- doc -- +Documentation. + +Info + +This is some information. + +Info + +This is some other information. +-- decl -- +var A, B int +-- want -- +
    +
      +
    • Info
      • +
    • Info
      • +
    +
    +

    Documentation. +

    Info

    This is some information. +

    Info

    This is some other information. +

    + diff --git a/internal/godoc/dochtml/testdata/comments.golden b/internal/godoc/dochtml/testdata/comments.golden index e3a21b2eb..95ebb2e46 100644 --- a/internal/godoc/dochtml/testdata/comments.golden +++ b/internal/godoc/dochtml/testdata/comments.golden @@ -33,9 +33,7 @@ This refers to the standard library encoding/json p

    F refers to function G and method T.M. @@ -44,7 +42,7 @@ It also has three bullet points:

  • one
  • two
  • three
    • -

    Example

    Here is an example: +

    Example

    Here is an example: 

    F()