Skip to content

Commit

Permalink
internal/godoc: don't render doc on worker
Browse files Browse the repository at this point in the history
The Package.Render method is only used on the worker to get the
synopsis and other doc-related information. Ever since we started
rendering doc on the frontend, the worker has ignored the rendered
documentation.

So stop rendering the doc, and rename the method to DocInfo to reflect
that.

One minor consequence is that we no longer flag packages with
excessively large doc as having a bad status. As far as the worker is
concerned, they are fine; the error will manifest on the frontend (and
we will serve a "documentation too large" page). This is all good: we
only used a distinct status in this case so we could reprocess if our
limits changed, but these modules no longer need reprocessing.

For golang/go#40850

Change-Id: I3c7c49f0beb7a6d8a37daabdce75f83ef108eddb
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/312270
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
  • Loading branch information
jba committed Apr 21, 2021
1 parent 16c4ace commit 3b55c14
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 33 deletions.
3 changes: 1 addition & 2 deletions internal/fetch/fetchdata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,6 @@ var moduleDocTooLarge = &testModule{
},
},
fr: &FetchResult{
Status: derrors.ToStatus(derrors.HasIncompletePackages),
HasGoMod: false,
GoModPath: "bigdoc.test",
Module: &internal.Module{
Expand Down Expand Up @@ -642,7 +641,7 @@ var moduleDocTooLarge = &testModule{
PackagePath: "bigdoc.test",
ModulePath: "bigdoc.test",
Version: "v1.0.0",
Status: derrors.ToStatus(derrors.PackageDocumentationHTMLTooLarge),
Status: 200,
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions internal/fetch/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ func loadPackageForBuildContext(ctx context.Context, files map[string][]byte, in
return "", nil, "", nil, nil, err
}

synopsis, imports, _, api, err = docPkg.Render(ctx, innerPath, sourceInfo, modInfo)
if err != nil && !errors.Is(err, godoc.ErrTooLarge) {
synopsis, imports, api, err = docPkg.DocInfo(ctx, innerPath, sourceInfo, modInfo)
if err != nil {
return "", nil, "", nil, nil, err
}
return packageName, imports, synopsis, src, api, err
Expand Down
26 changes: 8 additions & 18 deletions internal/godoc/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"path"
"sort"

"github.com/google/safehtml"
"github.com/google/safehtml/template"
"golang.org/x/mod/semver"
"golang.org/x/pkgsite/internal"
Expand Down Expand Up @@ -44,33 +43,24 @@ var MaxDocumentationHTML = 20 * megabyte
type Renderer struct {
}

// Render renders the documentation for the package.
// Rendering destroys p's AST; do not call any methods of p after it returns.
func (p *Package) Render(ctx context.Context, innerPath string, sourceInfo *source.Info, modInfo *ModuleInfo) (
synopsis string, imports []string, html safehtml.HTML, api []*internal.Symbol, err error) {
// DocInfo returns information extracted from the package's documentation.
// This destroys p's AST; do not call any methods of p after it returns.
func (p *Package) DocInfo(ctx context.Context, innerPath string, sourceInfo *source.Info, modInfo *ModuleInfo) (
synopsis string, imports []string, api []*internal.Symbol, err error) {
// This is mostly copied from internal/fetch/fetch.go.
defer derrors.Wrap(&err, "godoc.Package.Render(%q, %q, %q)", modInfo.ModulePath, modInfo.ResolvedVersion, innerPath)
defer derrors.Wrap(&err, "godoc.Package.DocInfo(%q, %q, %q)", modInfo.ModulePath, modInfo.ResolvedVersion, innerPath)

p.renderCalled = true
d, err := p.docPackage(innerPath, modInfo)
if err != nil {
return "", nil, safehtml.HTML{}, nil, err
return "", nil, nil, err
}

api, err = dochtml.GetSymbols(d, p.Fset)
if err != nil {
return "", nil, safehtml.HTML{}, nil, err
}

// Render documentation HTML.
opts := p.renderOptions(innerPath, sourceInfo, modInfo, nil)
docHTML, err := dochtml.Render(ctx, p.Fset, d, opts)
if errors.Is(err, ErrTooLarge) {
docHTML = template.MustParseAndExecuteToHTML(DocTooLargeReplacement)
} else if err != nil {
return "", nil, safehtml.HTML{}, nil, fmt.Errorf("dochtml.Render: %v", err)
return "", nil, nil, err
}
return doc.Synopsis(d.Doc), d.Imports, docHTML, api, err
return doc.Synopsis(d.Doc), d.Imports, api, nil
}

// docPackage computes and returns a doc.Package.
Expand Down
14 changes: 3 additions & 11 deletions internal/godoc/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var (
hasText = htmlcheck.HasText
)

func TestRender(t *testing.T) {
func TestDocInfo(t *testing.T) {
dochtml.LoadTemplates(templateSource)
ctx := context.Background()
si := source.NewGitHubInfo("a.com/M", "", "abcde")
Expand All @@ -43,17 +43,14 @@ func TestRender(t *testing.T) {
t.Fatal(err)
}

wantSyn, wantImports, wantDoc, _, err := p.Render(ctx, "p", si, mi)
wantSyn, wantImports, _, err := p.DocInfo(ctx, "p", si, mi)
if err != nil {
t.Fatal(err)
}
if strings.Contains(wantDoc.String(), "return") {
t.Fatal("doc rendered with function bodies")
}

check := func(p *Package) {
t.Helper()
gotSyn, gotImports, gotDoc, _, err := p.Render(ctx, "p", si, mi)
gotSyn, gotImports, _, err := p.DocInfo(ctx, "p", si, mi)
if err != nil {
t.Fatal(err)
}
Expand All @@ -63,11 +60,6 @@ func TestRender(t *testing.T) {
if !cmp.Equal(gotImports, wantImports) {
t.Errorf("imports: got %v, want %v", gotImports, wantImports)
}
if diff := cmp.Diff(wantDoc.String(), gotDoc.String()); diff != "" {
t.Errorf("doc mismatch (-want, +got):\n%s", diff)
t.Logf("---- want ----\n%s", wantDoc)
t.Logf("---- got ----\n%s", gotDoc)
}
}

// Verify that removing AST nodes doesn't change the doc.
Expand Down

0 comments on commit 3b55c14

Please sign in to comment.