feat(godocfx): add nesting to TOC (#2972)

Author: tbpg

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

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) {

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
+}