internal/godoc/dochtml: remove hotlink code
Remove the code that creates implicit links to Go identifiers in
documentation comments.
This code has always been disabled, and with the likely acceptance of
some form of https://go.dev/issue/51082, which favors explicit links
in godoc over implicit ones, it is unlikely ever to be enabled.
Removing it will pave the way for easier integration with the
go/doc/comment package in the above proposal.
Change-Id: Ief69b392af44a1efde48fd648ef04de6574a8910
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/385194
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
diff --git a/internal/godoc/dochtml/dochtml.go b/internal/godoc/dochtml/dochtml.go
index af083b1..11600df 100644
--- a/internal/godoc/dochtml/dochtml.go
+++ b/internal/godoc/dochtml/dochtml.go
@@ -263,8 +263,7 @@
}
return "/" + versionedPath + search
},
- DisableHotlinking: true,
- EnableCommandTOC: true,
+ EnableCommandTOC: true,
})
fileLink := func(name string) safehtml.HTML {
diff --git a/internal/godoc/dochtml/internal/render/idents.go b/internal/godoc/dochtml/internal/render/idents.go
index b7911f1..a3f76f2 100644
--- a/internal/godoc/dochtml/internal/render/idents.go
+++ b/internal/godoc/dochtml/internal/render/idents.go
@@ -7,11 +7,7 @@
import (
"go/ast"
"go/token"
- "strings"
- "unicode"
- "unicode/utf8"
- "github.com/google/safehtml"
"github.com/google/safehtml/template"
"golang.org/x/pkgsite/internal/godoc/internal/doc"
)
@@ -214,101 +210,6 @@
return url
}
-// toHTML formats a dot-delimited word as HTML with each ID segment converted
-// to be a link to the relevant declaration.
-func (r identifierResolver) toHTML(word string) safehtml.HTML {
- // extraSuffix is extra identifier segments that can't be matched
- // probably because we lack type information.
- var extraSuffix safehtml.HTML // E.g., ".Get" for an unknown Get method
- origIDs := strings.Split(word, ".")
-
- // Skip any standalone unexported identifier.
- if !isExported(word) && len(origIDs) == 1 {
- return safehtml.HTMLEscaped(word)
- }
-
- // Generate variations on the original word.
- var altWords []string
- if vtype, ok := r.paramTypes[origIDs[0]]; ok {
- altWords = append(altWords, vtype+word[len(origIDs[0]):]) // E.g., "r.Read" => "io.Reader.Read"
- } else if r.recvType != "" {
- altWords = append(altWords, r.recvType+"."+word) // E.g., "Read" => "Reader.Read"
- }
- altWords = append(altWords, word)
-
- var altWord string
- for _, s := range altWords {
- if _, _, ok := r.lookup(s); ok {
- altWord = s // direct match
- goto linkify
- }
- if _, _, ok := r.lookup(strings.TrimSuffix(s, "s")); ok {
- altWord = s[:len(s)-len("s")] // E.g., "Caches" => "Cache"
- goto linkify
- }
- if _, _, ok := r.lookup(strings.TrimSuffix(s, "es")); ok {
- altWord = s[:len(s)-len("es")] // E.g., "Boxes" => "Box"
- goto linkify
- }
-
- // Repeatedly truncate the last segment, searching for a partial match.
- i, j := len(s), len(word)
- for i >= 0 && j >= 0 {
- allowPartial := isExported(word[:j]) || strings.Contains(word[:j], ".")
- if _, _, ok := r.lookup(s[:i]); ok && allowPartial {
- altWord = s[:i]
- origIDs = strings.Split(word[:j], ".")
- extraSuffix = safehtml.HTMLEscaped(word[j:])
- goto linkify
- }
- i = strings.LastIndexByte(s[:i], '.')
- j = strings.LastIndexByte(word[:j], '.')
- }
- }
- return safehtml.HTMLEscaped(word) // no match found
-
-linkify:
- // altWord contains a modified dot-separated identifier.
- // origIDs contains the segments of the original identifier.
- // It is possible for Split(altWord, ".") to be longer than origIDs
- // if an implicit type was prepended.
- // E.g., altWord="io.Reader.Read", origIDs=["r", "Read"]
-
- // Skip past implicit prefix selectors.
- // E.g., i=3, altWord[i:]="Reader.Read"
- var i int
- for strings.Count(altWord[i:], ".")+1 != len(origIDs) {
- i += strings.IndexByte(altWord[i:], '.') + 1
- }
-
- var outs []safehtml.HTML
- for _, s := range origIDs {
- // Advance to the next segment in altWord.
- // E.g., i=9, altWord[:i]="io.Reader", s="r"
- // E.g., i=14, altWord[:i]="io.Reader.Read", s="Read"
- if n := strings.IndexByte(altWord[i+1:], '.'); n >= 0 {
- i += 1 + n
- } else {
- i = len(altWord)
- }
-
- path, name, _ := r.lookup(altWord[:i])
- u := r.toURL(path, name)
- html, err := LinkTemplate.ExecuteToHTML(Link{Href: u, Text: s})
- if err != nil {
- html = safehtml.HTMLEscaped("[" + err.Error() + "]")
- }
- outs = append(outs, html)
- outs = append(outs, safehtml.HTMLEscaped("."))
- }
- if len(outs) == 0 {
- return extraSuffix
- }
- // Replace final dot with extraSuffix.
- outs[len(outs)-1] = extraSuffix
- return safehtml.HTMLConcat(outs...)
-}
-
// Link is the data passed to LinkTemplate.
type Link struct {
Href, Text string
@@ -318,27 +219,6 @@
var LinkTemplate = template.Must(template.New("link").Parse(
`<a {{with .Class}}class="{{.}}" {{end}}href="{{.Href}}">{{.Text}}</a>`))
-// lookup looks up a dot-separated identifier.
-// E.g., "pkg", "pkg.Var", "Recv.Method", "Struct.Field", "pkg.Struct.Field"
-func (r identifierResolver) lookup(id string) (pkgPath, name string, ok bool) {
- if r.pkgIDs[r.name][id] {
- return "", id, true // ID refers to local top-level declaration
- }
- if path := r.impPaths[id]; path != "" {
- return path, "", true // ID refers to a package
- }
- if i := strings.IndexByte(id, '.'); i >= 0 {
- prefix, suffix := id[:i], id[i+1:]
- if r.pkgIDs[prefix][suffix] {
- if prefix == r.name {
- prefix = ""
- }
- return r.impPaths[prefix], suffix, true // ID refers to a different package's top-level declaration
- }
- }
- return "", "", false // not found
-}
-
func nodeName(n ast.Node) (string, *ast.Ident) {
switch n := n.(type) {
case *ast.Ident:
@@ -354,8 +234,3 @@
return "", nil
}
}
-
-func isExported(id string) bool {
- r, _ := utf8.DecodeRuneInString(id)
- return unicode.IsUpper(r)
-}
diff --git a/internal/godoc/dochtml/internal/render/idents_test.go b/internal/godoc/dochtml/internal/render/idents_test.go
deleted file mode 100644
index f89d464..0000000
--- a/internal/godoc/dochtml/internal/render/idents_test.go
+++ /dev/null
@@ -1,133 +0,0 @@
-// Copyright 2017 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package render
-
-import (
- "go/ast"
- "testing"
-
- "golang.org/x/pkgsite/internal/godoc/internal/doc"
-)
-
-func TestResolveIdentifier(t *testing.T) {
- type (
- resolveTest struct {
- in string
- want string
- }
- declTest struct {
- name string
- tests []resolveTest
- }
- pkgTest struct {
- pkg *doc.Package
- related []*doc.Package
- tests []declTest
- }
- )
-
- tests := []pkgTest{{
- pkg: pkgTar,
- related: []*doc.Package{pkgIO, pkgOS, pkgTime},
- tests: []declTest{{
- name: "",
- tests: []resolveTest{
- {`"`, `"`}, // HTML escaping
- {`tar`, `tar`},
- {`blah`, `blah`},
- {`io.EOF`, `<a href="/io">io</a>.<a href="/io#EOF">EOF</a>`},
- {`otherPkg.Identifier`, `otherPkg.Identifier`},
- {`time.Time`, `<a href="/time">time</a>.<a href="/time#Time">Time</a>`},
- {`time.Time.String`, `<a href="/time">time</a>.<a href="/time#Time">Time</a>.<a href="/time#Time.String">String</a>`},
- {`time.NoExist`, `time.NoExist`},
- {`Writer.NoExist`, `<a href="#Writer">Writer</a>.NoExist`},
- {`Writer.WriteHeader`, `<a href="#Writer">Writer</a>.<a href="#Writer.WriteHeader">WriteHeader</a>`},
- {`ErrHeader.Error`, `<a href="#ErrHeader">ErrHeader</a>.Error`}, // Can't link Error method on variable
- {`Format`, `<a href="#Format">Format</a>`},
- {`Writers`, `<a href="#Writer">Writers</a>`},
- {`Write`, `Write`},
- },
- }, {
- name: "Header",
- tests: []resolveTest{
- {`Format`, `<a href="#Header.Format">Format</a>`}, // Header.Format field takes precedence over Format type
- {`tar.Format`, `<a href="/archive/tar">tar</a>.<a href="#Format">Format</a>`}, // Refers to package level Format type
- },
- }, {
- name: "Reader.Read",
- tests: []resolveTest{
- {`Read`, `<a href="#Reader.Read">Read</a>`},
- {`io.EOF`, `<a href="/io">io</a>.<a href="/io#EOF">EOF</a>`},
- {`Next`, `<a href="#Reader.Next">Next</a>`},
- {`Header.Size`, `<a href="#Header">Header</a>.<a href="#Header.Size">Size</a>`},
- {`TypeLink`, `<a href="#TypeLink">TypeLink</a>`},
- {`tr.WriteTo`, `<a href="#Reader">tr</a>.<a href="#Reader.WriteTo">WriteTo</a>`},
- {`tr.WriteTo.NoExist`, `<a href="#Reader">tr</a>.<a href="#Reader.WriteTo">WriteTo</a>.NoExist`},
- {`tr.NoExist`, `tr.NoExist`},
- },
- }},
- }, {
- pkg: pkgTime,
- tests: []declTest{{
- name: "",
- tests: []resolveTest{
- {`time.Time`, `<a href="/time">time</a>.<a href="#Time">Time</a>`},
- {`Time.MarshalBinary`, `<a href="#Time">Time</a>.<a href="#Time.MarshalBinary">MarshalBinary</a>`},
- {`RFC822`, `<a href="#RFC822">RFC822</a>`},
- {`RFC823`, `RFC823`}, // This constant does not exist
- },
- }, {
- name: "Timer.Reset",
- tests: []resolveTest{
- {`Reset`, `<a href="#Timer.Reset">Reset</a>`},
- {`t.C`, `<a href="#Timer">t</a>.<a href="#Timer.C">C</a>`},
- {`Stop`, `<a href="#Timer.Stop">Stop</a>`},
- {`t.MarshalBinary`, `t.MarshalBinary`}, // MarshalBinary is not a method of Timer
- },
- }},
- }}
-
- for _, pt := range tests {
- pids := newPackageIDs(pt.pkg, pt.related...)
- t.Run(pt.pkg.Name, func(t *testing.T) {
- for _, dt := range pt.tests {
- dids := newDeclIDs(findDecl(pt.pkg, dt.name))
- t.Run(dt.name, func(t *testing.T) {
- idr := &identifierResolver{pids, dids, nil}
- for i, rt := range dt.tests {
- got := idr.toHTML(rt.in)
- if got.String() != rt.want {
- t.Errorf("test %d, identifierResolver.toHTML():\ngot `%s`\nwant `%s`", i, got, rt.want)
- }
- }
- })
- }
- })
- }
-}
-
-func findDecl(pkg *doc.Package, id string) ast.Decl {
- for _, f := range pkg.Funcs {
- if f.Name == id {
- return f.Decl
- }
- }
- for _, t := range pkg.Types {
- if t.Name == id {
- return t.Decl
- }
- for _, f := range t.Funcs {
- if f.Name == id {
- return f.Decl
- }
- }
- for _, m := range t.Methods {
- if t.Name+"."+m.Name == id {
- return m.Decl
- }
- }
- }
- return nil
-}
diff --git a/internal/godoc/dochtml/internal/render/linkify.go b/internal/godoc/dochtml/internal/render/linkify.go
index cdb6111..bce2a87 100644
--- a/internal/godoc/dochtml/internal/render/linkify.go
+++ b/internal/godoc/dochtml/internal/render/linkify.go
@@ -48,16 +48,12 @@
// Optional path, query, fragment (e.g. "/path/index.html?q=foo#bar").
pathPart = `([.,:;?!]*[a-zA-Z0-9$'()*+&#=@~_/\-\[\]%])*`
- // Regexp for Go identifiers.
- identRx = `[\pL_][\pL_0-9]*`
- qualIdentRx = identRx + `(\.` + identRx + `)*`
-
// Regexp for RFCs.
rfcRx = `RFC\s+(\d{3,5})(,?\s+[Ss]ection\s+(\d+(\.\d+)*))?`
)
var (
- matchRx = regexp.MustCompile(urlRx + `|` + rfcRx + `|` + qualIdentRx)
+ matchRx = regexp.MustCompile(urlRx + `|` + rfcRx)
badAnchorRx = regexp.MustCompile(`[^a-zA-Z0-9]`)
)
@@ -91,7 +87,7 @@
if inLinks {
r.links = append(r.links, parseLinks(blk.lines)...)
} else {
- el.Body = r.linesToHTML(blk.lines, idr)
+ el.Body = r.linesToHTML(blk.lines)
els = append(els, el)
}
case *preformat:
@@ -99,7 +95,7 @@
r.links = append(r.links, parseLinks(blk.lines)...)
} else {
el.IsPreformat = true
- el.Body = r.linesToHTML(blk.lines, nil)
+ el.Body = r.linesToHTML(blk.lines)
els = append(els, el)
}
case *heading:
@@ -163,11 +159,11 @@
}
}
-func (r *Renderer) linesToHTML(lines []string, idr *identifierResolver) safehtml.HTML {
+func (r *Renderer) linesToHTML(lines []string) safehtml.HTML {
newline := safehtml.HTMLEscaped("\n")
htmls := make([]safehtml.HTML, 0, 2*len(lines))
for _, l := range lines {
- htmls = append(htmls, r.formatLineHTML(l, idr))
+ htmls = append(htmls, r.formatLineHTML(l))
htmls = append(htmls, newline)
}
return safehtml.HTMLConcat(htmls...)
@@ -175,7 +171,7 @@
func (r *Renderer) codeString(ex *doc.Example) (string, error) {
if ex == nil || ex.Code == nil {
- return "", errors.New("Please include an example with code")
+ return "", errors.New("please include an example with code")
}
var buf bytes.Buffer
@@ -268,9 +264,8 @@
// formatLineHTML formats the line as HTML-annotated text.
// URLs and Go identifiers are linked to corresponding declarations.
-func (r *Renderer) formatLineHTML(line string, idr *identifierResolver) safehtml.HTML {
+func (r *Renderer) formatLineHTML(line string) safehtml.HTML {
var htmls []safehtml.HTML
- var lastChar, nextChar byte
var numQuotes int
addLink := func(href, text string) {
@@ -286,25 +281,10 @@
if m0 > 0 {
nonWord := line[:m0]
htmls = append(htmls, safehtml.HTMLEscaped(nonWord))
- lastChar = nonWord[len(nonWord)-1]
numQuotes += countQuotes(nonWord)
}
if m1 > m0 {
word := line[m0:m1]
- nextChar = 0
- if m1 < len(line) {
- nextChar = line[m1]
- }
-
- // Reduce false-positives by having a list of allowed
- // characters preceding and succeeding an identifier.
- // Also, forbid ID linking within unbalanced quotes on same line.
- validPrefix := strings.IndexByte("\x00 \t()[]*\n", lastChar) >= 0
- validSuffix := strings.IndexByte("\x00 \t()[]:;,.'\n", nextChar) >= 0
- forbidLinking := !validPrefix || !validSuffix || numQuotes%2 != 0
-
- // TODO: Should we provide hotlinks for related packages?
-
switch {
case strings.Contains(word, "://"):
// Forbid closing brackets without prior opening brackets.
@@ -344,8 +324,6 @@
// RFC x
addLink(fmt.Sprintf("https://rfc-editor.org/rfc/rfc%s.html", rfcFields[1]), word)
}
- case !forbidLinking && !r.disableHotlinking && idr != nil: // && numQuotes%2 == 0:
- htmls = append(htmls, idr.toHTML(word))
default:
htmls = append(htmls, safehtml.HTMLEscaped(word))
}
@@ -447,7 +425,7 @@
tokType = commentType
htmlLines[line] = append(htmlLines[line],
template.MustParseAndExecuteToHTML(`<span class="comment">`),
- r.formatLineHTML(lit, idr),
+ r.formatLineHTML(lit),
template.MustParseAndExecuteToHTML(`</span>`))
lastOffset += len(lit)
case token.IDENT:
diff --git a/internal/godoc/dochtml/internal/render/render.go b/internal/godoc/dochtml/internal/render/render.go
index 021a281..c1f1138 100644
--- a/internal/godoc/dochtml/internal/render/render.go
+++ b/internal/godoc/dochtml/internal/render/render.go
@@ -29,7 +29,6 @@
fset *token.FileSet
pids *packageIDs
packageURL func(string) string
- disableHotlinking bool
disablePermalinks bool
enableCommandTOC bool
ctx context.Context
@@ -52,11 +51,6 @@
// Only relevant for HTML formatting.
PackageURL func(pkgPath string) (url string)
- // DisableHotlinking turns off hotlinking behavior.
- //
- // Only relevant for HTML formatting.
- DisableHotlinking bool
-
// DisablePermalinks turns off inserting of '¶' permalinks in headings.
//
// Only relevant for HTML formatting.
@@ -106,7 +100,6 @@
func New(ctx context.Context, fset *token.FileSet, pkg *doc.Package, opts *Options) *Renderer {
var others []*doc.Package
var packageURL func(string) string
- var disableHotlinking bool
var disablePermalinks bool
var enableCommandTOC bool
if opts != nil {
@@ -116,7 +109,6 @@
if opts.PackageURL != nil {
packageURL = opts.PackageURL
}
- disableHotlinking = opts.DisableHotlinking
disablePermalinks = opts.DisablePermalinks
enableCommandTOC = opts.EnableCommandTOC
}
@@ -126,7 +118,6 @@
fset: fset,
pids: pids,
packageURL: packageURL,
- disableHotlinking: disableHotlinking,
disablePermalinks: disablePermalinks,
enableCommandTOC: enableCommandTOC,
docTmpl: docDataTmpl,
