internal/godoc: preserve Scopes
It turns out that ast.Scopes are used to render doc, so preserve them.
Also, tear down the queue after each integration test: because of
retrying, the queue would still be active from previous tests, making
the output confusing for debugging.
Change-Id: I3f3c59d355c0e121e1d8866e621ed830d80e9b9c
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/259717
Trust: Jonathan Amsterdam <jba@google.com>
Trust: Julie Qiu <julie@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/internal/godoc/encode.go b/internal/godoc/encode.go
index 366563e..d0c7c32 100644
--- a/internal/godoc/encode.go
+++ b/internal/godoc/encode.go
@@ -12,6 +12,7 @@
"go/ast"
"go/token"
"io"
+ "sort"
"golang.org/x/pkgsite/internal/derrors"
)
@@ -164,8 +165,6 @@
// in the tree itself. We assign these numbers as well, and store the numbers
// in a separate field of File.
func removeCycles(f *File) {
- f.AST.Scope.Objects = nil // doc doesn't use scopes
-
// First pass: assign every Decl, Spec and Ident a number.
// Since these aren't shared and Inspect is deterministic,
// this walk will produce the same sequence of Decls after encoding/decoding.
@@ -227,6 +226,21 @@
}
}
f.AST.Unresolved = nil
+
+ // Remember only those scope items that have been assigned a number; the others
+ // are not relevant to doc (unexported functions, for instance).
+ f.ScopeItems = nil
+ for name, obj := range f.AST.Scope.Objects {
+ if num, ok := obj.Data.(int); ok {
+ f.ScopeItems = append(f.ScopeItems, scopeItem{name, num})
+ }
+ }
+ // Sort for deterministic encoding.
+ sort.Slice(f.ScopeItems, func(i, j int) bool {
+ return f.ScopeItems[i].Name < f.ScopeItems[j].Name
+ })
+ f.AST.Scope.Objects = nil
+
}
// fixupObjects re-establishes the original Object and Decl relationships of the
@@ -268,7 +282,6 @@
id.Obj = objs[num]
case num == len(objs):
// A new object; fix it up and remember it.
- obj.Data = nil
if obj.Decl != nil {
obj.Decl = decls[obj.Decl.(int)]
}
@@ -285,6 +298,13 @@
f.AST.Unresolved[i] = decls[num].(*ast.Ident)
}
f.UnresolvedNums = nil
+
+ // Fix up file scope objects.
+ f.AST.Scope.Objects = map[string]*ast.Object{}
+ for _, item := range f.ScopeItems {
+ f.AST.Scope.Objects[item.Name] = objs[item.Num]
+ }
+ f.ScopeItems = nil
}
// isRelevantDecl reports whether n is a Node for a declaration relevant to
diff --git a/internal/godoc/encode_test.go b/internal/godoc/encode_test.go
index 28a1b64..d5d315a 100644
--- a/internal/godoc/encode_test.go
+++ b/internal/godoc/encode_test.go
@@ -13,7 +13,7 @@
"testing"
)
-func TestEncodeDecodeASTFiles(t *testing.T) {
+func TestEncodeDecodePackage(t *testing.T) {
// Verify that we can encode and decode the Go files in this directory.
p, err := packageForDir(".", true)
if err != nil {
diff --git a/internal/godoc/godoc.go b/internal/godoc/godoc.go
index f1af28a..eb66d43 100644
--- a/internal/godoc/godoc.go
+++ b/internal/godoc/godoc.go
@@ -32,9 +32,17 @@
// A File contains everything needed about a source file to render documentation.
type File struct {
- Name string // full file pathname relative to zip content directory
- AST *ast.File
- UnresolvedNums []int // used to handle sharing of unresolved identifiers
+ Name string // full file pathname relative to zip content directory
+ AST *ast.File
+ // The following fields are only for encoding and decoding. They are public
+ // only because gob requires them to be. Clients should ignore them.
+ UnresolvedNums []int // used to handle sharing of unresolved identifiers
+ ScopeItems []scopeItem // sorted by name for deterministic encoding
+}
+
+type scopeItem struct {
+ Name string
+ Num int
}
// NewPackage returns a new Package with the given fset and set of module package paths.
diff --git a/internal/testing/integration/frontend_doc_render_test.go b/internal/testing/integration/frontend_doc_render_test.go
index 3c53e2e..dd74ec5 100644
--- a/internal/testing/integration/frontend_doc_render_test.go
+++ b/internal/testing/integration/frontend_doc_render_test.go
@@ -5,9 +5,13 @@
package integration
import (
+ "bufio"
+ "bytes"
"context"
+ "fmt"
"io/ioutil"
"net/http"
+ "strings"
"testing"
"github.com/google/go-cmp/cmp"
@@ -39,8 +43,9 @@
const C = 123
// F is a function.
- func F(t time.Time, s string) T {
- _ = C
+ func F(t time.Time, s string) (T, u) {
+ x := 3
+ x = C
}
`,
"file2.go": `
@@ -48,38 +53,28 @@
var V = C
type T int
+ type u int
`,
},
}
- commonExps := []string{
- internal.ExperimentRemoveUnusedAST,
- internal.ExperimentInsertPackageSource,
- internal.ExperimentSidenav,
- internal.ExperimentUnitPage,
- }
+ // Process with saving the source.
+ processVersions(
+ experiment.NewContext(context.Background(), internal.ExperimentUnitPage, internal.ExperimentRemoveUnusedAST, internal.ExperimentInsertPackageSource),
+ t, []*proxy.Module{m})
- ctx := experiment.NewContext(context.Background(), commonExps...)
- processVersions(ctx, t, []*proxy.Module{m})
-
- getDoc := func(exps ...string) string {
- t.Helper()
-
- ectx := experiment.NewContext(ctx, append(exps, commonExps...)...)
- ts := setupFrontend(ectx, t, nil)
- url := ts.URL + "/" + m.ModulePath
- return getPage(ectx, t, url)
- }
-
- workerDoc := getDoc()
- frontendDoc := getDoc(internal.ExperimentFrontendRenderDoc)
-
+ workerDoc := getDoc(t, m.ModulePath)
+ frontendDoc := getDoc(t, m.ModulePath, internal.ExperimentUnitPage, internal.ExperimentFrontendRenderDoc, internal.ExperimentInsertPackageSource)
if diff := cmp.Diff(workerDoc, frontendDoc); diff != "" {
t.Errorf("mismatch (-worker, +frontend):\n%s", diff)
}
}
-func getPage(ctx context.Context, t *testing.T, url string) string {
+func getDoc(t *testing.T, modulePath string, exps ...string) string {
+ ctx := experiment.NewContext(context.Background(),
+ append(exps, internal.ExperimentSidenav, internal.ExperimentUnitPage)...)
+ ts := setupFrontend(ctx, t, nil)
+ url := ts.URL + "/" + modulePath
resp, err := http.Get(url)
if err != nil {
t.Fatalf("%s: %v", url, err)
@@ -92,5 +87,14 @@
if err != nil {
t.Fatalf("%s: %v", url, err)
}
- return string(content)
+ // Remove surrounding whitespace from lines.
+ scan := bufio.NewScanner(bytes.NewReader(content))
+ var b strings.Builder
+ for scan.Scan() {
+ fmt.Fprintln(&b, strings.TrimSpace(scan.Text()))
+ }
+ if scan.Err() != nil {
+ t.Fatal(scan.Err())
+ }
+ return b.String()
}
diff --git a/internal/testing/integration/frontend_test.go b/internal/testing/integration/frontend_test.go
index c9ac5f2..4aedf81 100644
--- a/internal/testing/integration/frontend_test.go
+++ b/internal/testing/integration/frontend_test.go
@@ -213,13 +213,15 @@
// TODO(https://github.com/golang/go/issues/40098): factor out this code reduce
// duplication
func setupQueue(ctx context.Context, t *testing.T, proxyModules []*proxy.Module, experimentNames ...string) (queue.Queue, func()) {
+ cctx, cancel := context.WithCancel(ctx)
proxyClient, teardown := proxy.SetupTestClient(t, proxyModules)
sourceClient := source.NewClient(1 * time.Second)
- q := queue.NewInMemory(ctx, 1, experimentNames,
- func(ctx context.Context, mpath, version string) (int, error) {
+ q := queue.NewInMemory(cctx, 1, experimentNames,
+ func(ctx context.Context, mpath, version string) (_ int, err error) {
return frontend.FetchAndUpdateState(ctx, mpath, version, proxyClient, sourceClient, testDB)
})
return q, func() {
+ cancel()
teardown()
}
}
