From f084690a2ea08ce73bafaaced95ad271fd01e11e Mon Sep 17 00:00:00 2001 From: Tyler Bui-Palsulich <26876514+tbpg@users.noreply.github.com> Date: Tue, 29 Sep 2020 10:40:06 -0400 Subject: [PATCH] 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 https://github.com/googleapis/doc-pipeline/pull/39. --- internal/godocfx/godocfx_test.go | 20 ++++----- internal/godocfx/main.go | 39 +++++++++++----- internal/godocfx/parse.go | 52 +++++++++++++++++----- internal/godocfx/testdata/golden/README.md | 32 +++++++++++++ internal/godocfx/testdata/golden/toc.yml | 3 ++ 5 files changed, 114 insertions(+), 32 deletions(-) create mode 100644 internal/godocfx/testdata/golden/README.md diff --git a/internal/godocfx/godocfx_test.go b/internal/godocfx/godocfx_test.go index c344a01df44..d69903a6c76 100644 --- a/internal/godocfx/godocfx_test.go +++ b/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) } diff --git a/internal/godocfx/main.go b/internal/godocfx/main.go index c3eda07e506..c58e13428c0 100644 --- a/internal/godocfx/main.go +++ b/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 } diff --git a/internal/godocfx/parse.go b/internal/godocfx/parse.go index af51a1ab3ed..cb0e7442d21 100644 --- a/internal/godocfx/parse.go +++ b/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,11 +94,19 @@ 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, @@ -104,12 +114,12 @@ func parse(glob string) (map[string]*page, tableOfContents, *packages.Module, er 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,7 +154,7 @@ 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) } } } @@ -152,7 +162,7 @@ func parse(glob string) (map[string]*page, tableOfContents, *packages.Module, er // 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,7 +185,7 @@ 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) } @@ -183,7 +193,7 @@ func parse(glob string) (map[string]*page, tableOfContents, *packages.Module, er // 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. @@ -191,10 +201,25 @@ func parse(glob string) (map[string]*page, tableOfContents, *packages.Module, er 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. diff --git a/internal/godocfx/testdata/golden/README.md b/internal/godocfx/testdata/golden/README.md new file mode 100644 index 00000000000..a2253c4bb5a --- /dev/null +++ b/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) +} +``` \ No newline at end of file diff --git a/internal/godocfx/testdata/golden/toc.yml b/internal/godocfx/testdata/golden/toc.yml index 690eb5c3776..3b7294c60cb 100644 --- a/internal/godocfx/testdata/golden/toc.yml +++ b/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