From 3a49b2d142a353f98429235c3f380431430b4dbf Mon Sep 17 00:00:00 2001 From: Tyler Bui-Palsulich <26876514+tbpg@users.noreply.github.com> Date: Tue, 6 Oct 2020 12:18:52 -0400 Subject: [PATCH] feat(godocfx): add nesting to TOC (#2972) --- internal/godocfx/go.mod | 1 + internal/godocfx/godocfx_test.go | 43 ++++++++-- internal/godocfx/parse.go | 134 ++++++++++++++++++++++++++----- 3 files changed, 149 insertions(+), 29 deletions(-) diff --git a/internal/godocfx/go.mod b/internal/godocfx/go.mod index 9638e925d72..6e411b338db 100644 --- a/internal/godocfx/go.mod +++ b/internal/godocfx/go.mod @@ -4,6 +4,7 @@ go 1.15 require ( cloud.google.com/go v0.67.0 + cloud.google.com/go/bigquery v1.8.0 cloud.google.com/go/storage v1.11.0 github.com/kr/pretty v0.2.1 // indirect golang.org/x/tools v0.0.0-20201005185003-576e169c3de7 diff --git a/internal/godocfx/godocfx_test.go b/internal/godocfx/godocfx_test.go index d69903a6c76..0088e66222f 100644 --- a/internal/godocfx/godocfx_test.go +++ b/internal/godocfx/godocfx_test.go @@ -18,12 +18,14 @@ package main import ( "flag" + "fmt" "io/ioutil" "os" "path/filepath" "testing" - _ "cloud.google.com/go/storage" // Implicitly required by test. + _ "cloud.google.com/go/bigquery" // Implicitly required by test. + _ "cloud.google.com/go/storage" // Implicitly required by test. ) var updateGoldens bool @@ -35,22 +37,22 @@ func TestMain(m *testing.M) { } func TestParse(t *testing.T) { - testPath := "cloud.google.com/go/storage" - r, err := parse(testPath, nil) + mod := "cloud.google.com/go/bigquery" + r, err := parse(mod+"/...", []string{"README.md"}) if err != nil { t.Fatalf("Parse: %v", err) } if got, want := len(r.toc), 1; got != want { t.Fatalf("Parse got len(toc) = %d, want %d", got, want) } - if got, want := len(r.pages), 1; got != want { + if got, want := len(r.pages), 10; got != want { t.Errorf("Parse got len(pages) = %d, want %d", got, want) } - if got := r.module.Path; got != testPath { - t.Fatalf("Parse got module = %q, want %q", got, testPath) + if got := r.module.Path; got != mod { + t.Fatalf("Parse got module = %q, want %q", got, mod) } - page := r.pages[testPath] + page := r.pages[mod] // Check invariants for every item. for _, item := range page.Items { @@ -63,7 +65,7 @@ func TestParse(t *testing.T) { } } - // Check there is at least one type, const, variable, and function. + // Check there is at least one type, const, variable, function, and method. wants := []string{"type", "const", "variable", "function", "method"} for _, want := range wants { found := false @@ -77,6 +79,31 @@ func TestParse(t *testing.T) { t.Errorf("Parse got no %q, want at least one", want) } } + + foundREADME := false + foundNested := false + foundUnnested := false + for _, item := range r.toc[0].Items { + fmt.Println(item.Name) + if item.Name == "README" { + foundREADME = true + } + if len(item.Items) > 0 { + foundNested = true + } + if len(item.Items) == 0 && len(item.UID) > 0 && len(item.Name) > 0 { + foundUnnested = true + } + } + if !foundREADME { + t.Errorf("Parse didn't find a README in TOC") + } + if !foundNested { + t.Errorf("Parse didn't find a nested element in TOC") + } + if !foundUnnested { + t.Errorf("Parse didn't find an unnested element in TOC (e.g. datatransfer/apiv1)") + } } func TestGoldens(t *testing.T) { diff --git a/internal/godocfx/parse.go b/internal/godocfx/parse.go index cb0e7442d21..703a7789df6 100644 --- a/internal/godocfx/parse.go +++ b/internal/godocfx/parse.go @@ -123,7 +123,6 @@ func parse(glob string, extraFiles []string) (*result, error) { } pages := map[string]*page{} - toc := tableOfContents{} module := pkgs[0].Module skippedModules := map[string]struct{}{} @@ -177,6 +176,8 @@ func parse(glob string, extraFiles []string) (*result, error) { } sort.Strings(pkgNames) + toc := buildTOC(module.Path, pkgNames, extraFiles) + // Once the files are grouped by package, process each package // independently. for _, pkgPath := range pkgNames { @@ -201,26 +202,6 @@ func parse(glob string, extraFiles []string) (*result, error) { continue } - pkgTOCITem := &tocItem{ - UID: docPkg.ImportPath, - Name: docPkg.ImportPath, - } - toc = append(toc, pkgTOCITem) - - // If the package path == module path, add the extra files to the TOC - // as a child of the module==pkg item. - if pkgPath == module.Path { - for _, path := range extraFiles { - base := filepath.Base(path) - name := strings.TrimSuffix(base, filepath.Ext(base)) - name = strings.Title(name) - pkgTOCITem.addItem(&tocItem{ - Href: path, - Name: name, - }) - } - } - pkgItem := &item{ UID: docPkg.ImportPath, Name: docPkg.ImportPath, @@ -412,3 +393,114 @@ func processExamples(exs []*doc.Example, fset *token.FileSet) []example { } return result } + +func buildTOC(mod string, pkgs []string, extraFiles []string) tableOfContents { + toc := tableOfContents{} + + modTOC := &tocItem{ + UID: mod, // Assume the module root has a package. + Name: mod, + } + for _, path := range extraFiles { + base := filepath.Base(path) + name := strings.TrimSuffix(base, filepath.Ext(base)) + name = strings.Title(name) + modTOC.addItem(&tocItem{ + Href: path, + Name: name, + }) + } + + toc = append(toc, modTOC) + + if len(pkgs) == 1 { + // The module only has one package. + return toc + } + + // partialTrie represents the parts of each package name that come after + // the module name. + // + // A package name is three parts: mod/mid/suffix + // + // For example, cloud.google.com/go/bigquery/connection/apiv1 would be split + // into cloud.google.com/go, bigquery, and connection/apiv1. + // + // Don't do a full trie to keep nesting to a minimum. + partialTrie := map[string][]string{} + mids := []string{} + + for _, pkg := range pkgs { + if pkg == mod { + continue + } + if !strings.HasPrefix(pkg, mod) { + panic(fmt.Sprintf("Package %q does not start with %q, should never happen", pkg, mod)) + } + midAndSuffix := strings.TrimPrefix(pkg, mod+"/") + parts := strings.SplitN(midAndSuffix, "/", 2) + + mid := parts[0] + suffix := "" + if len(parts) > 1 { + suffix = parts[1] + } + + if _, ok := partialTrie[mid]; !ok { + mids = append(mids, mid) + } + + partialTrie[mid] = append(partialTrie[mid], suffix) + } + + sort.Strings(mids) + + for _, mid := range mids { + suffixes := partialTrie[mid] + + // No need to nest if there is only one suffix. + if len(suffixes) == 1 { + suffix := suffixes[0] + name := mid + if suffix != "" { + name = name + "/" + suffix + } + uid := mod + "/" + name + pkgTOCItem := &tocItem{ + UID: uid, + Name: name, + } + modTOC.addItem(pkgTOCItem) + continue + } + + sort.Strings(suffixes) + + midTOC := &tocItem{ + // No Uref or UID because this may not be a package. + Name: mid, + } + modTOC.addItem(midTOC) + + for _, suffix := range suffixes { + uid := mod + "/" + mid + if suffix != "" { + uid = uid + "/" + suffix + } + + // Empty suffix means this mid is a package itself. + if suffix == "" { + midTOC.UID = uid + continue + } + + pkgTOC := &tocItem{ + UID: uid, + Name: suffix, + } + midTOC.addItem(pkgTOC) + } + } + + return toc +}