Skip to content

Commit

Permalink
godoc: add version info for struct fields
Browse files Browse the repository at this point in the history 
Follow-up to CL 85396, which only did types, funcs, and methods.

This adds version info to struct fields (in the form of small
comments) if the struct field's version is different from the struct
itself, to minimize how often this fires.

Updates golang/go#5778

Change-Id: I34d60326cbef88c108d5c4ca487eeb98b039b16e
Reviewed-on: https://go-review.googlesource.com/124495
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Devon O'Dell <dhobsd@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
  • Loading branch information
@bradfitz
bradfitz committed Jul 17, 2018
1 parent 57f659e commit 32950ab
Show file tree
Hide file tree
Showing 5 changed files with 176 additions and 41 deletions.
85 changes: 62 additions & 23 deletions cmd/godoc/godoc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,77 +255,106 @@ func testWeb(t *testing.T, withIndex bool) {
}

tests := []struct {
path string
match []string
dontmatch []string
needIndex bool
path string
contains []string // substring
match []string // regexp
notContains []string
needIndex bool
}{
{
path: "/",
match: []string{"Go is an open source programming language"},
path: "/",
contains: []string{"Go is an open source programming language"},
},
{
path: "/pkg/fmt/",
match: []string{"Package fmt implements formatted I/O"},
path: "/pkg/fmt/",
contains: []string{"Package fmt implements formatted I/O"},
},
{
path: "/src/fmt/",
match: []string{"scan_test.go"},
path: "/src/fmt/",
contains: []string{"scan_test.go"},
},
{
path: "/src/fmt/print.go",
match: []string{"// Println formats using"},
path: "/src/fmt/print.go",
contains: []string{"// Println formats using"},
},
{
path: "/pkg",
match: []string{
contains: []string{
"Standard library",
"Package fmt implements formatted I/O",
},
dontmatch: []string{
notContains: []string{
"internal/syscall",
"cmd/gc",
},
},
{
path: "/pkg/?m=all",
match: []string{
contains: []string{
"Standard library",
"Package fmt implements formatted I/O",
"internal/syscall/?m=all",
},
dontmatch: []string{
notContains: []string{
"cmd/gc",
},
},
{
path: "/search?q=ListenAndServe",
match: []string{
contains: []string{
"/src",
},
dontmatch: []string{
notContains: []string{
"/pkg/bootstrap",
},
needIndex: true,
},
{
path: "/pkg/strings/",
match: []string{
contains: []string{
`href="/src/strings/strings.go"`,
},
},
{
path: "/cmd/compile/internal/amd64/",
match: []string{
contains: []string{
`href="/src/cmd/compile/internal/amd64/ssa.go"`,
},
},
{
path: "/pkg/math/bits/",
match: []string{
contains: []string{
`Added in Go 1.9`,
},
},
{
path: "/pkg/net/",
contains: []string{
`// IPv6 scoped addressing zone; added in Go 1.1`,
},
},
{
path: "/pkg/net/http/httptrace/",
match: []string{
`Got1xxResponse.*// Go 1\.11`,
},
},
// Verify we don't add version info to a struct field added the same time
// as the struct itself:
{
path: "/pkg/net/http/httptrace/",
match: []string{
`(?m)GotFirstResponseByte func\(\)\s*$`,
},
},
// Remove trailing periods before adding semicolons:
{
path: "/pkg/database/sql/",
contains: []string{
"The number of connections currently in use; added in Go 1.11",
"The number of idle connections; added in Go 1.11",
},
},
}
for _, test := range tests {
if test.needIndex && !withIndex {
Expand All @@ -338,18 +367,28 @@ func testWeb(t *testing.T, withIndex bool) {
continue
}
body, err := ioutil.ReadAll(resp.Body)
strBody := string(body)
resp.Body.Close()
if err != nil {
t.Errorf("GET %s: failed to read body: %s (response: %v)", url, err, resp)
}
isErr := false
for _, substr := range test.match {
for _, substr := range test.contains {
if !bytes.Contains(body, []byte(substr)) {
t.Errorf("GET %s: wanted substring %q in body", url, substr)
isErr = true
}
}
for _, substr := range test.dontmatch {
for _, re := range test.match {
if ok, err := regexp.MatchString(re, strBody); !ok || err != nil {
if err != nil {
t.Fatalf("Bad regexp %q: %v", re, err)
}
t.Errorf("GET %s: wanted to match %s in body", url, re)
isErr = true
}
}
for _, substr := range test.notContains {
if bytes.Contains(body, []byte(substr)) {
t.Errorf("GET %s: didn't want substring %q in body", url, substr)
isErr = true
Expand Down
90 changes: 84 additions & 6 deletions godoc/godoc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
package godoc // import "golang.org/x/tools/godoc"

import (
"bufio"
"bytes"
"fmt"
"go/ast"
Expand Down Expand Up @@ -188,13 +189,13 @@ func (p *Presentation) infoSnippet_htmlFunc(info SpotInfo) string {

func (p *Presentation) nodeFunc(info *PageInfo, node interface{}) string {
var buf bytes.Buffer
p.writeNode(&buf, info.FSet, node)
p.writeNode(&buf, info, info.FSet, node)
return buf.String()
}

func (p *Presentation) node_htmlFunc(info *PageInfo, node interface{}, linkify bool) string {
var buf1 bytes.Buffer
p.writeNode(&buf1, info.FSet, node)
p.writeNode(&buf1, info, info.FSet, node)

var buf2 bytes.Buffer
if n, _ := node.(ast.Node); n != nil && linkify && p.DeclLinks {
Expand Down Expand Up @@ -891,8 +892,12 @@ func replaceLeadingIndentation(body, oldIndent, newIndent string) string {
return buf.String()
}

// Write an AST node to w.
func (p *Presentation) writeNode(w io.Writer, fset *token.FileSet, x interface{}) {
// writeNode writes the AST node x to w.
//
// The provided fset must be non-nil. The pageInfo is optional. If
// present, the pageInfo is used to add comments to struct fields to
// say which version of Go introduced them.
func (p *Presentation) writeNode(w io.Writer, pageInfo *PageInfo, fset *token.FileSet, x interface{}) {
// convert trailing tabs into spaces using a tconv filter
// to ensure a good outcome in most browsers (there may still
// be tabs in comments and strings, but converting those into
Expand All @@ -901,15 +906,88 @@ func (p *Presentation) writeNode(w io.Writer, fset *token.FileSet, x interface{}
// TODO(gri) rethink printer flags - perhaps tconv can be eliminated
// with an another printer mode (which is more efficiently
// implemented in the printer than here with another layer)

var pkgName, structName string
var apiInfo pkgAPIVersions
if gd, ok := x.(*ast.GenDecl); ok && pageInfo != nil && pageInfo.PDoc != nil &&
p.Corpus != nil &&
gd.Tok == token.TYPE && len(gd.Specs) != 0 {
pkgName = pageInfo.PDoc.ImportPath
if ts, ok := gd.Specs[0].(*ast.TypeSpec); ok {
if _, ok := ts.Type.(*ast.StructType); ok {
structName = ts.Name.Name
}
}
apiInfo = p.Corpus.pkgAPIInfo[pkgName]
}

var out = w
var buf bytes.Buffer
if structName != "" {
out = &buf
}

mode := printer.TabIndent | printer.UseSpaces
err := (&printer.Config{Mode: mode, Tabwidth: p.TabWidth}).Fprint(&tconv{p: p, output: w}, fset, x)
err := (&printer.Config{Mode: mode, Tabwidth: p.TabWidth}).Fprint(&tconv{p: p, output: out}, fset, x)
if err != nil {
log.Print(err)
}

// Add comments to struct fields saying which Go version introducd them.
if structName != "" {
fieldSince := apiInfo.fieldSince[structName]
typeSince := apiInfo.typeSince[structName]
// Add/rewrite comments on struct fields to note which Go version added them.
var buf2 bytes.Buffer
buf2.Grow(buf.Len() + len(" // Added in Go 1.n")*10)
bs := bufio.NewScanner(&buf)
for bs.Scan() {
line := bs.Bytes()
field := firstIdent(line)
var since string
if field != "" {
since = fieldSince[field]
if since != "" && since == typeSince {
// Don't highlight field versions if they were the
// same as the struct itself.
since = ""
}
}
if since == "" {
buf2.Write(line)
} else {
if bytes.Contains(line, slashSlash) {
line = bytes.TrimRight(line, " \t.")
buf2.Write(line)
buf2.WriteString("; added in Go ")
} else {
buf2.Write(line)
buf2.WriteString(" // Go ")
}
buf2.WriteString(since)
}
buf2.WriteByte('\n')
}
w.Write(buf2.Bytes())
}
}

var slashSlash = []byte("//")

// WriteNode writes x to w.
// TODO(bgarcia) Is this method needed? It's just a wrapper for p.writeNode.
func (p *Presentation) WriteNode(w io.Writer, fset *token.FileSet, x interface{}) {
p.writeNode(w, fset, x)
p.writeNode(w, nil, fset, x)
}

// firstIdent returns the first identifier in x.
// This actually parses "identifiers" that begin with numbers too, but we
// never feed it such input, so it's fine.
func firstIdent(x []byte) string {
x = bytes.TrimSpace(x)
i := bytes.IndexFunc(x, func(r rune) bool { return !unicode.IsLetter(r) && !unicode.IsNumber(r) })
if i == -1 {
return string(x)
}
return string(x[:i])
}
2 changes: 1 addition & 1 deletion godoc/snippet.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type Snippet struct {
func (p *Presentation) newSnippet(fset *token.FileSet, decl ast.Decl, id *ast.Ident) *Snippet {
// TODO instead of pretty-printing the node, should use the original source instead
var buf1 bytes.Buffer
p.writeNode(&buf1, fset, decl)
p.writeNode(&buf1, nil, fset, decl)
// wrap text with <pre> tag
var buf2 bytes.Buffer
buf2.WriteString("<pre>")
Expand Down
33 changes: 23 additions & 10 deletions godoc/versions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ type apiVersions map[string]pkgAPIVersions // keyed by Go package ("net/http")
// form "1.1", "1.2", etc.
type pkgAPIVersions struct {
typeSince map[string]string // "Server" -> "1.7"
methodSince map[string]map[string]string // "*Server"->"Shutdown"->1.8
methodSince map[string]map[string]string // "*Server" ->"Shutdown"->1.8
funcSince map[string]string // "NewServer" -> "1.7"
fieldSince map[string]map[string]string // "ClientTrace" -> "Got1xxResponse" -> "1.11"
}

// sinceVersionFunc returns a string (such as "1.7") specifying which Go
Expand Down Expand Up @@ -62,10 +63,11 @@ func (v apiVersions) sinceVersionFunc(kind, receiver, name, pkg string) string {
// versionedRow represents an API feature, a parsed line of a
// $GOROOT/api/go.*txt file.
type versionedRow struct {
pkg string // "net/http"
kind string // "type", "func", "method", TODO: "const", "var"
recv string // for methods, the receiver type ("Server", "*Server")
name string // name of type, func, or method
pkg string // "net/http"
kind string // "type", "func", "method", "field" TODO: "const", "var"
recv string // for methods, the receiver type ("Server", "*Server")
name string // name of type, (struct) field, func, method
structName string // for struct fields, the outer struct name
}

// versionParser parses $GOROOT/api/go*.txt files and stores them in in its rows field.
Expand Down Expand Up @@ -100,6 +102,7 @@ func (vp *versionParser) parseFile(name string) error {
typeSince: make(map[string]string),
methodSince: make(map[string]map[string]string),
funcSince: make(map[string]string),
fieldSince: make(map[string]map[string]string),
}
vp.res[row.pkg] = pkgi
}
Expand All @@ -113,6 +116,11 @@ func (vp *versionParser) parseFile(name string) error {
pkgi.methodSince[row.recv] = make(map[string]string)
}
pkgi.methodSince[row.recv][row.name] = ver
case "field":
if _, ok := pkgi.fieldSince[row.structName]; !ok {
pkgi.fieldSince[row.structName] = make(map[string]string)
}
pkgi.fieldSince[row.structName][row.name] = ver
}
}
return sc.Err()
Expand All @@ -139,18 +147,23 @@ func parseRow(s string) (vr versionedRow, ok bool) {

switch {
case strings.HasPrefix(rest, "type "):
vr.kind = "type"
rest = rest[len("type "):]
sp := strings.IndexByte(rest, ' ')
if sp == -1 {
return
}
vr.name, rest = rest[:sp], rest[sp+1:]
if strings.HasPrefix(rest, "struct, ") {
// TODO: handle struct fields
return
if !strings.HasPrefix(rest, "struct, ") {
vr.kind = "type"
return vr, true
}
rest = rest[len("struct, "):]
if i := strings.IndexByte(rest, ' '); i != -1 {
vr.kind = "field"
vr.structName = vr.name
vr.name = rest[:i]
return vr, true
}
return vr, true
case strings.HasPrefix(rest, "func "):
vr.kind = "func"
rest = rest[len("func "):]
Expand Down
7 changes: 6 additions & 1 deletion godoc/versions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ func TestParseVersionRow(t *testing.T) {
},
{
row: "pkg archive/tar, type Header struct, AccessTime time.Time",
// TODO: implement; for now we expect nothing
want: versionedRow{
pkg: "archive/tar",
kind: "field",
structName: "Header",
name: "AccessTime",
},
},
{
row: "pkg archive/tar, method (*Reader) Read([]uint8) (int, error)",
Expand Down

0 comments on commit 32950ab

Please sign in to comment.