Skip to content

Commit

Permalink
fix(internal/godocfx): use correct anchor links (#3738)
Browse files Browse the repository at this point in the history
This works by pre-processing the package and generating all of the correct anchor links. Then, when converting an ID to a link, we check if the ID is listed in the map of anchors.

It's possible there will be some duplication between IDs. But, that is already an issue because there is no way to know which one is correct to link to. An alternative would be to set the anchor for any duplicated links to `""` so they don't link at all. In practice, I don't think this is an issue. So, I left it as-is and we can revisit later if needed.

Fixes #3737.
  • Loading branch information
tbpg committed Feb 25, 2021
1 parent 8cc52d1 commit 919039a
Show file tree
Hide file tree
Showing 2 changed files with 360 additions and 239 deletions.
100 changes: 85 additions & 15 deletions internal/godocfx/parse.go
Expand Up @@ -34,6 +34,7 @@ import (
"log"
"os"
"path/filepath"
"regexp"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -155,7 +156,7 @@ func parse(glob string, workingDir string, optionalExtraFiles []string) (*result
// Once the files are grouped by package, process each package
// independently.
for _, pi := range pkgInfos {
link := newLinker(pi.pkg.Imports, pi.importRenames)
link := newLinker(pi)
topLevelDecls := pkgsite.TopLevelDecls(pi.doc)
pkgItem := &item{
UID: pi.doc.ImportPath,
Expand Down Expand Up @@ -183,7 +184,7 @@ func parse(glob string, workingDir string, optionalExtraFiles []string) (*result
Type: "const",
Summary: c.Doc,
Langs: onlyGo,
Syntax: syntax{Content: pkgsite.PrintType(pi.fset, c.Decl, toURL, topLevelDecls)},
Syntax: syntax{Content: pkgsite.PrintType(pi.fset, c.Decl, link.toURL, topLevelDecls)},
})
}
for _, v := range pi.doc.Vars {
Expand All @@ -199,7 +200,7 @@ func parse(glob string, workingDir string, optionalExtraFiles []string) (*result
Type: "variable",
Summary: v.Doc,
Langs: onlyGo,
Syntax: syntax{Content: pkgsite.PrintType(pi.fset, v.Decl, toURL, topLevelDecls)},
Syntax: syntax{Content: pkgsite.PrintType(pi.fset, v.Decl, link.toURL, topLevelDecls)},
})
}
for _, t := range pi.doc.Types {
Expand All @@ -213,7 +214,7 @@ func parse(glob string, workingDir string, optionalExtraFiles []string) (*result
Type: "type",
Summary: t.Doc,
Langs: onlyGo,
Syntax: syntax{Content: pkgsite.PrintType(pi.fset, t.Decl, toURL, topLevelDecls)},
Syntax: syntax{Content: pkgsite.PrintType(pi.fset, t.Decl, link.toURL, topLevelDecls)},
Examples: processExamples(t.Examples, pi.fset),
}
// Note: items are added as page.Children, rather than
Expand All @@ -232,7 +233,7 @@ func parse(glob string, workingDir string, optionalExtraFiles []string) (*result
Type: "const",
Summary: c.Doc,
Langs: onlyGo,
Syntax: syntax{Content: pkgsite.PrintType(pi.fset, c.Decl, toURL, topLevelDecls)},
Syntax: syntax{Content: pkgsite.PrintType(pi.fset, c.Decl, link.toURL, topLevelDecls)},
})
}
for _, v := range t.Vars {
Expand All @@ -248,7 +249,7 @@ func parse(glob string, workingDir string, optionalExtraFiles []string) (*result
Type: "variable",
Summary: v.Doc,
Langs: onlyGo,
Syntax: syntax{Content: pkgsite.PrintType(pi.fset, v.Decl, toURL, topLevelDecls)},
Syntax: syntax{Content: pkgsite.PrintType(pi.fset, v.Decl, link.toURL, topLevelDecls)},
})
}

Expand Down Expand Up @@ -313,18 +314,84 @@ type linker struct {
// Behavior is undefined when a single import has different names in
// different files.
imports map[string]string

// idToAnchor is a map from an ID to the anchor URL for that ID.
idToAnchor map[string]string
}

func newLinker(rawImports map[string]*packages.Package, importSyntax map[string]string) *linker {
func newLinker(pi pkgInfo) *linker {
imports := map[string]string{}
for path, pkg := range rawImports {
for path, pkg := range pi.pkg.Imports {
name := pkg.Name
if rename := importSyntax[path]; rename != "" {
if rename := pi.importRenames[path]; rename != "" {
name = rename
}
imports[name] = path
}
return &linker{imports: imports}

idToAnchor := buildIDToAnchor(pi)

return &linker{imports: imports, idToAnchor: idToAnchor}
}

// nonWordRegex is based on
// https://github.com/googleapis/doc-templates/blob/70eba5908e7b9aef5525d0f1f24194ae750f267e/third_party/docfx/templates/devsite/common.js#L27-L30.
var nonWordRegex = regexp.MustCompile("\\W")

func buildIDToAnchor(pi pkgInfo) map[string]string {
idToAnchor := map[string]string{}
idToAnchor[pi.doc.ImportPath] = pi.doc.ImportPath

for _, c := range pi.doc.Consts {
commaID := strings.Join(c.Names, ",")
uid := pi.doc.ImportPath + "." + commaID
for _, name := range c.Names {
idToAnchor[name] = uid
}
}
for _, v := range pi.doc.Vars {
commaID := strings.Join(v.Names, ",")
uid := pi.doc.ImportPath + "." + commaID
for _, name := range v.Names {
idToAnchor[name] = uid
}
}
for _, f := range pi.doc.Funcs {
uid := pi.doc.ImportPath + "." + f.Name
idToAnchor[f.Name] = uid
}
for _, t := range pi.doc.Types {
uid := pi.doc.ImportPath + "." + t.Name
idToAnchor[t.Name] = uid
for _, c := range t.Consts {
commaID := strings.Join(c.Names, ",")
uid := pi.doc.ImportPath + "." + commaID
for _, name := range c.Names {
idToAnchor[name] = uid
}
}
for _, v := range t.Vars {
commaID := strings.Join(v.Names, ",")
uid := pi.doc.ImportPath + "." + commaID
for _, name := range v.Names {
idToAnchor[name] = uid
}
}
for _, f := range t.Funcs {
uid := pi.doc.ImportPath + "." + t.Name + "." + f.Name
idToAnchor[f.Name] = uid
}
for _, m := range t.Methods {
uid := pi.doc.ImportPath + "." + t.Name + "." + m.Name
idToAnchor[m.Name] = uid
}
}

for id, anchor := range idToAnchor {
idToAnchor[id] = nonWordRegex.ReplaceAllString(anchor, "_")
}

return idToAnchor
}

func (l *linker) linkify(s string) string {
Expand All @@ -342,11 +409,11 @@ func (l *linker) linkify(s string) string {
// If s is not exported, it's probably a builtin.
if !token.IsExported(s) {
if doc.IsPredeclared(s) {
return href(toURL("builtin", s), s)
return href(l.toURL("builtin", s), s)
}
return fmt.Sprintf("%s%s", prefix, s)
}
return fmt.Sprintf("%s%s", prefix, href(toURL("", s), s))
return fmt.Sprintf("%s%s", prefix, href(l.toURL("", s), s))
}
// Otherwise, it's in another package.
split := strings.Split(s, ".")
Expand All @@ -362,14 +429,17 @@ func (l *linker) linkify(s string) string {
return fmt.Sprintf("%s%s", prefix, s)
}
name := split[1]
return fmt.Sprintf("%s%s.%s", prefix, href(toURL(pkgPath, ""), pkg), href(toURL(pkgPath, name), name))
return fmt.Sprintf("%s%s.%s", prefix, href(l.toURL(pkgPath, ""), pkg), href(l.toURL(pkgPath, name), name))
}

// TODO: link to the right baseURL, with the right module name and version
// pattern.
func toURL(pkg, name string) string {
func (l *linker) toURL(pkg, name string) string {
if pkg == "" {
return fmt.Sprintf("#%s", strings.ToLower(name))
if anchor := l.idToAnchor[name]; anchor != "" {
name = anchor
}
return fmt.Sprintf("#%s", name)
}
baseURL := "https://pkg.go.dev"
if name == "" {
Expand Down

0 comments on commit 919039a

Please sign in to comment.