diff --git a/cmd/godoc/godoc_test.go b/cmd/godoc/godoc_test.go index 67760fcb..681818ec 100644 --- a/cmd/godoc/godoc_test.go +++ b/cmd/godoc/godoc_test.go @@ -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 { @@ -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 diff --git a/godoc/godoc.go b/godoc/godoc.go index 1b0d5b93..f191a9d9 100644 --- a/godoc/godoc.go +++ b/godoc/godoc.go @@ -10,6 +10,7 @@ package godoc // import "golang.org/x/tools/godoc" import ( + "bufio" "bytes" "fmt" "go/ast" @@ -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 { @@ -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 @@ -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]) } diff --git a/godoc/snippet.go b/godoc/snippet.go index dd9c8225..17504786 100644 --- a/godoc/snippet.go +++ b/godoc/snippet.go @@ -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
tag var buf2 bytes.Buffer buf2.WriteString("") diff --git a/godoc/versions.go b/godoc/versions.go index e476ab16..8a04905e 100644 --- a/godoc/versions.go +++ b/godoc/versions.go @@ -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 @@ -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. @@ -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 } @@ -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() @@ -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 "):] diff --git a/godoc/versions_test.go b/godoc/versions_test.go index 439fc025..bb408514 100644 --- a/godoc/versions_test.go +++ b/godoc/versions_test.go @@ -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)",