feat(godocfx): include README in output (#2927)

I also changed the return type of parse to be a result type to simplify
passing the... results around.

Related to googleapis/doc-pipeline#39.

Author: tbpg

internal/godocfx/godocfx_test.go

@@ -36,21 +36,21 @@ func TestMain(m *testing.M) {
 
 func TestParse(t *testing.T) {
 	testPath := "cloud.google.com/go/storage"
-	pages, toc, module, err := parse(testPath)
+	r, err := parse(testPath, nil)
 	if err != nil {
 		t.Fatalf("Parse: %v", err)
 	}
-	if got, want := len(toc), 1; got != want {
+	if got, want := len(r.toc), 1; got != want {
 		t.Fatalf("Parse got len(toc) = %d, want %d", got, want)
 	}
-	if got, want := len(pages), 1; got != want {
+	if got, want := len(r.pages), 1; got != want {
 		t.Errorf("Parse got len(pages) = %d, want %d", got, want)
 	}
-	if got := module.Path; got != testPath {
+	if got := r.module.Path; got != testPath {
 		t.Fatalf("Parse got module = %q, want %q", got, testPath)
 	}
 
-	page := pages[testPath]
+	page := r.pages[testPath]
 
 	// Check invariants for every item.
 	for _, item := range page.Items {
@@ -64,8 +64,7 @@ func TestParse(t *testing.T) {
 	}
 
 	// Check there is at least one type, const, variable, and function.
-	// Note: no method because they aren't printed for Namespaces yet.
-	wants := []string{"type", "const", "variable", "function"}
+	wants := []string{"type", "const", "variable", "function", "method"}
 	for _, want := range wants {
 		found := false
 		for _, c := range page.Items {
@@ -83,9 +82,10 @@ func TestParse(t *testing.T) {
 func TestGoldens(t *testing.T) {
 	gotDir := "testdata/out"
 	goldenDir := "testdata/golden"
+	extraFiles := []string{"README.md"}
 
 	testPath := "cloud.google.com/go/storage"
-	pages, toc, module, err := parse(testPath)
+	r, err := parse(testPath, extraFiles)
 	if err != nil {
 		t.Fatalf("parse: %v", err)
 	}
@@ -95,7 +95,7 @@ func TestGoldens(t *testing.T) {
 	if updateGoldens {
 		os.RemoveAll(goldenDir)
 
-		if err := write(goldenDir, pages, toc, module); err != nil {
+		if err := write(goldenDir, r, extraFiles); err != nil {
 			t.Fatalf("write: %v", err)
 		}
 
@@ -110,7 +110,7 @@ func TestGoldens(t *testing.T) {
 		return
 	}
 
-	if err := write(gotDir, pages, toc, module); err != nil {
+	if err := write(gotDir, r, extraFiles); err != nil {
 		t.Fatalf("write: %v", err)
 	}
 

internal/godocfx/main.go

@@ -39,13 +39,13 @@ package main
 import (
 	"flag"
 	"fmt"
+	"io"
 	"log"
 	"os"
 	"path/filepath"
 	"strings"
 	"time"
 
-	"golang.org/x/tools/go/packages"
 	"gopkg.in/yaml.v2"
 )
 
@@ -58,17 +58,20 @@ func main() {
 		log.Fatalf("%s missing required argument: module path", os.Args[0])
 	}
 
-	pages, toc, module, err := parse(flag.Arg(0))
+	extraFiles := []string{
+		"README.md",
+	}
+	r, err := parse(flag.Arg(0), extraFiles)
 	if err != nil {
 		log.Fatal(err)
 	}
 
 	if *print {
-		if err := yaml.NewEncoder(os.Stdout).Encode(pages); err != nil {
+		if err := yaml.NewEncoder(os.Stdout).Encode(r.pages); err != nil {
 			log.Fatal(err)
 		}
 		fmt.Println("----- toc.yaml")
-		if err := yaml.NewEncoder(os.Stdout).Encode(toc); err != nil {
+		if err := yaml.NewEncoder(os.Stdout).Encode(r.toc); err != nil {
 			log.Fatal(err)
 		}
 		return
@@ -78,22 +81,22 @@ func main() {
 		os.RemoveAll(*outDir)
 	}
 
-	if err := write(*outDir, pages, toc, module); err != nil {
+	if err := write(*outDir, r, extraFiles); err != nil {
 		log.Fatalf("write: %v", err)
 	}
 }
 
-func write(outDir string, pages map[string]*page, toc tableOfContents, module *packages.Module) error {
+func write(outDir string, r *result, extraFiles []string) error {
 	if err := os.MkdirAll(outDir, os.ModePerm); err != nil {
 		return fmt.Errorf("os.MkdirAll: %v", err)
 	}
-	for path, p := range pages {
+	for path, p := range r.pages {
 		// Make the module root page the index.
-		if path == module.Path {
+		if path == r.module.Path {
 			path = "index"
 		}
 		// Trim the module path from all other paths.
-		path = strings.TrimPrefix(path, module.Path+"/")
+		path = strings.TrimPrefix(path, r.module.Path+"/")
 		path = filepath.Join(outDir, path+".yml")
 		if err := os.MkdirAll(filepath.Dir(path), os.ModePerm); err != nil {
 			return fmt.Errorf("os.MkdirAll: %v", err)
@@ -115,9 +118,23 @@ func write(outDir string, pages map[string]*page, toc tableOfContents, module *p
 		}
 		defer f.Close()
 		fmt.Fprintln(f, "### YamlMime:TableOfContent")
-		if err := yaml.NewEncoder(f).Encode(toc); err != nil {
+		if err := yaml.NewEncoder(f).Encode(r.toc); err != nil {
+			return err
+		}
+	}
+
+	for _, path := range extraFiles {
+		src, err := os.Open(filepath.Join(r.module.Dir, path))
+		if err != nil {
 			return err
 		}
+		dst, err := os.Create(filepath.Join(outDir, path))
+		if err != nil {
+			return err
+		}
+		if _, err := io.Copy(dst, src); err != nil {
+			return nil
+		}
 	}
 
 	// Write the docuploader docs.metadata file. Not for DocFX.
@@ -145,6 +162,6 @@ func write(outDir string, pages map[string]*page, toc tableOfContents, module *p
 }
 name: %q
 version: %q
-language: "go"`, now.Unix(), now.Nanosecond(), module.Path, module.Version)
+language: "go"`, now.Unix(), now.Nanosecond(), r.module.Path, r.module.Version)
 	return nil
 }

internal/godocfx/parse.go

@@ -26,6 +26,7 @@ import (
 	"go/printer"
 	"go/token"
 	"log"
+	"path/filepath"
 	"sort"
 	"strings"
 
@@ -41,6 +42,7 @@ type tocItem struct {
 	UID   string     `yaml:"uid,omitempty"`
 	Name  string     `yaml:"name,omitempty"`
 	Items []*tocItem `yaml:"items,omitempty"`
+	Href  string     `yaml:"href,omitempty"`
 }
 
 func (t *tocItem) addItem(i *tocItem) {
@@ -92,24 +94,32 @@ func (i *item) addChild(c child) {
 
 var onlyGo = []string{"go"}
 
+type result struct {
+	pages  map[string]*page
+	toc    tableOfContents
+	module *packages.Module
+}
+
 // parse parses the directory into a map of import path -> page and a TOC.
 //
 // glob is the path to parse, usually ending in `...`. glob is passed directly
 // to packages.Load as-is.
-func parse(glob string) (map[string]*page, tableOfContents, *packages.Module, error) {
+//
+// extraFiles is a list of paths relative to the module root to include.
+func parse(glob string, extraFiles []string) (*result, error) {
 	config := &packages.Config{
 		Mode:  packages.NeedName | packages.NeedSyntax | packages.NeedTypes | packages.NeedTypesInfo | packages.NeedModule,
 		Tests: true,
 	}
 
 	pkgs, err := packages.Load(config, glob)
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("packages.Load: %v", err)
+		return nil, fmt.Errorf("packages.Load: %v", err)
 	}
 	packages.PrintErrors(pkgs) // Don't fail everything because of one package.
 
 	if len(pkgs) == 0 {
-		return nil, nil, nil, fmt.Errorf("pattern %q matched 0 packages", glob)
+		return nil, fmt.Errorf("pattern %q matched 0 packages", glob)
 	}
 
 	pages := map[string]*page{}
@@ -122,7 +132,7 @@ func parse(glob string) (map[string]*page, tableOfContents, *packages.Module, er
 
 	// First, collect all of the files grouped by package, including test
 	// packages.
-	allPkgFiles := map[string][]string{}
+	goPkgFiles := map[string][]string{}
 	for _, pkg := range pkgs {
 		id := pkg.ID
 		// See https://pkg.go.dev/golang.org/x/tools/go/packages#Config.
@@ -144,15 +154,15 @@ func parse(glob string) (map[string]*page, tableOfContents, *packages.Module, er
 		for _, f := range pkg.Syntax {
 			name := pkg.Fset.File(f.Pos()).Name()
 			if strings.HasSuffix(name, ".go") {
-				allPkgFiles[id] = append(allPkgFiles[id], name)
+				goPkgFiles[id] = append(goPkgFiles[id], name)
 			}
 		}
 	}
 
 	// Test files don't have Module set. Filter out packages in skipped modules.
 	pkgFiles := map[string][]string{}
 	pkgNames := []string{}
-	for pkgPath, files := range allPkgFiles {
+	for pkgPath, files := range goPkgFiles {
 		skip := false
 		for skipped := range skippedModules {
 			if strings.HasPrefix(pkgPath, skipped) {
@@ -175,26 +185,41 @@ func parse(glob string) (map[string]*page, tableOfContents, *packages.Module, er
 		for _, f := range pkgFiles[pkgPath] {
 			pf, err := parser.ParseFile(fset, f, nil, parser.ParseComments)
 			if err != nil {
-				return nil, nil, nil, fmt.Errorf("ParseFile: %v", err)
+				return nil, fmt.Errorf("ParseFile: %v", err)
 			}
 			parsedFiles = append(parsedFiles, pf)
 		}
 
 		// Parse out GoDoc.
 		docPkg, err := doc.NewFromFiles(fset, parsedFiles, pkgPath)
 		if err != nil {
-			return nil, nil, nil, fmt.Errorf("doc.NewFromFiles: %v", err)
+			return nil, fmt.Errorf("doc.NewFromFiles: %v", err)
 		}
 
 		// Extra filter in case the file filtering didn't catch everything.
 		if !strings.HasPrefix(docPkg.ImportPath, module.Path) {
 			continue
 		}
 
-		toc = append(toc, &tocItem{
+		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,
@@ -345,7 +370,12 @@ func parse(glob string) (map[string]*page, tableOfContents, *packages.Module, er
 		sort.Strings(skipped)
 		log.Printf("Warning: Only processed %s@%s, skipped:\n%s\n", module.Path, module.Version, strings.Join(skipped, "\n"))
 	}
-	return pages, toc, module, nil
+
+	return &result{
+		pages:  pages,
+		toc:    toc,
+		module: module,
+	}, nil
 }
 
 // processExamples converts the examples to []example.

internal/godocfx/testdata/golden/README.md

@@ -0,0 +1,32 @@
+## Cloud Storage [![GoDoc](https://godoc.org/cloud.google.com/go/storage?status.svg)](https://godoc.org/cloud.google.com/go/storage)
+
+- [About Cloud Storage](https://cloud.google.com/storage/)
+- [API documentation](https://cloud.google.com/storage/docs)
+- [Go client documentation](https://godoc.org/cloud.google.com/go/storage)
+- [Complete sample programs](https://github.com/GoogleCloudPlatform/golang-samples/tree/master/storage)
+
+### Example Usage
+
+First create a `storage.Client` to use throughout your application:
+
+[snip]:# (storage-1)
+```go
+client, err := storage.NewClient(ctx)
+if err != nil {
+	log.Fatal(err)
+}
+```
+
+[snip]:# (storage-2)
+```go
+// Read the object1 from bucket.
+rc, err := client.Bucket("bucket").Object("object1").NewReader(ctx)
+if err != nil {
+	log.Fatal(err)
+}
+defer rc.Close()
+body, err := ioutil.ReadAll(rc)
+if err != nil {
+	log.Fatal(err)
+}
+```

internal/godocfx/testdata/golden/toc.yml

@@ -1,3 +1,6 @@
 ### YamlMime:TableOfContent
 - uid: cloud.google.com/go/storage
   name: cloud.google.com/go/storage
+  items:
+  - name: README
+    href: README.md