From c6fca020044817188bd95c060ad9f0bec5c67aaa Mon Sep 17 00:00:00 2001 From: aarzilli Date: Tue, 1 Feb 2022 18:45:42 +0100 Subject: [PATCH] godoc: handle type parameters correctly in LinkifyText LinkifyText should not generate links to global declarations for identifiers that are type parameters. Because the syntactic resolver does not record the declaration for type parameters declared in the method receiver (see https://golang.org/issue/50956) a name lookup is used as a workaround. This is fine here because it is only applied to undeclared indentifiers and LinkifyText is only ever called on a single declaration at a time, not on a full AST. Updates golang/go#50717 Change-Id: I32f2203ce80c060ee18ca8b964a2d5fd80f41957 Reviewed-on: https://go-review.googlesource.com/c/tools/+/382714 Reviewed-by: Robert Findley Run-TryBot: Alessandro Arzilli gopls-CI: kokoro TryBot-Result: Gopher Robot Trust: Peter Weinberger --- godoc/godoc_test.go | 57 +++++++++++++++++++++++++++++++++++++++++++++ godoc/linkify.go | 44 ++++++++++++++++++++++++++++++++-- 2 files changed, 99 insertions(+), 2 deletions(-) diff --git a/godoc/godoc_test.go b/godoc/godoc_test.go index 33dbe3f619..7f3470ed07 100644 --- a/godoc/godoc_test.go +++ b/godoc/godoc_test.go @@ -10,6 +10,8 @@ import ( "go/token" "strings" "testing" + + "golang.org/x/tools/internal/typeparams" ) func TestPkgLinkFunc(t *testing.T) { @@ -368,3 +370,58 @@ func TestFilterOutBuildAnnotations(t *testing.T) { t.Errorf("filterOutBuildAnnotations should not remove non-build tag comment") } } + +func TestLinkifyGenerics(t *testing.T) { + if !typeparams.Enabled { + t.Skip("type params are not enabled at this Go version") + } + + got := linkifySource(t, []byte(` +package foo + +type T struct { + field *T +} + +type ParametricStruct[T any] struct { + field *T +} + +func F1[T any](arg T) { } + +func F2(arg T) { } + +func (*ParametricStruct[T]) M(arg T) { } + +func (*T) M(arg T) { } + +type ParametricStruct2[T1, T2 any] struct { + a T1 + b T2 +} + +func (*ParametricStruct2[T1, T2]) M(a T1, b T2) { } + + +`)) + + want := `type T struct { +field *T +} +type ParametricStruct[T any] struct { +field *T +} +func F1[T any](arg T) {} +func F2(arg T) {} +func (*ParametricStruct[T]) M(arg T) {} +func (*T) M(arg T) {} +type ParametricStruct2[T1, T2 any] struct { +a T1 +b T2 +} +func (*ParametricStruct2[T1, T2]) M(a T1, b T2) {}` + + if got != want { + t.Errorf("got: %s\n\nwant: %s\n", got, want) + } +} diff --git a/godoc/linkify.go b/godoc/linkify.go index e4add22a10..4a9c506047 100644 --- a/godoc/linkify.go +++ b/godoc/linkify.go @@ -17,6 +17,8 @@ import ( "go/token" "io" "strconv" + + "golang.org/x/tools/internal/typeparams" ) // LinkifyText HTML-escapes source text and writes it to w. @@ -89,6 +91,8 @@ func linksFor(node ast.Node) (links []link) { // their ast.Ident nodes are visited. linkMap := make(map[*ast.Ident]link) + typeParams := make(map[string]bool) + ast.Inspect(node, func(node ast.Node) bool { switch n := node.(type) { case *ast.Field: @@ -105,6 +109,24 @@ func linksFor(node ast.Node) (links []link) { } case *ast.FuncDecl: linkMap[n.Name] = link{} + if n.Recv != nil { + recv := n.Recv.List[0].Type + if r, isstar := recv.(*ast.StarExpr); isstar { + recv = r.X + } + switch x := recv.(type) { + case *ast.IndexExpr: + if ident, _ := x.Index.(*ast.Ident); ident != nil { + typeParams[ident.Name] = true + } + case *typeparams.IndexListExpr: + for _, index := range x.Indices { + if ident, _ := index.(*ast.Ident); ident != nil { + typeParams[ident.Name] = true + } + } + } + } case *ast.TypeSpec: linkMap[n.Name] = link{} case *ast.AssignStmt: @@ -183,8 +205,26 @@ func linksFor(node ast.Node) (links []link) { links = append(links, l) } else { l := link{name: n.Name} - if n.Obj == nil && doc.IsPredeclared(n.Name) { - l.path = builtinPkgPath + if n.Obj == nil { + if doc.IsPredeclared(n.Name) { + l.path = builtinPkgPath + } else { + if typeParams[n.Name] { + // If a type parameter was declared then do not generate a link. + // Doing this is necessary because type parameter identifiers do not + // have their Decl recorded sometimes, see + // https://golang.org/issue/50956. + l = link{} + } + } + } else { + if n.Obj.Kind == ast.Typ { + if _, isfield := n.Obj.Decl.(*ast.Field); isfield { + // If an identifier is a type declared in a field assume it is a type + // parameter and do not generate a link. + l = link{} + } + } } links = append(links, l) }