diff --git a/godoc/godoc17_test.go b/godoc/godoc17_test.go index 82df6e7c2f..d153991c78 100644 --- a/godoc/godoc17_test.go +++ b/godoc/godoc17_test.go @@ -29,7 +29,7 @@ func TestStructField(t *testing.T) { fmt.Fprintf(&buf, "\t// Field%d is foo.\n\tField%d int\n\n", i, i) } fmt.Fprintf(&buf, "}\n") - linkifyStructFields(t, buf.Bytes()) + linkifySource(t, buf.Bytes()) }) } } diff --git a/godoc/godoc_test.go b/godoc/godoc_test.go index 0c32f39ab9..0595a153ac 100644 --- a/godoc/godoc_test.go +++ b/godoc/godoc_test.go @@ -5,7 +5,7 @@ package godoc import ( - "go/ast" + "bytes" "go/parser" "go/token" "strings" @@ -124,7 +124,7 @@ func TestSanitizeFunc(t *testing.T) { // Test that we add elements // to the HTML of struct fields. func TestStructFieldsIDAttributes(t *testing.T) { - got := linkifyStructFields(t, []byte(` + got := linkifySource(t, []byte(` package foo type T struct { @@ -157,7 +157,25 @@ Opt *int } } -func linkifyStructFields(t *testing.T, src []byte) string { +func TestCompositeLitLinkFields(t *testing.T) { + got := linkifySource(t, []byte(` +package foo + +type T struct { + X int +} + +var S T = T{X: 12}`)) + want := `type T struct { +X int +} +var S T = T{X: 12}` + if got != want { + t.Errorf("got: %s\n\nwant: %s\n", got, want) + } +} + +func linkifySource(t *testing.T, src []byte) string { p := &Presentation{ DeclLinks: true, } @@ -166,11 +184,17 @@ func linkifyStructFields(t *testing.T, src []byte) string { if err != nil { t.Fatal(err) } - genDecl := af.Decls[0].(*ast.GenDecl) + var buf bytes.Buffer pi := &PageInfo{ FSet: fset, } - return p.node_htmlFunc(pi, genDecl, true) + sep := "" + for _, decl := range af.Decls { + buf.WriteString(sep) + sep = "\n" + buf.WriteString(p.node_htmlFunc(pi, decl, true)) + } + return buf.String() } func TestScanIdentifier(t *testing.T) { diff --git a/godoc/linkify.go b/godoc/linkify.go index 0a8fb474ec..089737079d 100644 --- a/godoc/linkify.go +++ b/godoc/linkify.go @@ -78,61 +78,6 @@ type link struct { path, name string // package path, identifier name } -// linksFor returns the list of links for the identifiers used -// by node in the same order as they appear in the source. -// -func linksFor(node ast.Node) (list []link) { - modes := identModesFor(node) - - // NOTE: We are expecting ast.Inspect to call the - // callback function in source text order. - ast.Inspect(node, func(node ast.Node) bool { - switch n := node.(type) { - case *ast.Ident: - m := modes[n] - info := link{mode: m} - switch m { - case identUse: - if n.Obj == nil && predeclared[n.Name] { - info.path = builtinPkgPath - } - info.name = n.Name - case identDef: - // any declaration expect const or var - empty link - case identVal: - // const or var declaration - info.name = n.Name - } - list = append(list, info) - return false - case *ast.SelectorExpr: - // Detect qualified identifiers of the form pkg.ident. - // If anything fails we return true and collect individual - // identifiers instead. - if x, _ := n.X.(*ast.Ident); x != nil { - // x must be a package for a qualified identifier - if obj := x.Obj; obj != nil && obj.Kind == ast.Pkg { - if spec, _ := obj.Decl.(*ast.ImportSpec); spec != nil { - // spec.Path.Value is the import path - if path, err := strconv.Unquote(spec.Path.Value); err == nil { - // Register two links, one for the package - // and one for the qualified identifier. - info := link{path: path} - list = append(list, info) - info.name = n.Sel.Name - list = append(list, info) - return false - } - } - } - } - } - return true - }) - - return -} - // The identMode describes how an identifier is "used" at its source location. type identMode int @@ -142,28 +87,32 @@ const ( identVal // identifier is defined in a const or var declaration ) -// identModesFor returns a map providing the identMode for each identifier used by node. -func identModesFor(node ast.Node) map[*ast.Ident]identMode { - m := make(map[*ast.Ident]identMode) +// linksFor returns the list of links for the identifiers used +// by node in the same order as they appear in the source. +// +func linksFor(node ast.Node) (links []link) { + // linkMap tracks link information for each ast.Ident node. Entries may + // be created out of source order (for example, when we visit a parent + // definition node). These links are appended to the returned slice when + // their ast.Ident nodes are visited. + linkMap := make(map[*ast.Ident]link) ast.Inspect(node, func(node ast.Node) bool { switch n := node.(type) { case *ast.Field: for _, n := range n.Names { - m[n] = identDef + linkMap[n] = link{mode: identDef} } case *ast.ImportSpec: if name := n.Name; name != nil { - m[name] = identDef + linkMap[name] = link{mode: identDef} } case *ast.ValueSpec: for _, n := range n.Names { - m[n] = identVal + linkMap[n] = link{mode: identVal} } case *ast.TypeSpec: - m[n.Name] = identDef - case *ast.FuncDecl: - m[n.Name] = identDef + linkMap[n.Name] = link{mode: identDef} case *ast.AssignStmt: // Short variable declarations only show up if we apply // this code to all source code (as opposed to exported @@ -176,15 +125,79 @@ func identModesFor(node ast.Node) map[*ast.Ident]identMode { // Each lhs expression should be an // ident, but we are conservative and check. if n, _ := x.(*ast.Ident); n != nil { - m[n] = identVal + linkMap[n] = link{mode: identVal} } } } + case *ast.SelectorExpr: + // Detect qualified identifiers of the form pkg.ident. + // If anything fails we return true and collect individual + // identifiers instead. + if x, _ := n.X.(*ast.Ident); x != nil { + // Create links only if x is a qualified identifier. + if obj := x.Obj; obj != nil && obj.Kind == ast.Pkg { + if spec, _ := obj.Decl.(*ast.ImportSpec); spec != nil { + // spec.Path.Value is the import path + if path, err := strconv.Unquote(spec.Path.Value); err == nil { + // Register two links, one for the package + // and one for the qualified identifier. + linkMap[x] = link{mode: identUse, path: path} + linkMap[n.Sel] = link{mode: identUse, path: path, name: n.Sel.Name} + } + } + } + } + case *ast.CompositeLit: + // Detect field names within composite literals. These links should + // be prefixed by the type name. + fieldPath := "" + prefix := "" + switch typ := n.Type.(type) { + case *ast.Ident: + prefix = typ.Name + "." + case *ast.SelectorExpr: + if x, _ := typ.X.(*ast.Ident); x != nil { + // Create links only if x is a qualified identifier. + if obj := x.Obj; obj != nil && obj.Kind == ast.Pkg { + if spec, _ := obj.Decl.(*ast.ImportSpec); spec != nil { + // spec.Path.Value is the import path + if path, err := strconv.Unquote(spec.Path.Value); err == nil { + // Register two links, one for the package + // and one for the qualified identifier. + linkMap[x] = link{mode: identUse, path: path} + linkMap[typ.Sel] = link{mode: identUse, path: path, name: typ.Sel.Name} + fieldPath = path + prefix = typ.Sel.Name + "." + } + } + } + } + } + for _, e := range n.Elts { + if kv, ok := e.(*ast.KeyValueExpr); ok { + if k, ok := kv.Key.(*ast.Ident); ok { + // Note: there is some syntactic ambiguity here. We cannot determine + // if this is a struct literal or a map literal without type + // information. We assume struct literal. + name := prefix + k.Name + linkMap[k] = link{mode: identUse, path: fieldPath, name: name} + } + } + } + case *ast.Ident: + if l, ok := linkMap[n]; ok { + links = append(links, l) + } else { + l := link{mode: identUse, name: n.Name} + if n.Obj == nil && predeclared[n.Name] { + l.path = builtinPkgPath + } + links = append(links, l) + } } return true }) - - return m + return } // The predeclared map represents the set of all predeclared identifiers.