From 243d24c2a202f182379a72209a3203bc5d082a74 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Mon, 13 Mar 2023 12:32:26 +0100 Subject: [PATCH 01/24] feat(gnodev): add preliminary doc command, refactor common flags verbose and root-dir --- cmd/gnodev/build.go | 13 ++++--------- cmd/gnodev/doc.go | 39 +++++++++++++++++++++++++++++++++++++++ cmd/gnodev/main.go | 2 +- cmd/gnodev/precompile.go | 9 ++------- cmd/gnodev/repl.go | 19 ++++--------------- cmd/gnodev/run.go | 19 ++++--------------- cmd/gnodev/test.go | 18 ++++-------------- cmd/gnodev/util.go | 27 +++++++++++++++++++++++++++ tests/package_test.go | 1 + 9 files changed, 86 insertions(+), 61 deletions(-) create mode 100644 cmd/gnodev/doc.go diff --git a/cmd/gnodev/build.go b/cmd/gnodev/build.go index ae0423ad4d5..9c42a930b62 100644 --- a/cmd/gnodev/build.go +++ b/cmd/gnodev/build.go @@ -11,13 +11,13 @@ import ( ) type buildCfg struct { - verbose bool + verboseStruct goBinary string } var defaultBuildOptions = &buildCfg{ - verbose: false, - goBinary: "go", + verboseStruct: verboseStruct{false}, + goBinary: "go", } func newBuildCmd(io *commands.IO) *commands.Command { @@ -37,12 +37,7 @@ func newBuildCmd(io *commands.IO) *commands.Command { } func (c *buildCfg) RegisterFlags(fs *flag.FlagSet) { - fs.BoolVar( - &c.verbose, - "verbose", - defaultBuildOptions.verbose, - "verbose output when building", - ) + c.verboseStruct.RegisterFlags(fs) fs.StringVar( &c.goBinary, diff --git a/cmd/gnodev/doc.go b/cmd/gnodev/doc.go new file mode 100644 index 00000000000..d45215f6d5a --- /dev/null +++ b/cmd/gnodev/doc.go @@ -0,0 +1,39 @@ +package main + +import ( + "context" + "flag" + + "github.com/gnolang/gno/pkgs/commands" +) + +type docCfg struct { + all bool + src bool + unexported bool + rootDirStruct +} + +func newDocCmd(io *commands.IO) *commands.Command { + c := &docCfg{} + return commands.NewCommand( + commands.Metadata{ + Name: "doc", + ShortUsage: "doc [flags] ", + ShortHelp: "get documentation for the specified package or symbol (type, function, method, or variable/constant).", + LongHelp: "", + }, + c, + func(_ context.Context, args []string) error { + return execDoc(c, args, io) + }, + ) +} + +func (c *docCfg) RegisterFlags(fs *flag.FlagSet) { + c.rootDirStruct.RegisterFlags(fs) +} + +func execDoc(cfg *docCfg, args []string, io *commands.IO) error { + panic("not implemented") +} diff --git a/cmd/gnodev/main.go b/cmd/gnodev/main.go index cbfc44cfa4d..1246b6ad5e6 100644 --- a/cmd/gnodev/main.go +++ b/cmd/gnodev/main.go @@ -35,6 +35,7 @@ func newGnodevCmd(io *commands.IO) *commands.Command { newTestCmd(io), newModCmd(io), newReplCmd(), + newDocCmd(io), // fmt -- gofmt // clean // graph @@ -43,7 +44,6 @@ func newGnodevCmd(io *commands.IO) *commands.Command { // render -- call render()? // publish/release // generate - // doc -- godoc // "vm" -- starts an in-memory chain that can be interacted with? // bug -- start a bug report // version -- show gnodev, golang versions diff --git a/cmd/gnodev/precompile.go b/cmd/gnodev/precompile.go index 12a4458c0eb..6171e08f15e 100644 --- a/cmd/gnodev/precompile.go +++ b/cmd/gnodev/precompile.go @@ -15,7 +15,7 @@ import ( type importPath string type precompileCfg struct { - verbose bool + verboseStruct skipFmt bool skipImports bool goBinary string @@ -64,12 +64,7 @@ func newPrecompileCmd(io *commands.IO) *commands.Command { } func (c *precompileCfg) RegisterFlags(fs *flag.FlagSet) { - fs.BoolVar( - &c.verbose, - "verbose", - false, - "verbose output when running", - ) + c.verboseStruct.RegisterFlags(fs) fs.BoolVar( &c.skipFmt, diff --git a/cmd/gnodev/repl.go b/cmd/gnodev/repl.go index 1a09aac3eef..21bb9739ced 100644 --- a/cmd/gnodev/repl.go +++ b/cmd/gnodev/repl.go @@ -16,8 +16,8 @@ import ( ) type replCfg struct { - verbose bool - rootDir string + verboseStruct + rootDirStruct } func newReplCmd() *commands.Command { @@ -37,19 +37,8 @@ func newReplCmd() *commands.Command { } func (c *replCfg) RegisterFlags(fs *flag.FlagSet) { - fs.BoolVar( - &c.verbose, - "verbose", - false, - "verbose output when running", - ) - - fs.StringVar( - &c.rootDir, - "root-dir", - "", - "clone location of github.com/gnolang/gno (gnodev tries to guess it)", - ) + c.verboseStruct.RegisterFlags(fs) + c.rootDirStruct.RegisterFlags(fs) } func execRepl(cfg *replCfg, args []string) error { diff --git a/cmd/gnodev/run.go b/cmd/gnodev/run.go index 8155dfd6305..ede45863711 100644 --- a/cmd/gnodev/run.go +++ b/cmd/gnodev/run.go @@ -10,8 +10,8 @@ import ( ) type runCfg struct { - verbose bool - rootDir string + verboseStruct + rootDirStruct } func newRunCmd(io *commands.IO) *commands.Command { @@ -31,19 +31,8 @@ func newRunCmd(io *commands.IO) *commands.Command { } func (c *runCfg) RegisterFlags(fs *flag.FlagSet) { - fs.BoolVar( - &c.verbose, - "verbose", - false, - "verbose output when running", - ) - - fs.StringVar( - &c.rootDir, - "root-dir", - "", - "clone location of github.com/gnolang/gno (gnodev tries to guess it)", - ) + c.verboseStruct.RegisterFlags(fs) + c.rootDirStruct.RegisterFlags(fs) } func execRun(cfg *runCfg, args []string, io *commands.IO) error { diff --git a/cmd/gnodev/test.go b/cmd/gnodev/test.go index d2e66710ea3..8b398240ee6 100644 --- a/cmd/gnodev/test.go +++ b/cmd/gnodev/test.go @@ -24,8 +24,8 @@ import ( ) type testCfg struct { - verbose bool - rootDir string + verboseStruct + rootDirStruct run string timeout time.Duration precompile bool // TODO: precompile should be the default, but it needs to automatically precompile dependencies in memory. @@ -49,12 +49,7 @@ func newTestCmd(io *commands.IO) *commands.Command { } func (c *testCfg) RegisterFlags(fs *flag.FlagSet) { - fs.BoolVar( - &c.verbose, - "verbose", - false, - "verbose output when running", - ) + c.verboseStruct.RegisterFlags(fs) fs.BoolVar( &c.precompile, @@ -70,12 +65,7 @@ func (c *testCfg) RegisterFlags(fs *flag.FlagSet) { "writes actual as wanted in test comments", ) - fs.StringVar( - &c.rootDir, - "root-dir", - "", - "clone location of github.com/gnolang/gno (gnodev tries to guess it)", - ) + c.rootDirStruct.RegisterFlags(fs) fs.StringVar( &c.run, diff --git a/cmd/gnodev/util.go b/cmd/gnodev/util.go index c514ae8d6f3..093167f0140 100644 --- a/cmd/gnodev/util.go +++ b/cmd/gnodev/util.go @@ -1,6 +1,7 @@ package main import ( + "flag" "fmt" "go/ast" "io" @@ -15,6 +16,32 @@ import ( gno "github.com/gnolang/gno/pkgs/gnolang" ) +type rootDirStruct struct { + rootDir string +} + +func (c *rootDirStruct) RegisterFlags(fs *flag.FlagSet) { + fs.StringVar( + &c.rootDir, + "root-dir", + "", + "clone location of github.com/gnolang/gno (gnodev tries to guess it)", + ) +} + +type verboseStruct struct { + verbose bool +} + +func (c *verboseStruct) RegisterFlags(fs *flag.FlagSet) { + fs.BoolVar( + &c.verbose, + "verbose", + false, + "print verbose output", + ) +} + func isGnoFile(f fs.DirEntry) bool { name := f.Name() return !strings.HasPrefix(name, ".") && strings.HasSuffix(name, ".gno") && !f.IsDir() diff --git a/tests/package_test.go b/tests/package_test.go index 5420a0ed881..d2c06e87d6e 100644 --- a/tests/package_test.go +++ b/tests/package_test.go @@ -47,6 +47,7 @@ func TestPackages(t *testing.T) { } // Sort pkgPaths for determinism. sort.Strings(pkgPaths) + t.Log(pkgPaths) // For each package with testfiles (in testDirs), call Machine.TestMemPackage. for _, pkgPath := range pkgPaths { testDir := testDirs[pkgPath] From 34e7a952bc4d7df3367edee2f8bdc81f8d785989 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Fri, 17 Mar 2023 13:03:59 +0100 Subject: [PATCH 02/24] feat(gnodev): initial documentation command for stdlibs packages --- cmd/gnodev/doc.go | 47 +- pkgs/commands/doc/dirs.go | 153 ++++++ pkgs/commands/doc/doc.go | 350 +++++++++++++ pkgs/commands/doc/pkg.go | 200 ++++++++ pkgs/commands/doc/print.go | 975 +++++++++++++++++++++++++++++++++++++ 5 files changed, 1723 insertions(+), 2 deletions(-) create mode 100644 pkgs/commands/doc/dirs.go create mode 100644 pkgs/commands/doc/doc.go create mode 100644 pkgs/commands/doc/pkg.go create mode 100644 pkgs/commands/doc/print.go diff --git a/cmd/gnodev/doc.go b/cmd/gnodev/doc.go index d45215f6d5a..32f34a2daed 100644 --- a/cmd/gnodev/doc.go +++ b/cmd/gnodev/doc.go @@ -3,8 +3,10 @@ package main import ( "context" "flag" + "path/filepath" "github.com/gnolang/gno/pkgs/commands" + "github.com/gnolang/gno/pkgs/commands/doc" ) type docCfg struct { @@ -21,7 +23,6 @@ func newDocCmd(io *commands.IO) *commands.Command { Name: "doc", ShortUsage: "doc [flags] ", ShortHelp: "get documentation for the specified package or symbol (type, function, method, or variable/constant).", - LongHelp: "", }, c, func(_ context.Context, args []string) error { @@ -31,9 +32,51 @@ func newDocCmd(io *commands.IO) *commands.Command { } func (c *docCfg) RegisterFlags(fs *flag.FlagSet) { + fs.BoolVar( + &c.all, + "all", + false, + "show documentation for all symbols in package", + ) + + fs.BoolVar( + &c.src, + "src", + false, + "show source code for symbols", + ) + + fs.BoolVar( + &c.unexported, + "u", + false, + "show unexported symbols as well as exported", + ) + c.rootDirStruct.RegisterFlags(fs) } func execDoc(cfg *docCfg, args []string, io *commands.IO) error { - panic("not implemented") + // guess opts.RootDir + if cfg.rootDir == "" { + cfg.rootDir = guessRootDir() + } + dirs := doc.NewDirs(filepath.Join(cfg.rootDir, "stdlibs")) + res, err := doc.ResolveDocumentable(dirs, args, cfg.unexported) + switch { + case res == nil: + return err + case err != nil: + io.Printfln("warning: error parsing some candidate packages:\n%v", err) + } + output, err := res.Document( + doc.WithShowAll(cfg.all), + doc.WithSource(cfg.src), + doc.WithUnexported(cfg.unexported), + ) + if err != nil { + return err + } + io.Out.Write([]byte(output)) + return nil } diff --git a/pkgs/commands/doc/dirs.go b/pkgs/commands/doc/dirs.go new file mode 100644 index 00000000000..af282b66450 --- /dev/null +++ b/pkgs/commands/doc/dirs.go @@ -0,0 +1,153 @@ +// Mostly copied from go source at tip, commit d922c0a. +// +// Copyright 2015 The Go Authors. All rights reserved. + +package doc + +import ( + "log" + "os" + "path/filepath" + "sort" + "strings" +) + +// A Dir describes a directory holding code by specifying +// the expected import path and the file system directory. +type Dir struct { + importPath string // import path for that dir + dir string // file system directory +} + +// Dirs is a structure for scanning the directory tree. +// Its Next method returns the next Go source directory it finds. +// Although it can be used to scan the tree multiple times, it +// only walks the tree once, caching the data it finds. +type Dirs struct { + scan chan Dir // Directories generated by walk. + hist []Dir // History of reported Dirs. + offset int // Counter for Next. +} + +// NewDirs begins scanning the given stdlibs directory. +func NewDirs(stdlibsDir string) *Dirs { + d := &Dirs{ + hist: make([]Dir, 0, 256), + scan: make(chan Dir), + } + go d.walk([]string{stdlibsDir}) + return d +} + +// Reset puts the scan back at the beginning. +func (d *Dirs) Reset() { + d.offset = 0 +} + +// Next returns the next directory in the scan. The boolean +// is false when the scan is done. +func (d *Dirs) Next() (Dir, bool) { + if d.offset < len(d.hist) { + dir := d.hist[d.offset] + d.offset++ + return dir, true + } + dir, ok := <-d.scan + if !ok { + return Dir{}, false + } + d.hist = append(d.hist, dir) + d.offset++ + return dir, ok +} + +// walk walks the trees in the given roots. +func (d *Dirs) walk(roots []string) { + for _, root := range roots { + d.bfsWalkRoot(root) + } + close(d.scan) +} + +// bfsWalkRoot walks a single directory hierarchy in breadth-first lexical order. +// Each Go source directory it finds is delivered on d.scan. +func (d *Dirs) bfsWalkRoot(root string) { + root = filepath.Clean(root) + + // this is the queue of directories to examine in this pass. + this := []string{} + // next is the queue of directories to examine in the next pass. + next := []string{root} + + for len(next) > 0 { + this, next = next, this[:0] + for _, dir := range this { + fd, err := os.Open(dir) + if err != nil { + log.Print(err) + continue + } + entries, err := fd.Readdir(0) + fd.Close() + if err != nil { + log.Print(err) + continue + } + hasGnoFiles := false + for _, entry := range entries { + name := entry.Name() + // For plain files, remember if this directory contains any .gno + // source files, but ignore them otherwise. + if !entry.IsDir() { + if !hasGnoFiles && strings.HasSuffix(name, ".gno") { + hasGnoFiles = true + } + continue + } + // Entry is a directory. + + // Ignore same directories ignored by the go tool. + if name[0] == '.' || name[0] == '_' || name == "testdata" { + continue + } + // Remember this (fully qualified) directory for the next pass. + next = append(next, filepath.Join(dir, name)) + } + if hasGnoFiles { + // It's a candidate. + var importPath string + if len(dir) > len(root) { + if importPath != "" { + importPath += "/" + } + importPath += filepath.ToSlash(dir[len(root)+1:]) + } + d.scan <- Dir{importPath, dir} + } + } + } +} + +// findPackage finds a package iterating over d where the import path has +// name as a suffix (which may be a package name or a fully-qualified path). +// returns a list of possible directories. If a directory's import path matched +// exactly, it will be returned as first. +func (d *Dirs) findPackage(name string) []Dir { + d.Reset() + candidates := make([]Dir, 0, 4) + for dir, ok := d.Next(); ok; dir, ok = d.Next() { + if strings.HasSuffix(dir.importPath, name) { + candidates = append(candidates, dir) + } + } + sort.Slice(candidates, func(i, j int) bool { + // prefer exact matches with name + if candidates[i].importPath == name { + return true + } else if candidates[j].importPath == name { + return false + } + return candidates[i].importPath < candidates[j].importPath + }) + return candidates +} diff --git a/pkgs/commands/doc/doc.go b/pkgs/commands/doc/doc.go new file mode 100644 index 00000000000..1a213192566 --- /dev/null +++ b/pkgs/commands/doc/doc.go @@ -0,0 +1,350 @@ +// Package doc implements support for documentation of Gno packages and realms, +// in a similar fashion to `go doc`. +// As a reference, the [official implementation] for `go doc` is used. +// +// [official implementation]: https://github.com/golang/go/tree/90dde5dec1126ddf2236730ec57511ced56a512d/src/cmd/doc +package doc + +import ( + "bytes" + "fmt" + "go/ast" + "go/doc" + "go/token" + "io" + "strings" + + "go.uber.org/multierr" +) + +// DocumentOption is used to pass options to the [Documentable].Document. +type DocumentOption func(s *documentOptions) + +type documentOptions struct { + all bool + src bool + unexported bool + short bool + w io.Writer +} + +// WithShowAll shows all symbols when displaying documentation about a package. +func WithShowAll(b bool) DocumentOption { + return func(s *documentOptions) { s.all = b } +} + +// WithSource shows source when documenting a symbol. +func WithSource(b bool) DocumentOption { + return func(s *documentOptions) { s.src = b } +} + +// WithUnexported shows unexported symbols as well as exported. +func WithUnexported(b bool) DocumentOption { + return func(s *documentOptions) { s.unexported = b } +} + +// WithShort shows a one-line representation for each symbol. +func WithShort(b bool) DocumentOption { + return func(s *documentOptions) { s.short = b } +} + +// Documentable is a package, symbol, or accessible which can be documented. +type Documentable interface { + Document(...DocumentOption) (string, error) +} + +type documentable struct { + Dir + symbol string + accessible string + pkgData *pkgData +} + +func (d *documentable) Document(opts ...DocumentOption) (string, error) { + buf := new(bytes.Buffer) + o := &documentOptions{w: buf} + for _, opt := range opts { + opt(o) + } + + var err error + if d.pkgData == nil { + d.pkgData, err = newPkgData(d.Dir, o.unexported) + if err != nil { + return "", err + } + } + + astpkg, pkg, err := d.pkgData.docPackage(o) + if err != nil { + return "", err + } + + typedValue := make(map[*doc.Value]bool) + constructor := make(map[*doc.Func]bool) + for _, typ := range pkg.Types { + pkg.Consts = append(pkg.Consts, typ.Consts...) + pkg.Vars = append(pkg.Vars, typ.Vars...) + pkg.Funcs = append(pkg.Funcs, typ.Funcs...) + if o.unexported || token.IsExported(typ.Name) { + for _, value := range typ.Consts { + typedValue[value] = true + } + for _, value := range typ.Vars { + typedValue[value] = true + } + for _, fun := range typ.Funcs { + // We don't count it as a constructor bound to the type + // if the type itself is not exported. + constructor[fun] = true + } + } + } + + pp := &pkgPrinter{ + name: d.pkgData.name, + pkg: astpkg, + file: ast.MergePackageFiles(astpkg, 0), + doc: pkg, + typedValue: typedValue, + constructor: constructor, + fs: d.pkgData.fset, + opt: o, + importPath: d.importPath, + } + pp.buf.pkg = pp + + err = d.output(pp) + return buf.String(), err +} + +func (d *documentable) output(pp *pkgPrinter) (err error) { + defer func() { + pp.flush() + if err == nil { + err = pp.err + } + }() + + switch { + case d.symbol == "" && d.accessible == "": + if pp.opt.all { + pp.allDoc() + return + } + pp.packageDoc() + case d.symbol == "" && d.accessible != "": + d.symbol, d.accessible = d.accessible, "" + fallthrough + case d.symbol != "" && d.accessible == "": + pp.symbolDoc(d.symbol) + default: // both non-empty + if pp.methodDoc(d.symbol, d.accessible) { + return + } + if pp.fieldDoc(d.symbol, d.accessible) { + return + } + } + + return +} + +// ResolveDocumentable returns a Documentable from the given arguments. +// Refer to the documentation of gnodev doc for the formats accepted (in general +// the same as the go doc command). +func ResolveDocumentable(dirs *Dirs, args []string, unexported bool) (Documentable, error) { + parsed := parseArgParts(args) + if parsed == nil { + return nil, fmt.Errorf("commands/doc: invalid arguments: %v", args) + } + + var candidates []Dir + + // if we have a candidate package name, search dirs for a dir that matches it. + // prefer directories whose import path match precisely the package + if parsed[0].typ&argPkg > 0 { + if strings.Contains(parsed[0].val, ".") { + panic("local packages not yet supported") + } + candidates = dirs.findPackage(parsed[0].val) + // easy case: we wanted documentation about a package, and we found one! + if len(parsed) == 1 && len(candidates) > 0 { + return &documentable{Dir: candidates[0]}, nil + } + if len(candidates) == 0 { + // there are no candidates. + // if this can be something other than a package, remove argPkg as an + // option, otherwise return not found. + if parsed[0].typ == argPkg { + return nil, fmt.Errorf("commands/doc: package not found: %q", parsed[0].val) + } + parsed[0].typ &= ^argPkg + } else { + // if there are candidates, then the first argument was definitely + // a package. remove it from parsed so we don't worry about it again. + parsed = parsed[1:] + } + } + + // we also (or only) have a symbol/accessible. + // search for the symbol through the candidates + if len(candidates) == 0 { + // no candidates means local directory here + panic("local packages not yet supported") + } + + doc := &documentable{} + + var matchFunc func(s string) bool + if len(parsed) == 2 { + // assert that we have and + if parsed[0].typ&argSym == 0 || parsed[1].typ&argAcc == 0 { + panic(fmt.Errorf("invalid remaining parsed: %+v", parsed)) + } + doc.symbol = parsed[0].val + doc.accessible = parsed[1].val + matchFunc = func(s string) bool { return s == parsed[0].val+"."+parsed[1].val } + } else { + switch parsed[0].typ { + case argSym: + doc.symbol = parsed[0].val + matchFunc = func(s string) bool { return s == parsed[0].val } + case argAcc: + doc.accessible = parsed[0].val + matchFunc = func(s string) bool { return strings.HasSuffix(s, "."+parsed[0].val) } + case argSym | argAcc: + matchFunc = func(s string) bool { + switch { + case s == parsed[0].val: + doc.symbol = parsed[0].val + return true + case strings.HasSuffix(s, "."+parsed[0].val): + doc.accessible = parsed[0].val + return true + } + return false + } + default: + panic(fmt.Errorf("invalid remaining parsed: %+v", parsed)) + } + } + + var errs []error + for _, candidate := range candidates { + pd, err := newPkgData(candidate, unexported) + if err != nil { + // silently ignore errors - + // likely invalid AST in a file. + errs = append(errs, err) + continue + } + for _, sym := range pd.symbols { + if matchFunc(sym) { + doc.Dir = candidate + doc.pkgData = pd + // match found. return this as documentable. + return doc, multierr.Combine(errs...) + } + } + } + return nil, multierr.Append( + fmt.Errorf("commands/doc: could not resolve arguments: %v", args), + multierr.Combine(errs...), + ) +} + +// these are used to specify the type of argPart. +const ( + // ie. "crypto/cipher". + argPkg byte = 1 << iota + // ie. "TrimSuffix", "Builder" + argSym + // ie. "WriteString". method or field of a type ("accessible", from the + // word "accessor") + argAcc +) + +// argPart contains the value of the argument, together with the type of value +// it could be, through the flags argPkg, argSym and argAcc. +type argPart struct { + val string + typ byte +} + +func parseArgParts(args []string) []argPart { + parsed := make([]argPart, 0, 3) + switch len(args) { + case 0: + parsed = append(parsed, argPart{val: ".", typ: argPkg}) + case 1: + // allowed syntaxes (acc is method or field, [] marks optional): + // + // [.][.] + // [.][.] + // if the (part) argument contains a slash, then it is most certainly + // a pkg. + // note: pkg can be a relative path. this is mostly problematic for ".." and + // ".". so we count full stops from the last slash. + slash := strings.LastIndexByte(args[0], '/') + if args[0] == "." || args[0] == ".." || + (slash != -1 && args[0][slash+1:] == "..") { + // special handling for common ., .. and ./.. + // these will generally work poorly if you try to use the one-argument + // syntax to access a symbol/accessible. + parsed = append(parsed, argPart{val: args[0], typ: argPkg}) + break + } + switch strings.Count(args[0][slash+1:], ".") { + case 0: + t := argPkg | argSym | argAcc + if strings.IndexByte(args[0], '/') != -1 { + t = argPkg + } + parsed = append(parsed, argPart{args[0], t}) + case 1: + pos := strings.IndexByte(args[0], '.') + // pkg.sym, pkg.acc, sym.acc + t1, t2 := argPkg|argSym, argSym|argAcc + if strings.IndexByte(args[0][:pos], '/') != -1 { + t1 = argPkg + } else if token.IsExported(args[0]) { + // See rationale here: + // https://github.com/golang/go/blob/90dde5dec1126ddf2236730ec57511ced56a512d/src/cmd/doc/main.go#L265 + t1, t2 = argSym, argAcc + } + parsed = append(parsed, + argPart{args[0][:pos], t1}, + argPart{args[0][pos+1:], t2}, + ) + case 2: + // pkg.sym.acc + parts := strings.Split(args[0], ".") + parsed = append(parsed, + argPart{parts[0], argPkg}, + argPart{parts[1], argSym}, + argPart{parts[2], argAcc}, + ) + default: + return nil + } + case 2: + // , , . + parsed = append(parsed, argPart{args[0], argPkg}) + switch strings.Count(args[1], ".") { + case 0: + parsed = append(parsed, argPart{args[1], argSym | argAcc}) + case 1: + pos := strings.IndexByte(args[1], '.') + parsed = append(parsed, + argPart{args[1][:pos], argSym}, + argPart{args[1][pos+1:], argAcc}, + ) + default: + return nil + } + default: + return nil + } + return parsed +} diff --git a/pkgs/commands/doc/pkg.go b/pkgs/commands/doc/pkg.go new file mode 100644 index 00000000000..2ffdacd69a6 --- /dev/null +++ b/pkgs/commands/doc/pkg.go @@ -0,0 +1,200 @@ +package doc + +import ( + "fmt" + "go/ast" + "go/doc" + "go/parser" + "go/token" + "os" + "path/filepath" + "strings" +) + +type pkgData struct { + name string + dir Dir + fset *token.FileSet + files []*ast.File + testFiles []*ast.File + symbols []string +} + +func newPkgData(dir Dir, unexported bool) (*pkgData, error) { + files, err := os.ReadDir(dir.dir) + if err != nil { + return nil, fmt.Errorf("commands/doc: open %q: %w", dir.dir, err) + } + pkg := &pkgData{ + dir: dir, + fset: token.NewFileSet(), + } + for _, file := range files { + n := file.Name() + // Ignore files with prefix . or _ like go tools do. + // Ignore _filetest.gno, but not _test.gno, as we use those to compute + // examples. + if file.IsDir() || + !strings.HasSuffix(n, ".gno") || + strings.HasPrefix(n, ".") || + strings.HasPrefix(n, "_") || + strings.HasSuffix(n, "_filetest.gno") { + continue + } + fullPath := filepath.Join(dir.dir, n) + err := pkg.parseFile(fullPath, unexported) + if err != nil { + return nil, fmt.Errorf("commands/doc: parse file %q: %w", fullPath, err) + } + } + + if len(pkg.files) == 0 { + return nil, fmt.Errorf("commands/doc: no valid gno files in %q", dir.dir) + } + pkgName := pkg.files[0].Name.Name + for _, file := range pkg.files[1:] { + if file.Name.Name != pkgName { + return nil, fmt.Errorf("commands/doc: multiple packages (%q / %q) in dir %q", pkgName, file.Name.Name, dir.dir) + } + } + pkg.name = pkgName + + return pkg, nil +} + +func (pkg *pkgData) parseFile(fileName string, unexported bool) error { + f, err := os.Open(fileName) + if err != nil { + return err + } + defer f.Close() + astf, err := parser.ParseFile(pkg.fset, filepath.Base(fileName), f, parser.ParseComments) + if err != nil { + return err + } + if strings.HasSuffix(fileName, "_test.gno") { + // add test files separately - we should not add their symbols to the package. + pkg.testFiles = append(pkg.testFiles, astf) + return nil + } + pkg.files = append(pkg.files, astf) + + // add symbols + for _, decl := range astf.Decls { + switch x := decl.(type) { + case *ast.FuncDecl: + // prepend receiver if this is a method + var rcv string + if x.Recv != nil { + rcv = typeExprString(x.Recv.List[0].Type) + "." + if !unexported && !token.IsExported(rcv) { + continue + } + } + pkg.symbols = append(pkg.symbols, rcv+x.Name.Name) + case *ast.GenDecl: + for _, spec := range x.Specs { + pkg.appendSpec(spec, unexported) + } + } + } + return nil +} + +func (pkg *pkgData) appendSpec(spec ast.Spec, unexported bool) { + switch s := spec.(type) { + case *ast.TypeSpec: + if !unexported && !token.IsExported(s.Name.Name) { + return + } + pkg.symbols = append(pkg.symbols, s.Name.Name) + switch st := s.Type.(type) { + case *ast.StructType: + pkg.appendFieldList(s.Name.Name, st.Fields) + case *ast.InterfaceType: + pkg.appendFieldList(s.Name.Name, st.Methods) + } + case *ast.ValueSpec: + for _, name := range s.Names { + if !unexported && !token.IsExported(name.Name) { + continue + } + pkg.symbols = append(pkg.symbols, name.Name) + } + } +} + +func (pkg *pkgData) appendFieldList(tName string, fl *ast.FieldList) { + if fl == nil { + return + } + for _, field := range fl.List { + if field.Names == nil { + // embedded struct + pkg.symbols = append(pkg.symbols, tName+"."+typeExprString(field.Type)) + continue + } + for _, name := range field.Names { + pkg.symbols = append(pkg.symbols, tName+"."+name.Name) + } + } +} + +func typeExprString(expr ast.Expr) string { + if expr == nil { + return "" + } + + switch t := expr.(type) { + case *ast.Ident: + return t.Name + case *ast.StarExpr: + return typeExprString(t.X) + } + return "" +} + +func (pkg *pkgData) docPackage(opts *documentOptions) (*ast.Package, *doc.Package, error) { + // largely taken from go/doc.NewFromFiles source + + // Collect .gno files in a map for ast.NewPackage. + fileMap := make(map[string]*ast.File) + for i, file := range pkg.files { + f := pkg.fset.File(file.Pos()) + if f == nil { + return nil, nil, fmt.Errorf("commands/doc: file pkg.files[%d] is not found in the provided file set", i) + } + fileMap[f.Name()] = file + } + + // from cmd/doc/pkg.go: + // go/doc does not include typed constants in the constants + // list, which is what we want. For instance, time.Sunday is of type + // time.Weekday, so it is defined in the type but not in the + // Consts list for the package. This prevents + // go doc time.Sunday + // from finding the symbol. This is why we always have AllDecls. + mode := doc.AllDecls + if opts.src { + mode |= doc.PreserveAST + } + + // Compute package documentation. + // Assign to blank to ignore errors that can happen due to unresolved identifiers. + astpkg, _ := ast.NewPackage(pkg.fset, fileMap, simpleImporter, nil) + p := doc.New(astpkg, pkg.dir.importPath, mode) + // TODO: classifyExamples(p, Examples(testGoFiles...)) + + return astpkg, p, nil +} + +func simpleImporter(imports map[string]*ast.Object, path string) (*ast.Object, error) { + pkg := imports[path] + if pkg == nil { + // note that strings.LastIndex returns -1 if there is no "/" + pkg = ast.NewObj(ast.Pkg, path[strings.LastIndex(path, "/")+1:]) + pkg.Data = ast.NewScope(nil) // required by ast.NewPackage for dot-import + imports[path] = pkg + } + return pkg, nil +} diff --git a/pkgs/commands/doc/print.go b/pkgs/commands/doc/print.go new file mode 100644 index 00000000000..2da1c23da8e --- /dev/null +++ b/pkgs/commands/doc/print.go @@ -0,0 +1,975 @@ +// Copyright 2015 The Go Authors. All rights reserved. +// Copied and modified from Go source: cmd/doc/pkg.go +// Modifications done include: +// - Removing code for supporting documenting commands +// - Removing code for supporting import commands + +package doc + +import ( + "bufio" + "bytes" + "fmt" + "go/ast" + "go/doc" + "go/format" + "go/printer" + "go/token" + "io" + "log" + "strings" + "unicode" + "unicode/utf8" +) + +const ( + punchedCardWidth = 80 + indent = " " +) + +type pkgPrinter struct { + name string // Package name, json for encoding/json. + pkg *ast.Package // Parsed package. + file *ast.File // Merged from all files in the package + doc *doc.Package + typedValue map[*doc.Value]bool // Consts and vars related to types. + constructor map[*doc.Func]bool // Constructors. + fs *token.FileSet // Needed for printing. + buf pkgBuffer + opt *documentOptions + importPath string + + // this is set when an error should be returned up the call chain; + // ie. it is set in the places where the cmd/go code would panic. + err error +} + +func (p *pkgPrinter) isExported(name string) bool { + // cmd/doc uses a global here, so we change this to be a method. + return p.opt.unexported || token.IsExported(name) +} + +func (p *pkgPrinter) ToText(w io.Writer, text, prefix, codePrefix string) { + d := p.doc.Parser().Parse(text) + pr := p.doc.Printer() + pr.TextPrefix = prefix + pr.TextCodePrefix = codePrefix + w.Write(pr.Text(d)) +} + +// pkgBuffer is a wrapper for bytes.Buffer that prints a package clause the +// first time Write is called. +type pkgBuffer struct { + pkg *pkgPrinter + printed bool // Prevent repeated package clauses. + bytes.Buffer +} + +func (pb *pkgBuffer) Write(p []byte) (int, error) { + pb.packageClause() + return pb.Buffer.Write(p) +} + +func (pb *pkgBuffer) packageClause() { + if !pb.printed { + pb.printed = true + pb.pkg.packageClause() + } +} + +// in cmd/go, pkg.Fatalf is like log.Fatalf, but panics so it can be recovered in the +// main do function, so it doesn't cause an exit. Allows testing to work +// without running a subprocess. +// For our purposes, we store the error in .err - the caller knows about this and will check it. +func (pkg *pkgPrinter) Fatalf(format string, args ...any) { + pkg.err = fmt.Errorf(format, args...) +} + +func (pkg *pkgPrinter) Printf(format string, args ...any) { + fmt.Fprintf(&pkg.buf, format, args...) +} + +func (pkg *pkgPrinter) flush() { + _, err := pkg.opt.w.Write(pkg.buf.Bytes()) + if err != nil { + log.Fatal(err) + } + pkg.buf.Reset() // Not needed, but it's a flush. +} + +var newlineBytes = []byte("\n\n") // We never ask for more than 2. + +// newlines guarantees there are n newlines at the end of the buffer. +func (pkg *pkgPrinter) newlines(n int) { + for !bytes.HasSuffix(pkg.buf.Bytes(), newlineBytes[:n]) { + pkg.buf.WriteRune('\n') + } +} + +// emit prints the node. If pkg.opt.src is true, it ignores the provided comment, +// assuming the comment is in the node itself. Otherwise, the go/doc package +// clears the stuff we don't want to print anyway. It's a bit of a magic trick. +func (pkg *pkgPrinter) emit(comment string, node ast.Node) { + if node != nil { + var arg any = node + if pkg.opt.src { + // Need an extra little dance to get internal comments to appear. + arg = &printer.CommentedNode{ + Node: node, + Comments: pkg.file.Comments, + } + } + err := format.Node(&pkg.buf, pkg.fs, arg) + if err != nil { + log.Fatal(err) + } + if comment != "" && !pkg.opt.src { + pkg.newlines(1) + pkg.ToText(&pkg.buf, comment, indent, indent+indent) + pkg.newlines(2) // Blank line after comment to separate from next item. + } else { + pkg.newlines(1) + } + } +} + +// oneLineNode returns a one-line summary of the given input node. +func (pkg *pkgPrinter) oneLineNode(node ast.Node) string { + const maxDepth = 10 + return pkg.oneLineNodeDepth(node, maxDepth) +} + +// oneLineNodeDepth returns a one-line summary of the given input node. +// The depth specifies the maximum depth when traversing the AST. +func (pkg *pkgPrinter) oneLineNodeDepth(node ast.Node, depth int) string { + const dotDotDot = "..." + if depth == 0 { + return dotDotDot + } + depth-- + + switch n := node.(type) { + case nil: + return "" + + case *ast.GenDecl: + // Formats const and var declarations. + trailer := "" + if len(n.Specs) > 1 { + trailer = " " + dotDotDot + } + + // Find the first relevant spec. + typ := "" + for i, spec := range n.Specs { + valueSpec := spec.(*ast.ValueSpec) // Must succeed; we can't mix types in one GenDecl. + + // The type name may carry over from a previous specification in the + // case of constants and iota. + if valueSpec.Type != nil { + typ = fmt.Sprintf(" %s", pkg.oneLineNodeDepth(valueSpec.Type, depth)) + } else if len(valueSpec.Values) > 0 { + typ = "" + } + + if !pkg.isExported(valueSpec.Names[0].Name) { + continue + } + val := "" + if i < len(valueSpec.Values) && valueSpec.Values[i] != nil { + val = fmt.Sprintf(" = %s", pkg.oneLineNodeDepth(valueSpec.Values[i], depth)) + } + return fmt.Sprintf("%s %s%s%s%s", n.Tok, valueSpec.Names[0], typ, val, trailer) + } + return "" + + case *ast.FuncDecl: + // Formats func declarations. + name := n.Name.Name + recv := pkg.oneLineNodeDepth(n.Recv, depth) + if len(recv) > 0 { + recv = "(" + recv + ") " + } + fnc := pkg.oneLineNodeDepth(n.Type, depth) + fnc = strings.TrimPrefix(fnc, "func") + return fmt.Sprintf("func %s%s%s", recv, name, fnc) + + case *ast.TypeSpec: + sep := " " + if n.Assign.IsValid() { + sep = " = " + } + tparams := pkg.formatTypeParams(n.TypeParams, depth) + return fmt.Sprintf("type %s%s%s%s", n.Name.Name, tparams, sep, pkg.oneLineNodeDepth(n.Type, depth)) + + case *ast.FuncType: + var params []string + if n.Params != nil { + for _, field := range n.Params.List { + params = append(params, pkg.oneLineField(field, depth)) + } + } + needParens := false + var results []string + if n.Results != nil { + needParens = needParens || len(n.Results.List) > 1 + for _, field := range n.Results.List { + needParens = needParens || len(field.Names) > 0 + results = append(results, pkg.oneLineField(field, depth)) + } + } + + tparam := pkg.formatTypeParams(n.TypeParams, depth) + param := joinStrings(params) + if len(results) == 0 { + return fmt.Sprintf("func%s(%s)", tparam, param) + } + result := joinStrings(results) + if !needParens { + return fmt.Sprintf("func%s(%s) %s", tparam, param, result) + } + return fmt.Sprintf("func%s(%s) (%s)", tparam, param, result) + + case *ast.StructType: + if n.Fields == nil || len(n.Fields.List) == 0 { + return "struct{}" + } + return "struct{ ... }" + + case *ast.InterfaceType: + if n.Methods == nil || len(n.Methods.List) == 0 { + return "interface{}" + } + return "interface{ ... }" + + case *ast.FieldList: + if n == nil || len(n.List) == 0 { + return "" + } + if len(n.List) == 1 { + return pkg.oneLineField(n.List[0], depth) + } + return dotDotDot + + case *ast.FuncLit: + return pkg.oneLineNodeDepth(n.Type, depth) + " { ... }" + + case *ast.CompositeLit: + typ := pkg.oneLineNodeDepth(n.Type, depth) + if len(n.Elts) == 0 { + return fmt.Sprintf("%s{}", typ) + } + return fmt.Sprintf("%s{ %s }", typ, dotDotDot) + + case *ast.ArrayType: + length := pkg.oneLineNodeDepth(n.Len, depth) + element := pkg.oneLineNodeDepth(n.Elt, depth) + return fmt.Sprintf("[%s]%s", length, element) + + case *ast.MapType: + key := pkg.oneLineNodeDepth(n.Key, depth) + value := pkg.oneLineNodeDepth(n.Value, depth) + return fmt.Sprintf("map[%s]%s", key, value) + + case *ast.CallExpr: + fnc := pkg.oneLineNodeDepth(n.Fun, depth) + var args []string + for _, arg := range n.Args { + args = append(args, pkg.oneLineNodeDepth(arg, depth)) + } + return fmt.Sprintf("%s(%s)", fnc, joinStrings(args)) + + case *ast.UnaryExpr: + return fmt.Sprintf("%s%s", n.Op, pkg.oneLineNodeDepth(n.X, depth)) + + case *ast.Ident: + return n.Name + + default: + // As a fallback, use default formatter for all unknown node types. + buf := new(strings.Builder) + format.Node(buf, pkg.fs, node) + s := buf.String() + if strings.Contains(s, "\n") { + return dotDotDot + } + return s + } +} + +func (pkg *pkgPrinter) formatTypeParams(list *ast.FieldList, depth int) string { + if list.NumFields() == 0 { + return "" + } + var tparams []string + for _, field := range list.List { + tparams = append(tparams, pkg.oneLineField(field, depth)) + } + return "[" + joinStrings(tparams) + "]" +} + +// oneLineField returns a one-line summary of the field. +func (pkg *pkgPrinter) oneLineField(field *ast.Field, depth int) string { + var names []string + for _, name := range field.Names { + names = append(names, name.Name) + } + if len(names) == 0 { + return pkg.oneLineNodeDepth(field.Type, depth) + } + return joinStrings(names) + " " + pkg.oneLineNodeDepth(field.Type, depth) +} + +// joinStrings formats the input as a comma-separated list, +// but truncates the list at some reasonable length if necessary. +func joinStrings(ss []string) string { + var n int + for i, s := range ss { + n += len(s) + len(", ") + if n > punchedCardWidth { + ss = append(ss[:i:i], "...") + break + } + } + return strings.Join(ss, ", ") +} + +// allDoc prints all the docs for the package. +func (pkg *pkgPrinter) allDoc() { + pkg.Printf("") // Trigger the package clause; we know the package exists. + pkg.ToText(&pkg.buf, pkg.doc.Doc, "", indent) + pkg.newlines(1) + + printed := make(map[*ast.GenDecl]bool) + + hdr := "" + printHdr := func(s string) { + if hdr != s { + pkg.Printf("\n%s\n\n", s) + hdr = s + } + } + + // Constants. + for _, value := range pkg.doc.Consts { + // Constants and variables come in groups, and valueDoc prints + // all the items in the group. We only need to find one exported symbol. + for _, name := range value.Names { + if pkg.isExported(name) && !pkg.typedValue[value] { + printHdr("CONSTANTS") + pkg.valueDoc(value, printed) + break + } + } + } + + // Variables. + for _, value := range pkg.doc.Vars { + // Constants and variables come in groups, and valueDoc prints + // all the items in the group. We only need to find one exported symbol. + for _, name := range value.Names { + if pkg.isExported(name) && !pkg.typedValue[value] { + printHdr("VARIABLES") + pkg.valueDoc(value, printed) + break + } + } + } + + // Functions. + for _, fun := range pkg.doc.Funcs { + if pkg.isExported(fun.Name) && !pkg.constructor[fun] { + printHdr("FUNCTIONS") + pkg.emit(fun.Doc, fun.Decl) + } + } + + // Types. + for _, typ := range pkg.doc.Types { + if pkg.isExported(typ.Name) { + printHdr("TYPES") + pkg.typeDoc(typ) + } + } +} + +// packageDoc prints the docs for the package (package doc plus one-liners of the rest). +func (pkg *pkgPrinter) packageDoc() { + pkg.Printf("") // Trigger the package clause; we know the package exists. + if !pkg.opt.short { + pkg.ToText(&pkg.buf, pkg.doc.Doc, "", indent) + pkg.newlines(1) + } + + if !pkg.opt.short { + pkg.newlines(2) // Guarantee blank line before the components. + } + + pkg.valueSummary(pkg.doc.Consts, false) + pkg.valueSummary(pkg.doc.Vars, false) + pkg.funcSummary(pkg.doc.Funcs, false) + pkg.typeSummary() + if !pkg.opt.short { + pkg.bugs() + } +} + +// packageClause prints the package clause. +func (pkg *pkgPrinter) packageClause() { + if pkg.opt.short { + return + } + + // If we're using modules, the import path derived from module code locations wins. + // If we did a file system scan, we knew the import path when we found the directory. + // But if we started with a directory name, we never knew the import path. + // Either way, we don't know it now, and it's cheap to (re)compute it. + /* TODO: add when supporting gnodev doc on local directories + if usingModules { + for _, root := range codeRoots() { + if pkg.build.Dir == root.dir { + importPath = root.importPath + break + } + if strings.HasPrefix(pkg.build.Dir, root.dir+string(filepath.Separator)) { + suffix := filepath.ToSlash(pkg.build.Dir[len(root.dir)+1:]) + if root.importPath == "" { + importPath = suffix + } else { + importPath = root.importPath + "/" + suffix + } + break + } + } + } + */ + + pkg.Printf("package %s // import %q\n\n", pkg.name, pkg.importPath) + /* TODO + if !usingModules && importPath != pkg.build.ImportPath { + pkg.Printf("WARNING: package source is installed in %q\n", pkg.build.ImportPath) + } */ +} + +// valueSummary prints a one-line summary for each set of values and constants. +// If all the types in a constant or variable declaration belong to the same +// type they can be printed by typeSummary, and so can be suppressed here. +func (pkg *pkgPrinter) valueSummary(values []*doc.Value, showGrouped bool) { + var isGrouped map[*doc.Value]bool + if !showGrouped { + isGrouped = make(map[*doc.Value]bool) + for _, typ := range pkg.doc.Types { + if !pkg.isExported(typ.Name) { + continue + } + for _, c := range typ.Consts { + isGrouped[c] = true + } + for _, v := range typ.Vars { + isGrouped[v] = true + } + } + } + + for _, value := range values { + if !isGrouped[value] { + if decl := pkg.oneLineNode(value.Decl); decl != "" { + pkg.Printf("%s\n", decl) + } + } + } +} + +// funcSummary prints a one-line summary for each function. Constructors +// are printed by typeSummary, below, and so can be suppressed here. +func (pkg *pkgPrinter) funcSummary(funcs []*doc.Func, showConstructors bool) { + for _, fun := range funcs { + // Exported functions only. The go/doc package does not include methods here. + if pkg.isExported(fun.Name) { + if showConstructors || !pkg.constructor[fun] { + pkg.Printf("%s\n", pkg.oneLineNode(fun.Decl)) + } + } + } +} + +// typeSummary prints a one-line summary for each type, followed by its constructors. +func (pkg *pkgPrinter) typeSummary() { + for _, typ := range pkg.doc.Types { + for _, spec := range typ.Decl.Specs { + typeSpec := spec.(*ast.TypeSpec) // Must succeed. + if pkg.isExported(typeSpec.Name.Name) { + pkg.Printf("%s\n", pkg.oneLineNode(typeSpec)) + // Now print the consts, vars, and constructors. + for _, c := range typ.Consts { + if decl := pkg.oneLineNode(c.Decl); decl != "" { + pkg.Printf(indent+"%s\n", decl) + } + } + for _, v := range typ.Vars { + if decl := pkg.oneLineNode(v.Decl); decl != "" { + pkg.Printf(indent+"%s\n", decl) + } + } + for _, constructor := range typ.Funcs { + if pkg.isExported(constructor.Name) { + pkg.Printf(indent+"%s\n", pkg.oneLineNode(constructor.Decl)) + } + } + } + } + } +} + +// bugs prints the BUGS information for the package. +// TODO: Provide access to TODOs and NOTEs as well (very noisy so off by default)? +func (pkg *pkgPrinter) bugs() { + if pkg.doc.Notes["BUG"] == nil { + return + } + pkg.Printf("\n") + for _, note := range pkg.doc.Notes["BUG"] { + pkg.Printf("%s: %v\n", "BUG", note.Body) + } +} + +// findValues finds the doc.Values that describe the symbol. +func (pkg *pkgPrinter) findValues(symbol string, docValues []*doc.Value) (values []*doc.Value) { + for _, value := range docValues { + for _, name := range value.Names { + if pkg.match(symbol, name) { + values = append(values, value) + } + } + } + return +} + +// findFuncs finds the doc.Funcs that describes the symbol. +func (pkg *pkgPrinter) findFuncs(symbol string) (funcs []*doc.Func) { + for _, fun := range pkg.doc.Funcs { + if pkg.match(symbol, fun.Name) { + funcs = append(funcs, fun) + } + } + return +} + +// findTypes finds the doc.Types that describes the symbol. +// If symbol is empty, it finds all exported types. +func (pkg *pkgPrinter) findTypes(symbol string) (types []*doc.Type) { + for _, typ := range pkg.doc.Types { + if symbol == "" && pkg.isExported(typ.Name) || pkg.match(symbol, typ.Name) { + types = append(types, typ) + } + } + return +} + +// findTypeSpec returns the ast.TypeSpec within the declaration that defines the symbol. +// The name must match exactly. +func (pkg *pkgPrinter) findTypeSpec(decl *ast.GenDecl, symbol string) *ast.TypeSpec { + for _, spec := range decl.Specs { + typeSpec := spec.(*ast.TypeSpec) // Must succeed. + if symbol == typeSpec.Name.Name { + return typeSpec + } + } + return nil +} + +// symbolDoc prints the docs for symbol. There may be multiple matches. +// If symbol matches a type, output includes its methods factories and associated constants. +// If there is no top-level symbol, symbolDoc looks for methods that match. +func (pkg *pkgPrinter) symbolDoc(symbol string) bool { + found := false + // Functions. + for _, fun := range pkg.findFuncs(symbol) { + // Symbol is a function. + decl := fun.Decl + pkg.emit(fun.Doc, decl) + found = true + } + // Constants and variables behave the same. + values := pkg.findValues(symbol, pkg.doc.Consts) + values = append(values, pkg.findValues(symbol, pkg.doc.Vars)...) + // A declaration like + // const ( c = 1; C = 2 ) + // could be printed twice if the -u flag is set, as it matches twice. + // So we remember which declarations we've printed to avoid duplication. + printed := make(map[*ast.GenDecl]bool) + for _, value := range values { + pkg.valueDoc(value, printed) + found = true + } + // Types. + for _, typ := range pkg.findTypes(symbol) { + pkg.typeDoc(typ) + found = true + } + if !found { + // See if there are methods. + if !pkg.printMethodDoc("", symbol) { + return false + } + } + return true +} + +// valueDoc prints the docs for a constant or variable. +func (pkg *pkgPrinter) valueDoc(value *doc.Value, printed map[*ast.GenDecl]bool) { + if printed[value.Decl] { + return + } + // Print each spec only if there is at least one exported symbol in it. + // (See issue 11008.) + // TODO: Should we elide unexported symbols from a single spec? + // It's an unlikely scenario, probably not worth the trouble. + // TODO: Would be nice if go/doc did this for us. + specs := make([]ast.Spec, 0, len(value.Decl.Specs)) + var typ ast.Expr + for _, spec := range value.Decl.Specs { + vspec := spec.(*ast.ValueSpec) + + // The type name may carry over from a previous specification in the + // case of constants and iota. + if vspec.Type != nil { + typ = vspec.Type + } + + for _, ident := range vspec.Names { + if pkg.opt.src || pkg.isExported(ident.Name) { + if vspec.Type == nil && vspec.Values == nil && typ != nil { + // This a standalone identifier, as in the case of iota usage. + // Thus, assume the type comes from the previous type. + vspec.Type = &ast.Ident{ + Name: pkg.oneLineNode(typ), + NamePos: vspec.End() - 1, + } + } + + specs = append(specs, vspec) + typ = nil // Only inject type on first exported identifier + break + } + } + } + if len(specs) == 0 { + return + } + value.Decl.Specs = specs + pkg.emit(value.Doc, value.Decl) + printed[value.Decl] = true +} + +// typeDoc prints the docs for a type, including constructors and other items +// related to it. +func (pkg *pkgPrinter) typeDoc(typ *doc.Type) { + decl := typ.Decl + spec := pkg.findTypeSpec(decl, typ.Name) + pkg.trimUnexportedElems(spec) + // If there are multiple types defined, reduce to just this one. + if len(decl.Specs) > 1 { + decl.Specs = []ast.Spec{spec} + } + pkg.emit(typ.Doc, decl) + pkg.newlines(2) + // Show associated methods, constants, etc. + if pkg.opt.all { + printed := make(map[*ast.GenDecl]bool) + // We can use append here to print consts, then vars. Ditto for funcs and methods. + values := typ.Consts + values = append(values, typ.Vars...) + for _, value := range values { + for _, name := range value.Names { + if pkg.isExported(name) { + pkg.valueDoc(value, printed) + break + } + } + } + funcs := typ.Funcs + funcs = append(funcs, typ.Methods...) + for _, fun := range funcs { + if pkg.isExported(fun.Name) { + pkg.emit(fun.Doc, fun.Decl) + if fun.Doc == "" { + pkg.newlines(2) + } + } + } + } else { + pkg.valueSummary(typ.Consts, true) + pkg.valueSummary(typ.Vars, true) + pkg.funcSummary(typ.Funcs, true) + pkg.funcSummary(typ.Methods, true) + } +} + +// trimUnexportedElems modifies spec in place to elide unexported fields from +// structs and methods from interfaces (unless the unexported flag is set or we +// are asked to show the original source). +func (pkg *pkgPrinter) trimUnexportedElems(spec *ast.TypeSpec) { + if pkg.opt.unexported || pkg.opt.src { + return + } + switch typ := spec.Type.(type) { + case *ast.StructType: + typ.Fields = pkg.trimUnexportedFields(typ.Fields, false) + case *ast.InterfaceType: + typ.Methods = pkg.trimUnexportedFields(typ.Methods, true) + } +} + +// trimUnexportedFields returns the field list trimmed of unexported fields. +func (pkg *pkgPrinter) trimUnexportedFields(fields *ast.FieldList, isInterface bool) *ast.FieldList { + what := "methods" + if !isInterface { + what = "fields" + } + + trimmed := false + list := make([]*ast.Field, 0, len(fields.List)) + for _, field := range fields.List { + names := field.Names + if len(names) == 0 { + // Embedded type. Use the name of the type. It must be of the form ident or + // pkg.ident (for structs and interfaces), or *ident or *pkg.ident (structs only). + // Or a type embedded in a constraint. + // Nothing else is allowed. + ty := field.Type + if se, ok := field.Type.(*ast.StarExpr); !isInterface && ok { + // The form *ident or *pkg.ident is only valid on + // embedded types in structs. + ty = se.X + } + constraint := false + switch ident := ty.(type) { + case *ast.Ident: + if isInterface && ident.Name == "error" && ident.Obj == nil { + // For documentation purposes, we consider the builtin error + // type special when embedded in an interface, such that it + // always gets shown publicly. + list = append(list, field) + continue + } + names = []*ast.Ident{ident} + case *ast.SelectorExpr: + // An embedded type may refer to a type in another package. + names = []*ast.Ident{ident.Sel} + default: + // An approximation or union or type + // literal in an interface. + constraint = true + } + if names == nil && !constraint { + // Can only happen if AST is incorrect. Safe to continue with a nil list. + log.Print("invalid program: unexpected type for embedded field") + } + } + // Trims if any is unexported. Good enough in practice. + ok := true + for _, name := range names { + if !pkg.isExported(name.Name) { + trimmed = true + ok = false + break + } + } + if ok { + list = append(list, field) + } + } + if !trimmed { + return fields + } + unexportedField := &ast.Field{ + Type: &ast.Ident{ + // Hack: printer will treat this as a field with a named type. + // Setting Name and NamePos to ("", fields.Closing-1) ensures that + // when Pos and End are called on this field, they return the + // position right before closing '}' character. + Name: "", + NamePos: fields.Closing - 1, + }, + Comment: &ast.CommentGroup{ + List: []*ast.Comment{{Text: fmt.Sprintf("// Has unexported %s.\n", what)}}, + }, + } + return &ast.FieldList{ + Opening: fields.Opening, + List: append(list, unexportedField), + Closing: fields.Closing, + } +} + +// printMethodDoc prints the docs for matches of symbol.method. +// If symbol is empty, it prints all methods for any concrete type +// that match the name. It reports whether it found any methods. +func (pkg *pkgPrinter) printMethodDoc(symbol, method string) bool { + types := pkg.findTypes(symbol) + if types == nil { + if symbol == "" { + return false + } + pkg.Fatalf("symbol %s is not a type in package %s installed in %q", symbol, pkg.name, pkg.importPath) + } + found := false + for _, typ := range types { + if len(typ.Methods) > 0 { + for _, meth := range typ.Methods { + if pkg.match(method, meth.Name) { + decl := meth.Decl + pkg.emit(meth.Doc, decl) + found = true + } + } + continue + } + if symbol == "" { + continue + } + // Type may be an interface. The go/doc package does not attach + // an interface's methods to the doc.Type. We need to dig around. + spec := pkg.findTypeSpec(typ.Decl, typ.Name) + inter, ok := spec.Type.(*ast.InterfaceType) + if !ok { + // Not an interface type. + continue + } + + // Collect and print only the methods that match. + var methods []*ast.Field + for _, iMethod := range inter.Methods.List { + // This is an interface, so there can be only one name. + // TODO: Anonymous methods (embedding) + if len(iMethod.Names) == 0 { + continue + } + name := iMethod.Names[0].Name + if pkg.match(method, name) { + methods = append(methods, iMethod) + found = true + } + } + if found { + pkg.Printf("type %s ", spec.Name) + inter.Methods.List, methods = methods, inter.Methods.List + err := format.Node(&pkg.buf, pkg.fs, inter) + if err != nil { + log.Fatal(err) + } + pkg.newlines(1) + // Restore the original methods. + inter.Methods.List = methods + } + } + return found +} + +// printFieldDoc prints the docs for matches of symbol.fieldName. +// It reports whether it found any field. +// Both symbol and fieldName must be non-empty or it returns false. +func (pkg *pkgPrinter) printFieldDoc(symbol, fieldName string) bool { + if symbol == "" || fieldName == "" { + return false + } + types := pkg.findTypes(symbol) + if types == nil { + pkg.Fatalf("symbol %s is not a type in package %s installed in %q", symbol, pkg.name, pkg.importPath) + } + found := false + numUnmatched := 0 + for _, typ := range types { + // Type must be a struct. + spec := pkg.findTypeSpec(typ.Decl, typ.Name) + structType, ok := spec.Type.(*ast.StructType) + if !ok { + // Not a struct type. + continue + } + for _, field := range structType.Fields.List { + // TODO: Anonymous fields. + for _, name := range field.Names { + if !pkg.match(fieldName, name.Name) { + numUnmatched++ + continue + } + if !found { + pkg.Printf("type %s struct {\n", typ.Name) + } + if field.Doc != nil { + // To present indented blocks in comments correctly, process the comment as + // a unit before adding the leading // to each line. + docBuf := new(bytes.Buffer) + pkg.ToText(docBuf, field.Doc.Text(), "", indent) + scanner := bufio.NewScanner(docBuf) + for scanner.Scan() { + fmt.Fprintf(&pkg.buf, "%s// %s\n", indent, scanner.Bytes()) + } + } + s := pkg.oneLineNode(field.Type) + lineComment := "" + if field.Comment != nil { + lineComment = fmt.Sprintf(" %s", field.Comment.List[0].Text) + } + pkg.Printf("%s%s %s%s\n", indent, name, s, lineComment) + found = true + } + } + } + if found { + if numUnmatched > 0 { + pkg.Printf("\n // ... other fields elided ...\n") + } + pkg.Printf("}\n") + } + return found +} + +// methodDoc prints the docs for matches of symbol.method. +func (pkg *pkgPrinter) methodDoc(symbol, method string) bool { + return pkg.printMethodDoc(symbol, method) +} + +// fieldDoc prints the docs for matches of symbol.field. +func (pkg *pkgPrinter) fieldDoc(symbol, field string) bool { + return pkg.printFieldDoc(symbol, field) +} + +// match reports whether the user's symbol matches the program's. +// A lower-case character in the user's string matches either case in the program's. +// The program string must be exported. +func (pkg *pkgPrinter) match(user, program string) bool { + if !pkg.isExported(program) { + return false + } + /* TODO: might be useful to add for tooling. + if matchCase { + return user == program + } */ + for _, u := range user { + p, w := utf8.DecodeRuneInString(program) + program = program[w:] + if u == p { + continue + } + if unicode.IsLower(u) && simpleFold(u) == simpleFold(p) { + continue + } + return false + } + return program == "" +} + +// simpleFold returns the minimum rune equivalent to r +// under Unicode-defined simple case folding. +func simpleFold(r rune) rune { + for { + r1 := unicode.SimpleFold(r) + if r1 <= r { + return r1 // wrapped around, found min + } + r = r1 + } +} From 042f86e17892994842b25a1077be719443b9ae98 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Fri, 17 Mar 2023 13:10:14 +0100 Subject: [PATCH 03/24] chore: remove accidental log line --- tests/package_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/package_test.go b/tests/package_test.go index d2c06e87d6e..4c5b1a1fa02 100644 --- a/tests/package_test.go +++ b/tests/package_test.go @@ -39,7 +39,6 @@ func TestPackages(t *testing.T) { // already exists. } else { testDirs[dirPath] = filepath.Join(rootDir, dirPath) - pkgPaths = append(pkgPaths, dirPath) } } return nil From cbb80125e34ea63472b885f2262a942dc1427a68 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Fri, 17 Mar 2023 15:51:58 +0100 Subject: [PATCH 04/24] chore: linter --- pkgs/commands/doc/print.go | 14 +++++++------- tests/package_test.go | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkgs/commands/doc/print.go b/pkgs/commands/doc/print.go index 2da1c23da8e..638466dfa88 100644 --- a/pkgs/commands/doc/print.go +++ b/pkgs/commands/doc/print.go @@ -44,14 +44,14 @@ type pkgPrinter struct { err error } -func (p *pkgPrinter) isExported(name string) bool { +func (pkg *pkgPrinter) isExported(name string) bool { // cmd/doc uses a global here, so we change this to be a method. - return p.opt.unexported || token.IsExported(name) + return pkg.opt.unexported || token.IsExported(name) } -func (p *pkgPrinter) ToText(w io.Writer, text, prefix, codePrefix string) { - d := p.doc.Parser().Parse(text) - pr := p.doc.Printer() +func (pkg *pkgPrinter) ToText(w io.Writer, text, prefix, codePrefix string) { + d := pkg.doc.Parser().Parse(text) + pr := pkg.doc.Printer() pr.TextPrefix = prefix pr.TextCodePrefix = codePrefix w.Write(pr.Text(d)) @@ -301,7 +301,7 @@ func (pkg *pkgPrinter) formatTypeParams(list *ast.FieldList, depth int) string { if list.NumFields() == 0 { return "" } - var tparams []string + tparams := make([]string, 0, len(list.List)) for _, field := range list.List { tparams = append(tparams, pkg.oneLineField(field, depth)) } @@ -310,7 +310,7 @@ func (pkg *pkgPrinter) formatTypeParams(list *ast.FieldList, depth int) string { // oneLineField returns a one-line summary of the field. func (pkg *pkgPrinter) oneLineField(field *ast.Field, depth int) string { - var names []string + names := make([]string, 0, len(field.Names)) for _, name := range field.Names { names = append(names, name.Name) } diff --git a/tests/package_test.go b/tests/package_test.go index 4c5b1a1fa02..5420a0ed881 100644 --- a/tests/package_test.go +++ b/tests/package_test.go @@ -39,6 +39,7 @@ func TestPackages(t *testing.T) { // already exists. } else { testDirs[dirPath] = filepath.Join(rootDir, dirPath) + pkgPaths = append(pkgPaths, dirPath) } } return nil @@ -46,7 +47,6 @@ func TestPackages(t *testing.T) { } // Sort pkgPaths for determinism. sort.Strings(pkgPaths) - t.Log(pkgPaths) // For each package with testfiles (in testDirs), call Machine.TestMemPackage. for _, pkgPath := range pkgPaths { testDir := testDirs[pkgPath] From 54d5ff95d551d34d860b2a093f6a95d30fe5f950 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Tue, 21 Mar 2023 19:12:22 +0100 Subject: [PATCH 05/24] feat(commands/doc): add support for examples directory --- cmd/gnodev/doc.go | 2 +- pkgs/commands/doc/dirs.go | 16 ++++++++-- pkgs/commands/doc/doc.go | 64 ++++++++++++++++++++++++++++++++------- 3 files changed, 68 insertions(+), 14 deletions(-) diff --git a/cmd/gnodev/doc.go b/cmd/gnodev/doc.go index 32f34a2daed..fdf492efb4c 100644 --- a/cmd/gnodev/doc.go +++ b/cmd/gnodev/doc.go @@ -61,7 +61,7 @@ func execDoc(cfg *docCfg, args []string, io *commands.IO) error { if cfg.rootDir == "" { cfg.rootDir = guessRootDir() } - dirs := doc.NewDirs(filepath.Join(cfg.rootDir, "stdlibs")) + dirs := doc.NewDirs(filepath.Join(cfg.rootDir, "stdlibs"), filepath.Join(cfg.rootDir, "examples")) res, err := doc.ResolveDocumentable(dirs, args, cfg.unexported) switch { case res == nil: diff --git a/pkgs/commands/doc/dirs.go b/pkgs/commands/doc/dirs.go index af282b66450..47ff7d2148b 100644 --- a/pkgs/commands/doc/dirs.go +++ b/pkgs/commands/doc/dirs.go @@ -30,12 +30,12 @@ type Dirs struct { } // NewDirs begins scanning the given stdlibs directory. -func NewDirs(stdlibsDir string) *Dirs { +func NewDirs(dirs ...string) *Dirs { d := &Dirs{ hist: make([]Dir, 0, 256), scan: make(chan Dir), } - go d.walk([]string{stdlibsDir}) + go d.walk(dirs) return d } @@ -151,3 +151,15 @@ func (d *Dirs) findPackage(name string) []Dir { }) return candidates } + +// findDir determines if the given absdir is present in the Dirs. +// If not, the nil slice is returned. It returns always at most one dir. +func (d *Dirs) findDir(absdir string) []Dir { + d.Reset() + for dir, ok := d.Next(); ok; dir, ok = d.Next() { + if dir.dir == absdir { + return []Dir{dir} + } + } + return nil +} diff --git a/pkgs/commands/doc/doc.go b/pkgs/commands/doc/doc.go index 1a213192566..e39c7e7a8aa 100644 --- a/pkgs/commands/doc/doc.go +++ b/pkgs/commands/doc/doc.go @@ -12,6 +12,8 @@ import ( "go/doc" "go/token" "io" + "os" + "path/filepath" "strings" "go.uber.org/multierr" @@ -164,10 +166,18 @@ func ResolveDocumentable(dirs *Dirs, args []string, unexported bool) (Documentab // if we have a candidate package name, search dirs for a dir that matches it. // prefer directories whose import path match precisely the package if parsed[0].typ&argPkg > 0 { - if strings.Contains(parsed[0].val, ".") { - panic("local packages not yet supported") + if s, err := os.Stat(parsed[0].val); err == nil && s.IsDir() { + // expand to full path + absVal, err := filepath.Abs(parsed[0].val) + if err == nil { + candidates = dirs.findDir(absVal) + } + } + // first arg is either not a dir, or if it matched a local dir it was not + // valid (ie. not scanned by dirs). try parsing as a package + if len(candidates) == 0 { + candidates = dirs.findPackage(parsed[0].val) } - candidates = dirs.findPackage(parsed[0].val) // easy case: we wanted documentation about a package, and we found one! if len(parsed) == 1 && len(candidates) > 0 { return &documentable{Dir: candidates[0]}, nil @@ -177,7 +187,7 @@ func ResolveDocumentable(dirs *Dirs, args []string, unexported bool) (Documentab // if this can be something other than a package, remove argPkg as an // option, otherwise return not found. if parsed[0].typ == argPkg { - return nil, fmt.Errorf("commands/doc: package not found: %q", parsed[0].val) + return nil, fmt.Errorf("commands/doc: package not found: %q (note: local packages are not yet supported)", parsed[0].val) } parsed[0].typ &= ^argPkg } else { @@ -191,7 +201,13 @@ func ResolveDocumentable(dirs *Dirs, args []string, unexported bool) (Documentab // search for the symbol through the candidates if len(candidates) == 0 { // no candidates means local directory here - panic("local packages not yet supported") + wd, err := os.Getwd() + if err == nil { + candidates = dirs.findDir(wd) + } + if len(candidates) == 0 { + return nil, fmt.Errorf("commands/doc: local packages not yet supported") + } } doc := &documentable{} @@ -249,7 +265,7 @@ func ResolveDocumentable(dirs *Dirs, args []string, unexported bool) (Documentab } } return nil, multierr.Append( - fmt.Errorf("commands/doc: could not resolve arguments: %v", args), + fmt.Errorf("commands/doc: could not resolve arguments: %v", parsed), multierr.Combine(errs...), ) } @@ -272,6 +288,32 @@ type argPart struct { typ byte } +func (a argPart) String() string { + var b strings.Builder + if a.typ&argPkg != 0 { + b.WriteString("pkg") + } + if a.typ&argSym != 0 { + if b.Len() != 0 { + b.WriteByte('|') + } + b.WriteString("sym") + } + if a.typ&argAcc != 0 { + if b.Len() != 0 { + b.WriteByte('|') + } + b.WriteString("acc") + } + if b.Len() == 0 { + b.WriteString("inv:") + } else { + b.WriteByte(':') + } + b.WriteString(a.val) + return b.String() +} + func parseArgParts(args []string) []argPart { parsed := make([]argPart, 0, 3) switch len(args) { @@ -298,15 +340,15 @@ func parseArgParts(args []string) []argPart { switch strings.Count(args[0][slash+1:], ".") { case 0: t := argPkg | argSym | argAcc - if strings.IndexByte(args[0], '/') != -1 { + if slash != -1 { t = argPkg } parsed = append(parsed, argPart{args[0], t}) case 1: - pos := strings.IndexByte(args[0], '.') + pos := strings.IndexByte(args[0][slash+1:], '.') + slash + 1 // pkg.sym, pkg.acc, sym.acc t1, t2 := argPkg|argSym, argSym|argAcc - if strings.IndexByte(args[0][:pos], '/') != -1 { + if slash != -1 { t1 = argPkg } else if token.IsExported(args[0]) { // See rationale here: @@ -319,9 +361,9 @@ func parseArgParts(args []string) []argPart { ) case 2: // pkg.sym.acc - parts := strings.Split(args[0], ".") + parts := strings.Split(args[0][slash+1:], ".") parsed = append(parsed, - argPart{parts[0], argPkg}, + argPart{args[0][:slash+1] + parts[0], argPkg}, argPart{parts[1], argSym}, argPart{parts[2], argAcc}, ) From 49bc623078431f0ce259ba5e82e58bf492275b92 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Tue, 21 Mar 2023 20:30:12 +0100 Subject: [PATCH 06/24] chore: prefer writing to stdout directly instead of using a buffer --- cmd/gnodev/doc.go | 4 ++-- pkgs/commands/doc/doc.go | 24 +++++++++++++++--------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/cmd/gnodev/doc.go b/cmd/gnodev/doc.go index fdf492efb4c..4441873e108 100644 --- a/cmd/gnodev/doc.go +++ b/cmd/gnodev/doc.go @@ -69,14 +69,14 @@ func execDoc(cfg *docCfg, args []string, io *commands.IO) error { case err != nil: io.Printfln("warning: error parsing some candidate packages:\n%v", err) } - output, err := res.Document( + err = res.Document( doc.WithShowAll(cfg.all), doc.WithSource(cfg.src), doc.WithUnexported(cfg.unexported), + doc.WithWriter(io.Out), ) if err != nil { return err } - io.Out.Write([]byte(output)) return nil } diff --git a/pkgs/commands/doc/doc.go b/pkgs/commands/doc/doc.go index e39c7e7a8aa..9c93838ccd6 100644 --- a/pkgs/commands/doc/doc.go +++ b/pkgs/commands/doc/doc.go @@ -6,7 +6,6 @@ package doc import ( - "bytes" "fmt" "go/ast" "go/doc" @@ -50,9 +49,15 @@ func WithShort(b bool) DocumentOption { return func(s *documentOptions) { s.short = b } } +// WithWriter uses the given writer as an output. +// By default, os.Stdout is used. +func WithWriter(w io.Writer) DocumentOption { + return func(s *documentOptions) { s.w = w } +} + // Documentable is a package, symbol, or accessible which can be documented. type Documentable interface { - Document(...DocumentOption) (string, error) + Document(...DocumentOption) error } type documentable struct { @@ -62,26 +67,28 @@ type documentable struct { pkgData *pkgData } -func (d *documentable) Document(opts ...DocumentOption) (string, error) { - buf := new(bytes.Buffer) - o := &documentOptions{w: buf} +func (d *documentable) Document(opts ...DocumentOption) error { + o := &documentOptions{w: os.Stdout} for _, opt := range opts { opt(o) } var err error + // pkgData may already be initialised if we already had to look to see + // if it had the symbol we wanted; otherwise initialise it now. if d.pkgData == nil { d.pkgData, err = newPkgData(d.Dir, o.unexported) if err != nil { - return "", err + return err } } astpkg, pkg, err := d.pkgData.docPackage(o) if err != nil { - return "", err + return err } + // copied from go source - map vars, constants and constructors to their respective types. typedValue := make(map[*doc.Value]bool) constructor := make(map[*doc.Func]bool) for _, typ := range pkg.Types { @@ -116,8 +123,7 @@ func (d *documentable) Document(opts ...DocumentOption) (string, error) { } pp.buf.pkg = pp - err = d.output(pp) - return buf.String(), err + return d.output(pp) } func (d *documentable) output(pp *pkgPrinter) (err error) { From 4ff36c8ee9083df52698780aecc385eeca195d04 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Wed, 22 Mar 2023 10:49:42 +0100 Subject: [PATCH 07/24] fix(doc): improve arg parsing, more tests --- pkgs/commands/doc/dirs.go | 8 +- pkgs/commands/doc/dirs_test.go | 74 ++++++ pkgs/commands/doc/doc.go | 215 ++++++------------ pkgs/commands/doc/doc_test.go | 153 +++++++++++++ pkgs/commands/doc/pkg.go | 15 +- pkgs/commands/doc/test_notapkg/.keep | 0 .../doc/testdata/dirs/crypto/crypto.gno | 0 .../doc/testdata/dirs/crypto/rand/rand.gno | 0 .../dirs/crypto/testdata/rand/ignored.gno | 0 pkgs/commands/doc/testdata/dirs/math/math.gno | 0 .../doc/testdata/dirs/math/rand/rand.gno | 0 pkgs/commands/doc/testdata/dirs/rand/rand.gno | 0 .../doc/testdata/integ/crypto/rand/rand.gno | 29 +++ .../commands/doc/testdata/integ/rand/rand.gno | 5 + .../doc/testdata/integ/test_notapkg/a.gno | 3 + pkgs/commands/doc/testdata/integ/wd/wd.gno | 13 ++ 16 files changed, 360 insertions(+), 155 deletions(-) create mode 100644 pkgs/commands/doc/dirs_test.go create mode 100644 pkgs/commands/doc/doc_test.go create mode 100644 pkgs/commands/doc/test_notapkg/.keep create mode 100644 pkgs/commands/doc/testdata/dirs/crypto/crypto.gno create mode 100644 pkgs/commands/doc/testdata/dirs/crypto/rand/rand.gno create mode 100644 pkgs/commands/doc/testdata/dirs/crypto/testdata/rand/ignored.gno create mode 100644 pkgs/commands/doc/testdata/dirs/math/math.gno create mode 100644 pkgs/commands/doc/testdata/dirs/math/rand/rand.gno create mode 100644 pkgs/commands/doc/testdata/dirs/rand/rand.gno create mode 100644 pkgs/commands/doc/testdata/integ/crypto/rand/rand.gno create mode 100644 pkgs/commands/doc/testdata/integ/rand/rand.gno create mode 100644 pkgs/commands/doc/testdata/integ/test_notapkg/a.gno create mode 100644 pkgs/commands/doc/testdata/integ/wd/wd.gno diff --git a/pkgs/commands/doc/dirs.go b/pkgs/commands/doc/dirs.go index 47ff7d2148b..9963073d649 100644 --- a/pkgs/commands/doc/dirs.go +++ b/pkgs/commands/doc/dirs.go @@ -117,10 +117,7 @@ func (d *Dirs) bfsWalkRoot(root string) { // It's a candidate. var importPath string if len(dir) > len(root) { - if importPath != "" { - importPath += "/" - } - importPath += filepath.ToSlash(dir[len(root)+1:]) + importPath = filepath.ToSlash(dir[len(root)+1:]) } d.scan <- Dir{importPath, dir} } @@ -136,7 +133,8 @@ func (d *Dirs) findPackage(name string) []Dir { d.Reset() candidates := make([]Dir, 0, 4) for dir, ok := d.Next(); ok; dir, ok = d.Next() { - if strings.HasSuffix(dir.importPath, name) { + // want either exact matches or suffixes + if dir.importPath == name || strings.HasSuffix(dir.importPath, "/"+name) { candidates = append(candidates, dir) } } diff --git a/pkgs/commands/doc/dirs_test.go b/pkgs/commands/doc/dirs_test.go new file mode 100644 index 00000000000..99d3425b076 --- /dev/null +++ b/pkgs/commands/doc/dirs_test.go @@ -0,0 +1,74 @@ +package doc + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func newDirs(t *testing.T) (string, *Dirs) { + t.Helper() + p, err := filepath.Abs("./testdata/dirs") + require.NoError(t, err) + return p, NewDirs(p) +} + +func TestDirs_findPackage(t *testing.T) { + abs, d := newDirs(t) + tt := []struct { + name string + res []Dir + }{ + {"rand", []Dir{ + {importPath: "rand", dir: filepath.Join(abs, "rand")}, + {importPath: "crypto/rand", dir: filepath.Join(abs, "crypto/rand")}, + {importPath: "math/rand", dir: filepath.Join(abs, "math/rand")}, + }}, + {"crypto/rand", []Dir{ + {importPath: "crypto/rand", dir: filepath.Join(abs, "crypto/rand")}, + }}, + {"math", []Dir{ + {importPath: "math", dir: filepath.Join(abs, "math")}, + }}, + {"ath", []Dir{}}, + {"/math", []Dir{}}, + {"", []Dir{}}, + } + for _, tc := range tt { + tc := tc + t.Run("name_"+strings.Replace(tc.name, "/", "_", -1), func(t *testing.T) { + res := d.findPackage(tc.name) + assert.Equal(t, tc.res, res, "dirs returned should be the equal") + }) + } +} + +func TestDirs_findDir(t *testing.T) { + abs, d := newDirs(t) + tt := []struct { + name string + in string + res []Dir + }{ + {"rand", filepath.Join(abs, "rand"), []Dir{ + {importPath: "rand", dir: filepath.Join(abs, "rand")}, + }}, + {"crypto/rand", filepath.Join(abs, "crypto/rand"), []Dir{ + {importPath: "crypto/rand", dir: filepath.Join(abs, "crypto/rand")}, + }}, + // ignored (dir name testdata), so should not return anything. + {"crypto/testdata/rand", filepath.Join(abs, "crypto/testdata/rand"), nil}, + {"xx", filepath.Join(abs, "xx"), nil}, + {"xx2", "/xx2", nil}, + } + for _, tc := range tt { + tc := tc + t.Run(strings.Replace(tc.name, "/", "_", -1), func(t *testing.T) { + res := d.findDir(tc.in) + assert.Equal(t, tc.res, res, "dirs returned should be the equal") + }) + } +} diff --git a/pkgs/commands/doc/doc.go b/pkgs/commands/doc/doc.go index 9c93838ccd6..a622ccba7d0 100644 --- a/pkgs/commands/doc/doc.go +++ b/pkgs/commands/doc/doc.go @@ -158,98 +158,66 @@ func (d *documentable) output(pp *pkgPrinter) (err error) { return } +// set as a variable so it can be changed by testing. +var fpAbs = filepath.Abs + // ResolveDocumentable returns a Documentable from the given arguments. // Refer to the documentation of gnodev doc for the formats accepted (in general // the same as the go doc command). func ResolveDocumentable(dirs *Dirs, args []string, unexported bool) (Documentable, error) { - parsed := parseArgParts(args) - if parsed == nil { + parsed, ok := parseArgs(args) + if !ok { return nil, fmt.Errorf("commands/doc: invalid arguments: %v", args) } + return resolveDocumentable(dirs, parsed, unexported) +} +func resolveDocumentable(dirs *Dirs, parsed docArgs, unexported bool) (Documentable, error) { var candidates []Dir // if we have a candidate package name, search dirs for a dir that matches it. // prefer directories whose import path match precisely the package - if parsed[0].typ&argPkg > 0 { - if s, err := os.Stat(parsed[0].val); err == nil && s.IsDir() { - // expand to full path - absVal, err := filepath.Abs(parsed[0].val) - if err == nil { - candidates = dirs.findDir(absVal) - } - } - // first arg is either not a dir, or if it matched a local dir it was not - // valid (ie. not scanned by dirs). try parsing as a package - if len(candidates) == 0 { - candidates = dirs.findPackage(parsed[0].val) - } - // easy case: we wanted documentation about a package, and we found one! - if len(parsed) == 1 && len(candidates) > 0 { - return &documentable{Dir: candidates[0]}, nil - } - if len(candidates) == 0 { - // there are no candidates. - // if this can be something other than a package, remove argPkg as an - // option, otherwise return not found. - if parsed[0].typ == argPkg { - return nil, fmt.Errorf("commands/doc: package not found: %q (note: local packages are not yet supported)", parsed[0].val) - } - parsed[0].typ &= ^argPkg - } else { - // if there are candidates, then the first argument was definitely - // a package. remove it from parsed so we don't worry about it again. - parsed = parsed[1:] + if s, err := os.Stat(parsed.pkg); err == nil && s.IsDir() { + // expand to full path + absVal, err := fpAbs(parsed.pkg) + if err == nil { + candidates = dirs.findDir(absVal) } } + // arg is either not a dir, or if it matched a local dir it was not + // valid (ie. not scanned by dirs). try parsing as a package + if len(candidates) == 0 { + candidates = dirs.findPackage(parsed.pkg) + } - // we also (or only) have a symbol/accessible. - // search for the symbol through the candidates if len(candidates) == 0 { - // no candidates means local directory here - wd, err := os.Getwd() - if err == nil { - candidates = dirs.findDir(wd) - } - if len(candidates) == 0 { - return nil, fmt.Errorf("commands/doc: local packages not yet supported") + // there are no candidates. + // if this is ambiguous, remove ambiguity and try parsing args using pkg as the symbol. + if !parsed.pkgAmbiguous { + return nil, fmt.Errorf("commands/doc: package not found: %q (note: local packages are not yet supported)", parsed.pkg) } + parsed = docArgs{pkg: ".", sym: parsed.pkg, acc: parsed.sym} + return resolveDocumentable(dirs, parsed, unexported) + } + // we wanted documentation about a package, and we found one! + if parsed.sym == "" { + return &documentable{Dir: candidates[0]}, nil } - doc := &documentable{} + // we also have a symbol, and maybe accessible. + // search for the symbol through the candidates + + doc := &documentable{ + symbol: parsed.sym, + accessible: parsed.acc, + } var matchFunc func(s string) bool - if len(parsed) == 2 { - // assert that we have and - if parsed[0].typ&argSym == 0 || parsed[1].typ&argAcc == 0 { - panic(fmt.Errorf("invalid remaining parsed: %+v", parsed)) - } - doc.symbol = parsed[0].val - doc.accessible = parsed[1].val - matchFunc = func(s string) bool { return s == parsed[0].val+"."+parsed[1].val } + if parsed.acc == "" { + matchFunc = func(s string) bool { return s == parsed.sym || strings.HasSuffix(s, "."+parsed.sym) } } else { - switch parsed[0].typ { - case argSym: - doc.symbol = parsed[0].val - matchFunc = func(s string) bool { return s == parsed[0].val } - case argAcc: - doc.accessible = parsed[0].val - matchFunc = func(s string) bool { return strings.HasSuffix(s, "."+parsed[0].val) } - case argSym | argAcc: - matchFunc = func(s string) bool { - switch { - case s == parsed[0].val: - doc.symbol = parsed[0].val - return true - case strings.HasSuffix(s, "."+parsed[0].val): - doc.accessible = parsed[0].val - return true - } - return false - } - default: - panic(fmt.Errorf("invalid remaining parsed: %+v", parsed)) - } + full := parsed.sym + "." + parsed.acc + matchFunc = func(s string) bool { return s == full } } var errs []error @@ -271,60 +239,27 @@ func ResolveDocumentable(dirs *Dirs, args []string, unexported bool) (Documentab } } return nil, multierr.Append( - fmt.Errorf("commands/doc: could not resolve arguments: %v", parsed), + fmt.Errorf("commands/doc: could not resolve arguments: %+v", parsed), multierr.Combine(errs...), ) } -// these are used to specify the type of argPart. -const ( - // ie. "crypto/cipher". - argPkg byte = 1 << iota - // ie. "TrimSuffix", "Builder" - argSym - // ie. "WriteString". method or field of a type ("accessible", from the - // word "accessor") - argAcc -) - -// argPart contains the value of the argument, together with the type of value -// it could be, through the flags argPkg, argSym and argAcc. -type argPart struct { - val string - typ byte -} +// docArgs represents the parsed args of the doc command. +// sym could be a symbol, but the accessibles of types should also be shown if they match sym. +type docArgs struct { + pkg string // always set + sym string + acc string // short for "accessible". only set if sym is also set -func (a argPart) String() string { - var b strings.Builder - if a.typ&argPkg != 0 { - b.WriteString("pkg") - } - if a.typ&argSym != 0 { - if b.Len() != 0 { - b.WriteByte('|') - } - b.WriteString("sym") - } - if a.typ&argAcc != 0 { - if b.Len() != 0 { - b.WriteByte('|') - } - b.WriteString("acc") - } - if b.Len() == 0 { - b.WriteString("inv:") - } else { - b.WriteByte(':') - } - b.WriteString(a.val) - return b.String() + // pkg could be a symbol in the local dir. + // if that is the case, and sym != "", then sym, acc = pkg, sym + pkgAmbiguous bool } -func parseArgParts(args []string) []argPart { - parsed := make([]argPart, 0, 3) +func parseArgs(args []string) (docArgs, bool) { switch len(args) { case 0: - parsed = append(parsed, argPart{val: ".", typ: argPkg}) + return docArgs{pkg: "."}, true case 1: // allowed syntaxes (acc is method or field, [] marks optional): // @@ -340,59 +275,47 @@ func parseArgParts(args []string) []argPart { // special handling for common ., .. and ./.. // these will generally work poorly if you try to use the one-argument // syntax to access a symbol/accessible. - parsed = append(parsed, argPart{val: args[0], typ: argPkg}) - break + return docArgs{pkg: args[0]}, true } switch strings.Count(args[0][slash+1:], ".") { case 0: - t := argPkg | argSym | argAcc if slash != -1 { - t = argPkg + return docArgs{pkg: args[0]}, true } - parsed = append(parsed, argPart{args[0], t}) + return docArgs{pkg: args[0], pkgAmbiguous: true}, true case 1: pos := strings.IndexByte(args[0][slash+1:], '.') + slash + 1 - // pkg.sym, pkg.acc, sym.acc - t1, t2 := argPkg|argSym, argSym|argAcc if slash != -1 { - t1 = argPkg - } else if token.IsExported(args[0]) { + return docArgs{pkg: args[0][:pos], sym: args[0][pos+1:]}, true + } + if token.IsExported(args[0]) { // See rationale here: // https://github.com/golang/go/blob/90dde5dec1126ddf2236730ec57511ced56a512d/src/cmd/doc/main.go#L265 - t1, t2 = argSym, argAcc + return docArgs{pkg: ".", sym: args[0][:pos], acc: args[0][pos+1:]}, true } - parsed = append(parsed, - argPart{args[0][:pos], t1}, - argPart{args[0][pos+1:], t2}, - ) + return docArgs{pkg: args[0][:pos], sym: args[0][pos+1:], pkgAmbiguous: true}, true case 2: // pkg.sym.acc parts := strings.Split(args[0][slash+1:], ".") - parsed = append(parsed, - argPart{args[0][:slash+1] + parts[0], argPkg}, - argPart{parts[1], argSym}, - argPart{parts[2], argAcc}, - ) + return docArgs{ + pkg: args[0][:slash+1] + parts[0], + sym: parts[1], + acc: parts[2], + }, true default: - return nil + return docArgs{}, false } case 2: - // , , . - parsed = append(parsed, argPart{args[0], argPkg}) switch strings.Count(args[1], ".") { case 0: - parsed = append(parsed, argPart{args[1], argSym | argAcc}) + return docArgs{pkg: args[0], sym: args[1]}, true case 1: pos := strings.IndexByte(args[1], '.') - parsed = append(parsed, - argPart{args[1][:pos], argSym}, - argPart{args[1][pos+1:], argAcc}, - ) + return docArgs{pkg: args[0], sym: args[1][:pos], acc: args[1][pos+1:]}, true default: - return nil + return docArgs{}, false } default: - return nil + return docArgs{}, false } - return parsed } diff --git a/pkgs/commands/doc/doc_test.go b/pkgs/commands/doc/doc_test.go new file mode 100644 index 00000000000..6f3f96f5145 --- /dev/null +++ b/pkgs/commands/doc/doc_test.go @@ -0,0 +1,153 @@ +package doc + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestResolveDocumentable(t *testing.T) { + p, err := os.Getwd() + require.NoError(t, err) + path := func(s string) string { return filepath.Join(p, "testdata/integ", s) } + dirs := NewDirs(path("")) + getDir := func(p string) Dir { return dirs.findDir(path(p))[0] } + pdata := func(p string, unexp bool) *pkgData { + pd, err := newPkgData(getDir(p), unexp) + require.NoError(t, err) + return pd + } + + tt := []struct { + name string + args []string + unexp bool + expect Documentable + errContains string + }{ + {"package", []string{"crypto/rand"}, false, &documentable{Dir: getDir("crypto/rand")}, ""}, + {"dir", []string{"./testdata/integ/crypto/rand"}, false, &documentable{Dir: getDir("crypto/rand")}, ""}, + {"dirAbs", []string{path("crypto/rand")}, false, &documentable{Dir: getDir("crypto/rand")}, ""}, + // test_notapkg exists in local dir and also path("test_notapkg"). + // ResolveDocumentable should first try local dir, and seeing as it is not a valid dir, try searching it as a package. + {"dirLocalMisleading", []string{"test_notapkg"}, false, &documentable{Dir: getDir("test_notapkg")}, ""}, + {"normalSymbol", + []string{"crypto/rand.Flag"}, false, + &documentable{Dir: getDir("crypto/rand"), symbol: "Flag", pkgData: pdata("crypto/rand", false)}, ""}, + {"normalAccessible", + []string{"crypto/rand.Name"}, false, + &documentable{Dir: getDir("crypto/rand"), symbol: "Name", pkgData: pdata("crypto/rand", false)}, ""}, + {"normalSymbolUnexp", + []string{"crypto/rand.unexp"}, true, + &documentable{Dir: getDir("crypto/rand"), symbol: "unexp", pkgData: pdata("crypto/rand", true)}, ""}, + {"normalAccessibleFull", + []string{"crypto/rand.Rand.Name"}, false, + &documentable{Dir: getDir("crypto/rand"), symbol: "Rand", accessible: "Name", pkgData: pdata("crypto/rand", false)}, ""}, + {"disambiguate", + []string{"rand.Flag"}, false, + &documentable{Dir: getDir("crypto/rand"), symbol: "Flag", pkgData: pdata("crypto/rand", false)}, ""}, + {"disambiguate2", + []string{"rand.Crypto"}, false, + &documentable{Dir: getDir("crypto/rand"), symbol: "Crypto", pkgData: pdata("crypto/rand", false)}, ""}, + {"disambiguate3", + []string{"rand.Normal"}, false, + &documentable{Dir: getDir("rand"), symbol: "Normal", pkgData: pdata("rand", false)}, ""}, + {"disambiguate4", // just "rand" should use the directory that matches it exactly. + []string{"rand"}, false, + &documentable{Dir: getDir("rand")}, ""}, + {"wdSymbol", + []string{"WdConst"}, false, + &documentable{Dir: getDir("wd"), symbol: "WdConst", pkgData: pdata("wd", false)}, ""}, + + {"errInvalidArgs", []string{"1", "2", "3"}, false, nil, "invalid arguments: [1 2 3]"}, + {"errNoCandidates", []string{"math", "Big"}, false, nil, `package not found: "math"`}, + {"errNoCandidates2", []string{"LocalSymbol"}, false, nil, `local packages are not yet supported`}, + {"errNoCandidates3", []string{"Symbol.Accessible"}, false, nil, `local packages are not yet supported`}, + {"errNonExisting", []string{"rand.NotExisting"}, false, nil, `could not resolve arguments`}, + {"errUnexp", []string{"crypto/rand.unexp"}, false, nil, "could not resolve arguments"}, + {"errDirNotapkg", []string{"./test_notapkg"}, false, nil, `package not found: "./test_notapkg"`}, + } + + for _, tc := range tt { + tc := tc + t.Run(tc.name, func(t *testing.T) { + // Wd prefix mean test relative to local directory - + // mock change local dir by setting the fpAbs variable (see doc.go) to match + // testdata/integ/wd when we call it on ".". + if strings.HasPrefix(tc.args[0], "Wd") { + fpAbs = func(s string) (string, error) { return filepath.Clean(filepath.Join(path("wd"), s)), nil } + defer func() { fpAbs = filepath.Abs }() + } + result, err := ResolveDocumentable(dirs, tc.args, tc.unexp) + // we use stripFset because d.pkgData.fset contians sync/atomic values, + // which in turn makes reflect.DeepEqual compare the two sync.Atomic values. + assert.Equal(t, stripFset(tc.expect), stripFset(result), "documentables should match") + if tc.errContains == "" { + assert.NoError(t, err) + } else { + assert.ErrorContains(t, err, tc.errContains) + } + }) + } +} + +func stripFset(p Documentable) Documentable { + if d, ok := p.(*documentable); ok && d.pkgData != nil { + d.pkgData.fset = nil + } + return p +} + +func Test_parseArgParts(t *testing.T) { + tt := []struct { + name string + args []string + exp *docArgs + }{ + {"noArgs", []string{}, &docArgs{pkg: "."}}, + + {"oneAmbiguous", []string{"ambiguous"}, &docArgs{pkg: "ambiguous", pkgAmbiguous: true}}, + {"onePath", []string{"pkg/path"}, &docArgs{pkg: "pkg/path"}}, + {"oneSpecial", []string{".."}, &docArgs{pkg: ".."}}, + {"oneSpecial2", []string{"../../../.."}, &docArgs{pkg: "../../../.."}}, + {"oneSpecial3", []string{"../upper/.."}, &docArgs{pkg: "../upper/.."}}, + {"oneSpecial4", []string{"."}, &docArgs{pkg: "."}}, + + {"twoPkgSym", []string{"pkg.sym"}, &docArgs{pkg: "pkg", sym: "sym", pkgAmbiguous: true}}, + {"twoPkgPathSym", []string{"path/pkg.sym"}, &docArgs{pkg: "path/pkg", sym: "sym"}}, + {"twoPkgUpperSym", []string{"../pkg.sym"}, &docArgs{pkg: "../pkg", sym: "sym"}}, + {"twoPkgExportedSym", []string{"Writer.Write"}, &docArgs{pkg: ".", sym: "Writer", acc: "Write"}}, + {"twoPkgCapitalPathSym", []string{"Path/Capitalised.Sym"}, &docArgs{pkg: "Path/Capitalised", sym: "Sym"}}, + + {"threePkgSymAcc", []string{"pkg.sym.acc"}, &docArgs{pkg: "pkg", sym: "sym", acc: "acc"}}, + {"threePathPkgSymAcc", []string{"./pkg.sym.acc"}, &docArgs{pkg: "./pkg", sym: "sym", acc: "acc"}}, + {"threePathPkgSymAcc2", []string{"../pkg.sym.acc"}, &docArgs{pkg: "../pkg", sym: "sym", acc: "acc"}}, + {"threePathPkgSymAcc3", []string{"path/to/pkg.sym.acc"}, &docArgs{pkg: "path/to/pkg", sym: "sym", acc: "acc"}}, + {"threePathPkgSymAcc4", []string{"path/../to/pkg.sym.acc"}, &docArgs{pkg: "path/../to/pkg", sym: "sym", acc: "acc"}}, + + // the logic on the split is pretty unambiguously that the first argument + // is the path, so we can afford to be less thorough on that regard. + {"splitTwo", []string{"io", "Writer"}, &docArgs{pkg: "io", sym: "Writer"}}, + {"splitThree", []string{"io", "Writer.Write"}, &docArgs{pkg: "io", sym: "Writer", acc: "Write"}}, + + {"errTooManyDots", []string{"io.Writer.Write.Impossible"}, nil}, + {"errTooManyDotsSplit", []string{"io", "Writer.Write.Impossible"}, nil}, + {"errTooManyArgs", []string{"io", "Writer", "Write"}, nil}, + } + for _, tc := range tt { + tc := tc + t.Run(tc.name, func(t *testing.T) { + p, ok := parseArgs(tc.args) + if ok { + _ = assert.NotNil(t, tc.exp, "parseArgs is successful when should have failed") && + assert.Equal(t, *tc.exp, p) + } else { + assert.Nil(t, tc.exp, "parseArgs is unsuccessful") + } + }) + } +} diff --git a/pkgs/commands/doc/pkg.go b/pkgs/commands/doc/pkg.go index 2ffdacd69a6..6cb17cdc258 100644 --- a/pkgs/commands/doc/pkg.go +++ b/pkgs/commands/doc/pkg.go @@ -110,9 +110,9 @@ func (pkg *pkgData) appendSpec(spec ast.Spec, unexported bool) { pkg.symbols = append(pkg.symbols, s.Name.Name) switch st := s.Type.(type) { case *ast.StructType: - pkg.appendFieldList(s.Name.Name, st.Fields) + pkg.appendFieldList(s.Name.Name, st.Fields, unexported) case *ast.InterfaceType: - pkg.appendFieldList(s.Name.Name, st.Methods) + pkg.appendFieldList(s.Name.Name, st.Methods, unexported) } case *ast.ValueSpec: for _, name := range s.Names { @@ -124,17 +124,24 @@ func (pkg *pkgData) appendSpec(spec ast.Spec, unexported bool) { } } -func (pkg *pkgData) appendFieldList(tName string, fl *ast.FieldList) { +func (pkg *pkgData) appendFieldList(tName string, fl *ast.FieldList, unexported bool) { if fl == nil { return } for _, field := range fl.List { if field.Names == nil { + embName := typeExprString(field.Type) + if !unexported && !token.IsExported(embName) { + continue + } // embedded struct - pkg.symbols = append(pkg.symbols, tName+"."+typeExprString(field.Type)) + pkg.symbols = append(pkg.symbols, tName+"."+embName) continue } for _, name := range field.Names { + if !unexported && !token.IsExported(name.Name) { + continue + } pkg.symbols = append(pkg.symbols, tName+"."+name.Name) } } diff --git a/pkgs/commands/doc/test_notapkg/.keep b/pkgs/commands/doc/test_notapkg/.keep new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkgs/commands/doc/testdata/dirs/crypto/crypto.gno b/pkgs/commands/doc/testdata/dirs/crypto/crypto.gno new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkgs/commands/doc/testdata/dirs/crypto/rand/rand.gno b/pkgs/commands/doc/testdata/dirs/crypto/rand/rand.gno new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkgs/commands/doc/testdata/dirs/crypto/testdata/rand/ignored.gno b/pkgs/commands/doc/testdata/dirs/crypto/testdata/rand/ignored.gno new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkgs/commands/doc/testdata/dirs/math/math.gno b/pkgs/commands/doc/testdata/dirs/math/math.gno new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkgs/commands/doc/testdata/dirs/math/rand/rand.gno b/pkgs/commands/doc/testdata/dirs/math/rand/rand.gno new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkgs/commands/doc/testdata/dirs/rand/rand.gno b/pkgs/commands/doc/testdata/dirs/rand/rand.gno new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkgs/commands/doc/testdata/integ/crypto/rand/rand.gno b/pkgs/commands/doc/testdata/integ/crypto/rand/rand.gno new file mode 100644 index 00000000000..c8903cb05c7 --- /dev/null +++ b/pkgs/commands/doc/testdata/integ/crypto/rand/rand.gno @@ -0,0 +1,29 @@ +package rand + +// Crypto symbol. +func Crypto() {} + +// A Rand is a test symbol for structs. +type Rand struct { + Name string // comment1 + Value int // comment2 + Attempts bool // comment3 + unexp chan int // comment4 +} + +// NewRand generates a new Rand. +func NewRand() *Rand { + return nil +} + +// Flag is tested for constant doc. +type Flag int + +// Common flag values. +const ( + FlagA = 1 << iota + FlagB + FlagC +) + +var unexp = 1 diff --git a/pkgs/commands/doc/testdata/integ/rand/rand.gno b/pkgs/commands/doc/testdata/integ/rand/rand.gno new file mode 100644 index 00000000000..7e8c41f0db7 --- /dev/null +++ b/pkgs/commands/doc/testdata/integ/rand/rand.gno @@ -0,0 +1,5 @@ +package rand + +// Normal symbol. +func Normal() { +} diff --git a/pkgs/commands/doc/testdata/integ/test_notapkg/a.gno b/pkgs/commands/doc/testdata/integ/test_notapkg/a.gno new file mode 100644 index 00000000000..bd4a4e79f49 --- /dev/null +++ b/pkgs/commands/doc/testdata/integ/test_notapkg/a.gno @@ -0,0 +1,3 @@ +package notapkg + +var I int diff --git a/pkgs/commands/doc/testdata/integ/wd/wd.gno b/pkgs/commands/doc/testdata/integ/wd/wd.gno new file mode 100644 index 00000000000..23fbddf63ea --- /dev/null +++ b/pkgs/commands/doc/testdata/integ/wd/wd.gno @@ -0,0 +1,13 @@ +package wd + +// Used for testing symbols relative to local dir. + +const WdConst = 1 + +var WdVar = 1 + +type WdType int + +func (WdType) WdMethod() {} + +func WdFunc() {} From 46385a1606510b8710e5304ba72041fd0a0cdc90 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Wed, 22 Mar 2023 20:46:37 +0100 Subject: [PATCH 08/24] tests: add tests for Document --- pkgs/commands/doc/doc.go | 15 ++--- pkgs/commands/doc/doc_test.go | 59 ++++++++++++++++++- pkgs/commands/doc/pkg.go | 46 +++++++++++---- pkgs/commands/doc/print.go | 9 ++- .../doc/testdata/integ/crypto/rand/rand.gno | 24 +++++++- 5 files changed, 127 insertions(+), 26 deletions(-) diff --git a/pkgs/commands/doc/doc.go b/pkgs/commands/doc/doc.go index a622ccba7d0..cf5b6316901 100644 --- a/pkgs/commands/doc/doc.go +++ b/pkgs/commands/doc/doc.go @@ -141,9 +141,6 @@ func (d *documentable) output(pp *pkgPrinter) (err error) { return } pp.packageDoc() - case d.symbol == "" && d.accessible != "": - d.symbol, d.accessible = d.accessible, "" - fallthrough case d.symbol != "" && d.accessible == "": pp.symbolDoc(d.symbol) default: // both non-empty @@ -212,12 +209,16 @@ func resolveDocumentable(dirs *Dirs, parsed docArgs, unexported bool) (Documenta accessible: parsed.acc, } - var matchFunc func(s string) bool + var matchFunc func(s symbolData) bool if parsed.acc == "" { - matchFunc = func(s string) bool { return s == parsed.sym || strings.HasSuffix(s, "."+parsed.sym) } + matchFunc = func(s symbolData) bool { + return (s.accessible == "" && symbolMatch(parsed.sym, s.symbol)) || + (s.typ == symbolDataMethod && symbolMatch(parsed.sym, s.accessible)) + } } else { - full := parsed.sym + "." + parsed.acc - matchFunc = func(s string) bool { return s == full } + matchFunc = func(s symbolData) bool { + return symbolMatch(parsed.sym, s.symbol) && symbolMatch(parsed.acc, s.accessible) + } } var errs []error diff --git a/pkgs/commands/doc/doc_test.go b/pkgs/commands/doc/doc_test.go index 6f3f96f5145..d323b7f8f86 100644 --- a/pkgs/commands/doc/doc_test.go +++ b/pkgs/commands/doc/doc_test.go @@ -1,6 +1,7 @@ package doc import ( + "bytes" "os" "path/filepath" "strings" @@ -39,8 +40,8 @@ func TestResolveDocumentable(t *testing.T) { []string{"crypto/rand.Flag"}, false, &documentable{Dir: getDir("crypto/rand"), symbol: "Flag", pkgData: pdata("crypto/rand", false)}, ""}, {"normalAccessible", - []string{"crypto/rand.Name"}, false, - &documentable{Dir: getDir("crypto/rand"), symbol: "Name", pkgData: pdata("crypto/rand", false)}, ""}, + []string{"crypto/rand.Generate"}, false, + &documentable{Dir: getDir("crypto/rand"), symbol: "Generate", pkgData: pdata("crypto/rand", false)}, ""}, {"normalSymbolUnexp", []string{"crypto/rand.unexp"}, true, &documentable{Dir: getDir("crypto/rand"), symbol: "unexp", pkgData: pdata("crypto/rand", true)}, ""}, @@ -83,7 +84,7 @@ func TestResolveDocumentable(t *testing.T) { defer func() { fpAbs = filepath.Abs }() } result, err := ResolveDocumentable(dirs, tc.args, tc.unexp) - // we use stripFset because d.pkgData.fset contians sync/atomic values, + // we use stripFset because d.pkgData.fset contains sync/atomic values, // which in turn makes reflect.DeepEqual compare the two sync.Atomic values. assert.Equal(t, stripFset(tc.expect), stripFset(result), "documentables should match") if tc.errContains == "" { @@ -102,6 +103,58 @@ func stripFset(p Documentable) Documentable { return p } +func TestDocument(t *testing.T) { + // the format itself can change if the design is to be changed, + // we want to make sure that given information is avaiable when calling + // Document. + abspath, err := filepath.Abs("./testdata/integ/crypto/rand") + require.NoError(t, err) + dir := Dir{ + importPath: "crypto/rand", + dir: abspath, + } + + tt := []struct { + name string + d *documentable + opts []DocumentOption + contains []string + }{ + {"base", &documentable{Dir: dir}, nil, []string{"func Crypto", "!Crypto symbol", "func NewRand", "!unexp", "type Flag", "!Name"}}, + {"func", &documentable{Dir: dir, symbol: "crypto"}, nil, []string{"Crypto symbol", "func Crypto", "!func NewRand", "!type Flag"}}, + {"funcWriter", &documentable{Dir: dir, symbol: "NewWriter"}, nil, []string{"func NewWriter() io.Writer", "!func Crypto"}}, + {"tp", &documentable{Dir: dir, symbol: "Rand"}, nil, []string{"type Rand", "comment1", "!func Crypto", "!unexp ", "!comment4", "Has unexported"}}, + {"tpField", &documentable{Dir: dir, symbol: "Rand", accessible: "Value"}, nil, []string{"type Rand", "!comment1", "comment2", "!func Crypto", "!unexp", "elided"}}, + {"tpUnexp", + &documentable{Dir: dir, symbol: "Rand"}, []DocumentOption{WithUnexported(true)}, + []string{"type Rand", "comment1", "!func Crypto", "unexp ", "comment4", "!Has unexported"}}, + {"symUnexp", + &documentable{Dir: dir, symbol: "unexp"}, []DocumentOption{WithUnexported(true)}, + []string{"var unexp", "!type Rand", "!comment1", "!comment4", "!func Crypto", "!Has unexported"}}, + {"fieldUnexp", + &documentable{Dir: dir, symbol: "Rand", accessible: "unexp"}, []DocumentOption{WithUnexported(true)}, + []string{"type Rand", "!comment1", "comment4", "!func Crypto", "elided", "!Has unexported"}}, + } + + buf := &bytes.Buffer{} + for _, tc := range tt { + tc := tc + t.Run(tc.name, func(t *testing.T) { + buf.Reset() + err := tc.d.Document(append(tc.opts, WithWriter(buf))...) + require.NoError(t, err) + s := buf.String() + for _, c := range tc.contains { + if c[0] == '!' { + assert.NotContains(t, s, c[1:]) + } else { + assert.Contains(t, s, c) + } + } + }) + } +} + func Test_parseArgParts(t *testing.T) { tt := []struct { name string diff --git a/pkgs/commands/doc/pkg.go b/pkgs/commands/doc/pkg.go index 6cb17cdc258..70771dc5d58 100644 --- a/pkgs/commands/doc/pkg.go +++ b/pkgs/commands/doc/pkg.go @@ -17,7 +17,22 @@ type pkgData struct { fset *token.FileSet files []*ast.File testFiles []*ast.File - symbols []string + symbols []symbolData +} + +const ( + symbolDataValue byte = iota + symbolDataType + symbolDataFunc + symbolDataMethod + symbolDataStructField + symbolDataInterfaceMethod +) + +type symbolData struct { + symbol string + accessible string + typ byte } func newPkgData(dir Dir, unexported bool) (*pkgData, error) { @@ -84,14 +99,18 @@ func (pkg *pkgData) parseFile(fileName string, unexported bool) error { switch x := decl.(type) { case *ast.FuncDecl: // prepend receiver if this is a method - var rcv string + sd := symbolData{ + symbol: x.Name.Name, + typ: symbolDataFunc, + } if x.Recv != nil { - rcv = typeExprString(x.Recv.List[0].Type) + "." - if !unexported && !token.IsExported(rcv) { + sd.symbol, sd.accessible = typeExprString(x.Recv.List[0].Type), sd.symbol + if !unexported && !token.IsExported(sd.symbol) { continue } + sd.typ = symbolDataMethod } - pkg.symbols = append(pkg.symbols, rcv+x.Name.Name) + pkg.symbols = append(pkg.symbols, sd) case *ast.GenDecl: for _, spec := range x.Specs { pkg.appendSpec(spec, unexported) @@ -107,42 +126,45 @@ func (pkg *pkgData) appendSpec(spec ast.Spec, unexported bool) { if !unexported && !token.IsExported(s.Name.Name) { return } - pkg.symbols = append(pkg.symbols, s.Name.Name) + pkg.symbols = append(pkg.symbols, symbolData{symbol: s.Name.Name, typ: symbolDataType}) switch st := s.Type.(type) { case *ast.StructType: - pkg.appendFieldList(s.Name.Name, st.Fields, unexported) + pkg.appendFieldList(s.Name.Name, st.Fields, unexported, symbolDataStructField) case *ast.InterfaceType: - pkg.appendFieldList(s.Name.Name, st.Methods, unexported) + pkg.appendFieldList(s.Name.Name, st.Methods, unexported, symbolDataInterfaceMethod) } case *ast.ValueSpec: for _, name := range s.Names { if !unexported && !token.IsExported(name.Name) { continue } - pkg.symbols = append(pkg.symbols, name.Name) + pkg.symbols = append(pkg.symbols, symbolData{symbol: name.Name, typ: symbolDataValue}) } } } -func (pkg *pkgData) appendFieldList(tName string, fl *ast.FieldList, unexported bool) { +func (pkg *pkgData) appendFieldList(tName string, fl *ast.FieldList, unexported bool, typ byte) { if fl == nil { return } for _, field := range fl.List { if field.Names == nil { + if typ == symbolDataInterfaceMethod { + continue + } embName := typeExprString(field.Type) if !unexported && !token.IsExported(embName) { continue } // embedded struct - pkg.symbols = append(pkg.symbols, tName+"."+embName) + pkg.symbols = append(pkg.symbols, symbolData{symbol: tName, accessible: embName, typ: typ}) continue } for _, name := range field.Names { if !unexported && !token.IsExported(name.Name) { continue } - pkg.symbols = append(pkg.symbols, tName+"."+name.Name) + pkg.symbols = append(pkg.symbols, symbolData{symbol: tName, accessible: name.Name, typ: typ}) } } } diff --git a/pkgs/commands/doc/print.go b/pkgs/commands/doc/print.go index 638466dfa88..e811d2709eb 100644 --- a/pkgs/commands/doc/print.go +++ b/pkgs/commands/doc/print.go @@ -937,13 +937,16 @@ func (pkg *pkgPrinter) fieldDoc(symbol, field string) bool { return pkg.printFieldDoc(symbol, field) } -// match reports whether the user's symbol matches the program's. -// A lower-case character in the user's string matches either case in the program's. -// The program string must be exported. func (pkg *pkgPrinter) match(user, program string) bool { if !pkg.isExported(program) { return false } + return symbolMatch(user, program) +} + +// match reports whether the user's symbol matches the program's. +// A lower-case character in the user's string matches either case in the program's. +func symbolMatch(user, program string) bool { /* TODO: might be useful to add for tooling. if matchCase { return user == program diff --git a/pkgs/commands/doc/testdata/integ/crypto/rand/rand.gno b/pkgs/commands/doc/testdata/integ/crypto/rand/rand.gno index c8903cb05c7..f30d3557386 100644 --- a/pkgs/commands/doc/testdata/integ/crypto/rand/rand.gno +++ b/pkgs/commands/doc/testdata/integ/crypto/rand/rand.gno @@ -1,8 +1,14 @@ package rand +import ( + "io" +) + // Crypto symbol. func Crypto() {} +func NewWriter() io.Writer {} + // A Rand is a test symbol for structs. type Rand struct { Name string // comment1 @@ -11,19 +17,35 @@ type Rand struct { unexp chan int // comment4 } +type Rander interface { + Generate() +} + +type RandEmbedder struct { + A string + Rand +} + // NewRand generates a new Rand. func NewRand() *Rand { return nil } +func (*Rand) Generate() { +} + // Flag is tested for constant doc. type Flag int // Common flag values. const ( - FlagA = 1 << iota + FlagA Flag = 1 << iota FlagB FlagC ) +var FlagVar Flag = 9999 + +var ExportedVar = true + var unexp = 1 From b73161010d129dcf35fca5029ecde2d1783e78e8 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Wed, 22 Mar 2023 20:51:09 +0100 Subject: [PATCH 09/24] chore: fix typo --- pkgs/commands/doc/doc_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/commands/doc/doc_test.go b/pkgs/commands/doc/doc_test.go index d323b7f8f86..2d1fe5762d1 100644 --- a/pkgs/commands/doc/doc_test.go +++ b/pkgs/commands/doc/doc_test.go @@ -105,7 +105,7 @@ func stripFset(p Documentable) Documentable { func TestDocument(t *testing.T) { // the format itself can change if the design is to be changed, - // we want to make sure that given information is avaiable when calling + // we want to make sure that given information is available when calling // Document. abspath, err := filepath.Abs("./testdata/integ/crypto/rand") require.NoError(t, err) From 72f9d7fca7d8f75f5434a28af85477c805b919b9 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Tue, 18 Apr 2023 02:14:12 +0200 Subject: [PATCH 10/24] update to new dir structure --- gnovm/cmd/gno/doc.go | 6 +++--- {pkgs/commands => gnovm/pkg}/doc/dirs.go | 0 {pkgs/commands => gnovm/pkg}/doc/dirs_test.go | 0 {pkgs/commands => gnovm/pkg}/doc/doc.go | 0 {pkgs/commands => gnovm/pkg}/doc/doc_test.go | 0 {pkgs/commands => gnovm/pkg}/doc/pkg.go | 0 {pkgs/commands => gnovm/pkg}/doc/print.go | 0 {pkgs/commands => gnovm/pkg}/doc/test_notapkg/.keep | 0 .../pkg}/doc/testdata/dirs/crypto/crypto.gno | 0 .../pkg}/doc/testdata/dirs/crypto/rand/rand.gno | 0 .../pkg}/doc/testdata/dirs/crypto/testdata/rand/ignored.gno | 0 .../commands => gnovm/pkg}/doc/testdata/dirs/math/math.gno | 0 .../pkg}/doc/testdata/dirs/math/rand/rand.gno | 0 .../commands => gnovm/pkg}/doc/testdata/dirs/rand/rand.gno | 0 .../pkg}/doc/testdata/integ/crypto/rand/rand.gno | 0 .../commands => gnovm/pkg}/doc/testdata/integ/rand/rand.gno | 0 .../pkg}/doc/testdata/integ/test_notapkg/a.gno | 0 {pkgs/commands => gnovm/pkg}/doc/testdata/integ/wd/wd.gno | 0 18 files changed, 3 insertions(+), 3 deletions(-) rename {pkgs/commands => gnovm/pkg}/doc/dirs.go (100%) rename {pkgs/commands => gnovm/pkg}/doc/dirs_test.go (100%) rename {pkgs/commands => gnovm/pkg}/doc/doc.go (100%) rename {pkgs/commands => gnovm/pkg}/doc/doc_test.go (100%) rename {pkgs/commands => gnovm/pkg}/doc/pkg.go (100%) rename {pkgs/commands => gnovm/pkg}/doc/print.go (100%) rename {pkgs/commands => gnovm/pkg}/doc/test_notapkg/.keep (100%) rename {pkgs/commands => gnovm/pkg}/doc/testdata/dirs/crypto/crypto.gno (100%) rename {pkgs/commands => gnovm/pkg}/doc/testdata/dirs/crypto/rand/rand.gno (100%) rename {pkgs/commands => gnovm/pkg}/doc/testdata/dirs/crypto/testdata/rand/ignored.gno (100%) rename {pkgs/commands => gnovm/pkg}/doc/testdata/dirs/math/math.gno (100%) rename {pkgs/commands => gnovm/pkg}/doc/testdata/dirs/math/rand/rand.gno (100%) rename {pkgs/commands => gnovm/pkg}/doc/testdata/dirs/rand/rand.gno (100%) rename {pkgs/commands => gnovm/pkg}/doc/testdata/integ/crypto/rand/rand.gno (100%) rename {pkgs/commands => gnovm/pkg}/doc/testdata/integ/rand/rand.gno (100%) rename {pkgs/commands => gnovm/pkg}/doc/testdata/integ/test_notapkg/a.gno (100%) rename {pkgs/commands => gnovm/pkg}/doc/testdata/integ/wd/wd.gno (100%) diff --git a/gnovm/cmd/gno/doc.go b/gnovm/cmd/gno/doc.go index 4441873e108..e3147ce98b7 100644 --- a/gnovm/cmd/gno/doc.go +++ b/gnovm/cmd/gno/doc.go @@ -5,8 +5,8 @@ import ( "flag" "path/filepath" - "github.com/gnolang/gno/pkgs/commands" - "github.com/gnolang/gno/pkgs/commands/doc" + "github.com/gnolang/gno/gnovm/pkg/doc" + "github.com/gnolang/gno/tm2/pkg/commands" ) type docCfg struct { @@ -61,7 +61,7 @@ func execDoc(cfg *docCfg, args []string, io *commands.IO) error { if cfg.rootDir == "" { cfg.rootDir = guessRootDir() } - dirs := doc.NewDirs(filepath.Join(cfg.rootDir, "stdlibs"), filepath.Join(cfg.rootDir, "examples")) + dirs := doc.NewDirs(filepath.Join(cfg.rootDir, "gnovm/stdlibs"), filepath.Join(cfg.rootDir, "examples")) res, err := doc.ResolveDocumentable(dirs, args, cfg.unexported) switch { case res == nil: diff --git a/pkgs/commands/doc/dirs.go b/gnovm/pkg/doc/dirs.go similarity index 100% rename from pkgs/commands/doc/dirs.go rename to gnovm/pkg/doc/dirs.go diff --git a/pkgs/commands/doc/dirs_test.go b/gnovm/pkg/doc/dirs_test.go similarity index 100% rename from pkgs/commands/doc/dirs_test.go rename to gnovm/pkg/doc/dirs_test.go diff --git a/pkgs/commands/doc/doc.go b/gnovm/pkg/doc/doc.go similarity index 100% rename from pkgs/commands/doc/doc.go rename to gnovm/pkg/doc/doc.go diff --git a/pkgs/commands/doc/doc_test.go b/gnovm/pkg/doc/doc_test.go similarity index 100% rename from pkgs/commands/doc/doc_test.go rename to gnovm/pkg/doc/doc_test.go diff --git a/pkgs/commands/doc/pkg.go b/gnovm/pkg/doc/pkg.go similarity index 100% rename from pkgs/commands/doc/pkg.go rename to gnovm/pkg/doc/pkg.go diff --git a/pkgs/commands/doc/print.go b/gnovm/pkg/doc/print.go similarity index 100% rename from pkgs/commands/doc/print.go rename to gnovm/pkg/doc/print.go diff --git a/pkgs/commands/doc/test_notapkg/.keep b/gnovm/pkg/doc/test_notapkg/.keep similarity index 100% rename from pkgs/commands/doc/test_notapkg/.keep rename to gnovm/pkg/doc/test_notapkg/.keep diff --git a/pkgs/commands/doc/testdata/dirs/crypto/crypto.gno b/gnovm/pkg/doc/testdata/dirs/crypto/crypto.gno similarity index 100% rename from pkgs/commands/doc/testdata/dirs/crypto/crypto.gno rename to gnovm/pkg/doc/testdata/dirs/crypto/crypto.gno diff --git a/pkgs/commands/doc/testdata/dirs/crypto/rand/rand.gno b/gnovm/pkg/doc/testdata/dirs/crypto/rand/rand.gno similarity index 100% rename from pkgs/commands/doc/testdata/dirs/crypto/rand/rand.gno rename to gnovm/pkg/doc/testdata/dirs/crypto/rand/rand.gno diff --git a/pkgs/commands/doc/testdata/dirs/crypto/testdata/rand/ignored.gno b/gnovm/pkg/doc/testdata/dirs/crypto/testdata/rand/ignored.gno similarity index 100% rename from pkgs/commands/doc/testdata/dirs/crypto/testdata/rand/ignored.gno rename to gnovm/pkg/doc/testdata/dirs/crypto/testdata/rand/ignored.gno diff --git a/pkgs/commands/doc/testdata/dirs/math/math.gno b/gnovm/pkg/doc/testdata/dirs/math/math.gno similarity index 100% rename from pkgs/commands/doc/testdata/dirs/math/math.gno rename to gnovm/pkg/doc/testdata/dirs/math/math.gno diff --git a/pkgs/commands/doc/testdata/dirs/math/rand/rand.gno b/gnovm/pkg/doc/testdata/dirs/math/rand/rand.gno similarity index 100% rename from pkgs/commands/doc/testdata/dirs/math/rand/rand.gno rename to gnovm/pkg/doc/testdata/dirs/math/rand/rand.gno diff --git a/pkgs/commands/doc/testdata/dirs/rand/rand.gno b/gnovm/pkg/doc/testdata/dirs/rand/rand.gno similarity index 100% rename from pkgs/commands/doc/testdata/dirs/rand/rand.gno rename to gnovm/pkg/doc/testdata/dirs/rand/rand.gno diff --git a/pkgs/commands/doc/testdata/integ/crypto/rand/rand.gno b/gnovm/pkg/doc/testdata/integ/crypto/rand/rand.gno similarity index 100% rename from pkgs/commands/doc/testdata/integ/crypto/rand/rand.gno rename to gnovm/pkg/doc/testdata/integ/crypto/rand/rand.gno diff --git a/pkgs/commands/doc/testdata/integ/rand/rand.gno b/gnovm/pkg/doc/testdata/integ/rand/rand.gno similarity index 100% rename from pkgs/commands/doc/testdata/integ/rand/rand.gno rename to gnovm/pkg/doc/testdata/integ/rand/rand.gno diff --git a/pkgs/commands/doc/testdata/integ/test_notapkg/a.gno b/gnovm/pkg/doc/testdata/integ/test_notapkg/a.gno similarity index 100% rename from pkgs/commands/doc/testdata/integ/test_notapkg/a.gno rename to gnovm/pkg/doc/testdata/integ/test_notapkg/a.gno diff --git a/pkgs/commands/doc/testdata/integ/wd/wd.gno b/gnovm/pkg/doc/testdata/integ/wd/wd.gno similarity index 100% rename from pkgs/commands/doc/testdata/integ/wd/wd.gno rename to gnovm/pkg/doc/testdata/integ/wd/wd.gno From 3d2168e3bdd620d2c729044286cc45443fc90d0f Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Tue, 18 Apr 2023 14:27:32 +0200 Subject: [PATCH 11/24] fmt --- gnovm/pkg/doc/doc_test.go | 108 +++++++++++++++++++++++++------------- 1 file changed, 72 insertions(+), 36 deletions(-) diff --git a/gnovm/pkg/doc/doc_test.go b/gnovm/pkg/doc/doc_test.go index 2d1fe5762d1..93a986ca8a4 100644 --- a/gnovm/pkg/doc/doc_test.go +++ b/gnovm/pkg/doc/doc_test.go @@ -36,33 +36,60 @@ func TestResolveDocumentable(t *testing.T) { // test_notapkg exists in local dir and also path("test_notapkg"). // ResolveDocumentable should first try local dir, and seeing as it is not a valid dir, try searching it as a package. {"dirLocalMisleading", []string{"test_notapkg"}, false, &documentable{Dir: getDir("test_notapkg")}, ""}, - {"normalSymbol", - []string{"crypto/rand.Flag"}, false, - &documentable{Dir: getDir("crypto/rand"), symbol: "Flag", pkgData: pdata("crypto/rand", false)}, ""}, - {"normalAccessible", - []string{"crypto/rand.Generate"}, false, - &documentable{Dir: getDir("crypto/rand"), symbol: "Generate", pkgData: pdata("crypto/rand", false)}, ""}, - {"normalSymbolUnexp", - []string{"crypto/rand.unexp"}, true, - &documentable{Dir: getDir("crypto/rand"), symbol: "unexp", pkgData: pdata("crypto/rand", true)}, ""}, - {"normalAccessibleFull", - []string{"crypto/rand.Rand.Name"}, false, - &documentable{Dir: getDir("crypto/rand"), symbol: "Rand", accessible: "Name", pkgData: pdata("crypto/rand", false)}, ""}, - {"disambiguate", - []string{"rand.Flag"}, false, - &documentable{Dir: getDir("crypto/rand"), symbol: "Flag", pkgData: pdata("crypto/rand", false)}, ""}, - {"disambiguate2", - []string{"rand.Crypto"}, false, - &documentable{Dir: getDir("crypto/rand"), symbol: "Crypto", pkgData: pdata("crypto/rand", false)}, ""}, - {"disambiguate3", - []string{"rand.Normal"}, false, - &documentable{Dir: getDir("rand"), symbol: "Normal", pkgData: pdata("rand", false)}, ""}, - {"disambiguate4", // just "rand" should use the directory that matches it exactly. - []string{"rand"}, false, - &documentable{Dir: getDir("rand")}, ""}, - {"wdSymbol", - []string{"WdConst"}, false, - &documentable{Dir: getDir("wd"), symbol: "WdConst", pkgData: pdata("wd", false)}, ""}, + { + "normalSymbol", + []string{"crypto/rand.Flag"}, + false, + &documentable{Dir: getDir("crypto/rand"), symbol: "Flag", pkgData: pdata("crypto/rand", false)}, "", + }, + { + "normalAccessible", + []string{"crypto/rand.Generate"}, + false, + &documentable{Dir: getDir("crypto/rand"), symbol: "Generate", pkgData: pdata("crypto/rand", false)}, "", + }, + { + "normalSymbolUnexp", + []string{"crypto/rand.unexp"}, + true, + &documentable{Dir: getDir("crypto/rand"), symbol: "unexp", pkgData: pdata("crypto/rand", true)}, "", + }, + { + "normalAccessibleFull", + []string{"crypto/rand.Rand.Name"}, + false, + &documentable{Dir: getDir("crypto/rand"), symbol: "Rand", accessible: "Name", pkgData: pdata("crypto/rand", false)}, "", + }, + { + "disambiguate", + []string{"rand.Flag"}, + false, + &documentable{Dir: getDir("crypto/rand"), symbol: "Flag", pkgData: pdata("crypto/rand", false)}, "", + }, + { + "disambiguate2", + []string{"rand.Crypto"}, + false, + &documentable{Dir: getDir("crypto/rand"), symbol: "Crypto", pkgData: pdata("crypto/rand", false)}, "", + }, + { + "disambiguate3", + []string{"rand.Normal"}, + false, + &documentable{Dir: getDir("rand"), symbol: "Normal", pkgData: pdata("rand", false)}, "", + }, + { + "disambiguate4", // just "rand" should use the directory that matches it exactly. + []string{"rand"}, + false, + &documentable{Dir: getDir("rand")}, "", + }, + { + "wdSymbol", + []string{"WdConst"}, + false, + &documentable{Dir: getDir("wd"), symbol: "WdConst", pkgData: pdata("wd", false)}, "", + }, {"errInvalidArgs", []string{"1", "2", "3"}, false, nil, "invalid arguments: [1 2 3]"}, {"errNoCandidates", []string{"math", "Big"}, false, nil, `package not found: "math"`}, @@ -125,15 +152,24 @@ func TestDocument(t *testing.T) { {"funcWriter", &documentable{Dir: dir, symbol: "NewWriter"}, nil, []string{"func NewWriter() io.Writer", "!func Crypto"}}, {"tp", &documentable{Dir: dir, symbol: "Rand"}, nil, []string{"type Rand", "comment1", "!func Crypto", "!unexp ", "!comment4", "Has unexported"}}, {"tpField", &documentable{Dir: dir, symbol: "Rand", accessible: "Value"}, nil, []string{"type Rand", "!comment1", "comment2", "!func Crypto", "!unexp", "elided"}}, - {"tpUnexp", - &documentable{Dir: dir, symbol: "Rand"}, []DocumentOption{WithUnexported(true)}, - []string{"type Rand", "comment1", "!func Crypto", "unexp ", "comment4", "!Has unexported"}}, - {"symUnexp", - &documentable{Dir: dir, symbol: "unexp"}, []DocumentOption{WithUnexported(true)}, - []string{"var unexp", "!type Rand", "!comment1", "!comment4", "!func Crypto", "!Has unexported"}}, - {"fieldUnexp", - &documentable{Dir: dir, symbol: "Rand", accessible: "unexp"}, []DocumentOption{WithUnexported(true)}, - []string{"type Rand", "!comment1", "comment4", "!func Crypto", "elided", "!Has unexported"}}, + { + "tpUnexp", + &documentable{Dir: dir, symbol: "Rand"}, + []DocumentOption{WithUnexported(true)}, + []string{"type Rand", "comment1", "!func Crypto", "unexp ", "comment4", "!Has unexported"}, + }, + { + "symUnexp", + &documentable{Dir: dir, symbol: "unexp"}, + []DocumentOption{WithUnexported(true)}, + []string{"var unexp", "!type Rand", "!comment1", "!comment4", "!func Crypto", "!Has unexported"}, + }, + { + "fieldUnexp", + &documentable{Dir: dir, symbol: "Rand", accessible: "unexp"}, + []DocumentOption{WithUnexported(true)}, + []string{"type Rand", "!comment1", "comment4", "!func Crypto", "elided", "!Has unexported"}, + }, } buf := &bytes.Buffer{} From d7a296d0a2e164b58befdd6716e820b783bc02f0 Mon Sep 17 00:00:00 2001 From: Morgan Date: Tue, 18 Apr 2023 20:13:01 +0200 Subject: [PATCH 12/24] Update gnovm/cmd/gno/doc.go Co-authored-by: Manfred Touron <94029+moul@users.noreply.github.com> --- gnovm/cmd/gno/doc.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gnovm/cmd/gno/doc.go b/gnovm/cmd/gno/doc.go index e3147ce98b7..8d6a6dfc3e8 100644 --- a/gnovm/cmd/gno/doc.go +++ b/gnovm/cmd/gno/doc.go @@ -63,10 +63,10 @@ func execDoc(cfg *docCfg, args []string, io *commands.IO) error { } dirs := doc.NewDirs(filepath.Join(cfg.rootDir, "gnovm/stdlibs"), filepath.Join(cfg.rootDir, "examples")) res, err := doc.ResolveDocumentable(dirs, args, cfg.unexported) - switch { - case res == nil: + if res == nil { return err - case err != nil: + } + if err != nil { io.Printfln("warning: error parsing some candidate packages:\n%v", err) } err = res.Document( From e6c21d5ae7d7420a3196a311fc063953d2b7c9d7 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Tue, 18 Apr 2023 20:17:47 +0200 Subject: [PATCH 13/24] typo --- gnovm/pkg/doc/dirs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnovm/pkg/doc/dirs.go b/gnovm/pkg/doc/dirs.go index 9963073d649..fa53a9fe2f3 100644 --- a/gnovm/pkg/doc/dirs.go +++ b/gnovm/pkg/doc/dirs.go @@ -70,7 +70,7 @@ func (d *Dirs) walk(roots []string) { } // bfsWalkRoot walks a single directory hierarchy in breadth-first lexical order. -// Each Go source directory it finds is delivered on d.scan. +// Each Gno source directory it finds is delivered on d.scan. func (d *Dirs) bfsWalkRoot(root string) { root = filepath.Clean(root) From ec49331f4a4878add0d903fa9172a7450e0c0c28 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Tue, 18 Apr 2023 21:16:53 +0200 Subject: [PATCH 14/24] Revert "feat(gnodev): add preliminary doc command, refactor common flags verbose and root-dir" This reverts commit 243d24c2a202f182379a72209a3203bc5d082a74. --- gnovm/cmd/gno/build.go | 13 +++++++++---- gnovm/cmd/gno/doc.go | 9 +++++++-- gnovm/cmd/gno/precompile.go | 9 +++++++-- gnovm/cmd/gno/repl.go | 19 +++++++++++++++---- gnovm/cmd/gno/run.go | 19 +++++++++++++++---- gnovm/cmd/gno/test.go | 18 ++++++++++++++---- gnovm/cmd/gno/util.go | 27 --------------------------- 7 files changed, 67 insertions(+), 47 deletions(-) diff --git a/gnovm/cmd/gno/build.go b/gnovm/cmd/gno/build.go index f10954a99d6..b80711586e4 100644 --- a/gnovm/cmd/gno/build.go +++ b/gnovm/cmd/gno/build.go @@ -11,13 +11,13 @@ import ( ) type buildCfg struct { - verboseStruct + verbose bool goBinary string } var defaultBuildOptions = &buildCfg{ - verboseStruct: verboseStruct{false}, - goBinary: "go", + verbose: false, + goBinary: "go", } func newBuildCmd(io *commands.IO) *commands.Command { @@ -37,7 +37,12 @@ func newBuildCmd(io *commands.IO) *commands.Command { } func (c *buildCfg) RegisterFlags(fs *flag.FlagSet) { - c.verboseStruct.RegisterFlags(fs) + fs.BoolVar( + &c.verbose, + "verbose", + defaultBuildOptions.verbose, + "verbose output when building", + ) fs.StringVar( &c.goBinary, diff --git a/gnovm/cmd/gno/doc.go b/gnovm/cmd/gno/doc.go index e3147ce98b7..ea028f9b4f2 100644 --- a/gnovm/cmd/gno/doc.go +++ b/gnovm/cmd/gno/doc.go @@ -13,7 +13,7 @@ type docCfg struct { all bool src bool unexported bool - rootDirStruct + rootDir string } func newDocCmd(io *commands.IO) *commands.Command { @@ -53,7 +53,12 @@ func (c *docCfg) RegisterFlags(fs *flag.FlagSet) { "show unexported symbols as well as exported", ) - c.rootDirStruct.RegisterFlags(fs) + fs.StringVar( + &c.rootDir, + "root-dir", + "", + "clone location of github.com/gnolang/gno (gnodev tries to guess it)", + ) } func execDoc(cfg *docCfg, args []string, io *commands.IO) error { diff --git a/gnovm/cmd/gno/precompile.go b/gnovm/cmd/gno/precompile.go index fc6c1a8b451..7e895109706 100644 --- a/gnovm/cmd/gno/precompile.go +++ b/gnovm/cmd/gno/precompile.go @@ -15,7 +15,7 @@ import ( type importPath string type precompileCfg struct { - verboseStruct + verbose bool skipFmt bool skipImports bool goBinary string @@ -64,7 +64,12 @@ func newPrecompileCmd(io *commands.IO) *commands.Command { } func (c *precompileCfg) RegisterFlags(fs *flag.FlagSet) { - c.verboseStruct.RegisterFlags(fs) + fs.BoolVar( + &c.verbose, + "verbose", + false, + "verbose output when running", + ) fs.BoolVar( &c.skipFmt, diff --git a/gnovm/cmd/gno/repl.go b/gnovm/cmd/gno/repl.go index e1b56f3569b..c0debc332f1 100644 --- a/gnovm/cmd/gno/repl.go +++ b/gnovm/cmd/gno/repl.go @@ -15,8 +15,8 @@ import ( ) type replCfg struct { - verboseStruct - rootDirStruct + verbose bool + rootDir string } func newReplCmd() *commands.Command { @@ -36,8 +36,19 @@ func newReplCmd() *commands.Command { } func (c *replCfg) RegisterFlags(fs *flag.FlagSet) { - c.verboseStruct.RegisterFlags(fs) - c.rootDirStruct.RegisterFlags(fs) + fs.BoolVar( + &c.verbose, + "verbose", + false, + "verbose output when running", + ) + + fs.StringVar( + &c.rootDir, + "root-dir", + "", + "clone location of github.com/gnolang/gno (gnodev tries to guess it)", + ) } func execRepl(cfg *replCfg, args []string) error { diff --git a/gnovm/cmd/gno/run.go b/gnovm/cmd/gno/run.go index cac5f4b14a6..f0c4a1fb290 100644 --- a/gnovm/cmd/gno/run.go +++ b/gnovm/cmd/gno/run.go @@ -10,8 +10,8 @@ import ( ) type runCfg struct { - verboseStruct - rootDirStruct + verbose bool + rootDir string } func newRunCmd(io *commands.IO) *commands.Command { @@ -31,8 +31,19 @@ func newRunCmd(io *commands.IO) *commands.Command { } func (c *runCfg) RegisterFlags(fs *flag.FlagSet) { - c.verboseStruct.RegisterFlags(fs) - c.rootDirStruct.RegisterFlags(fs) + fs.BoolVar( + &c.verbose, + "verbose", + false, + "verbose output when running", + ) + + fs.StringVar( + &c.rootDir, + "root-dir", + "", + "clone location of github.com/gnolang/gno (gnodev tries to guess it)", + ) } func execRun(cfg *runCfg, args []string, io *commands.IO) error { diff --git a/gnovm/cmd/gno/test.go b/gnovm/cmd/gno/test.go index d41f80aad4d..d5da5c10347 100644 --- a/gnovm/cmd/gno/test.go +++ b/gnovm/cmd/gno/test.go @@ -24,8 +24,8 @@ import ( ) type testCfg struct { - verboseStruct - rootDirStruct + verbose bool + rootDir string run string timeout time.Duration precompile bool // TODO: precompile should be the default, but it needs to automatically precompile dependencies in memory. @@ -49,7 +49,12 @@ func newTestCmd(io *commands.IO) *commands.Command { } func (c *testCfg) RegisterFlags(fs *flag.FlagSet) { - c.verboseStruct.RegisterFlags(fs) + fs.BoolVar( + &c.verbose, + "verbose", + false, + "verbose output when running", + ) fs.BoolVar( &c.precompile, @@ -65,7 +70,12 @@ func (c *testCfg) RegisterFlags(fs *flag.FlagSet) { "writes actual as wanted in test comments", ) - c.rootDirStruct.RegisterFlags(fs) + fs.StringVar( + &c.rootDir, + "root-dir", + "", + "clone location of github.com/gnolang/gno (gnodev tries to guess it)", + ) fs.StringVar( &c.run, diff --git a/gnovm/cmd/gno/util.go b/gnovm/cmd/gno/util.go index b489e69dfe9..01ca51c27f1 100644 --- a/gnovm/cmd/gno/util.go +++ b/gnovm/cmd/gno/util.go @@ -1,7 +1,6 @@ package main import ( - "flag" "fmt" "go/ast" "io" @@ -16,32 +15,6 @@ import ( gno "github.com/gnolang/gno/gnovm/pkg/gnolang" ) -type rootDirStruct struct { - rootDir string -} - -func (c *rootDirStruct) RegisterFlags(fs *flag.FlagSet) { - fs.StringVar( - &c.rootDir, - "root-dir", - "", - "clone location of github.com/gnolang/gno (gnodev tries to guess it)", - ) -} - -type verboseStruct struct { - verbose bool -} - -func (c *verboseStruct) RegisterFlags(fs *flag.FlagSet) { - fs.BoolVar( - &c.verbose, - "verbose", - false, - "print verbose output", - ) -} - func isGnoFile(f fs.DirEntry) bool { name := f.Name() return !strings.HasPrefix(name, ".") && strings.HasSuffix(name, ".gno") && !f.IsDir() From f33e938660dec10eeed974b9dbccdb50fe40d758 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Tue, 18 Apr 2023 21:29:14 +0200 Subject: [PATCH 15/24] change documentable iface --- gnovm/cmd/gno/doc.go | 4 ++-- gnovm/pkg/doc/doc.go | 37 +++++++++++++++++-------------------- gnovm/pkg/doc/doc_test.go | 10 +++++----- gnovm/pkg/doc/pkg.go | 2 +- gnovm/pkg/doc/print.go | 2 +- 5 files changed, 26 insertions(+), 29 deletions(-) diff --git a/gnovm/cmd/gno/doc.go b/gnovm/cmd/gno/doc.go index e532b1a759c..ca24b118aec 100644 --- a/gnovm/cmd/gno/doc.go +++ b/gnovm/cmd/gno/doc.go @@ -74,11 +74,11 @@ func execDoc(cfg *docCfg, args []string, io *commands.IO) error { if err != nil { io.Printfln("warning: error parsing some candidate packages:\n%v", err) } - err = res.Document( + err = res.WriteDocumentation( + io.Out, doc.WithShowAll(cfg.all), doc.WithSource(cfg.src), doc.WithUnexported(cfg.unexported), - doc.WithWriter(io.Out), ) if err != nil { return err diff --git a/gnovm/pkg/doc/doc.go b/gnovm/pkg/doc/doc.go index cf5b6316901..cc6426ccdf1 100644 --- a/gnovm/pkg/doc/doc.go +++ b/gnovm/pkg/doc/doc.go @@ -18,10 +18,10 @@ import ( "go.uber.org/multierr" ) -// DocumentOption is used to pass options to the [Documentable].Document. -type DocumentOption func(s *documentOptions) +// WriteDocumentationOption is used to pass options to the [Documentable].WriteDocumentation. +type WriteDocumentationOption func(s *writeDocOptions) -type documentOptions struct { +type writeDocOptions struct { all bool src bool unexported bool @@ -30,36 +30,33 @@ type documentOptions struct { } // WithShowAll shows all symbols when displaying documentation about a package. -func WithShowAll(b bool) DocumentOption { - return func(s *documentOptions) { s.all = b } +func WithShowAll(b bool) WriteDocumentationOption { + return func(s *writeDocOptions) { s.all = b } } // WithSource shows source when documenting a symbol. -func WithSource(b bool) DocumentOption { - return func(s *documentOptions) { s.src = b } +func WithSource(b bool) WriteDocumentationOption { + return func(s *writeDocOptions) { s.src = b } } // WithUnexported shows unexported symbols as well as exported. -func WithUnexported(b bool) DocumentOption { - return func(s *documentOptions) { s.unexported = b } +func WithUnexported(b bool) WriteDocumentationOption { + return func(s *writeDocOptions) { s.unexported = b } } // WithShort shows a one-line representation for each symbol. -func WithShort(b bool) DocumentOption { - return func(s *documentOptions) { s.short = b } -} - -// WithWriter uses the given writer as an output. -// By default, os.Stdout is used. -func WithWriter(w io.Writer) DocumentOption { - return func(s *documentOptions) { s.w = w } +func WithShort(b bool) WriteDocumentationOption { + return func(s *writeDocOptions) { s.short = b } } // Documentable is a package, symbol, or accessible which can be documented. type Documentable interface { - Document(...DocumentOption) error + WriteDocumentation(w io.Writer, opts ...WriteDocumentationOption) error } +// static implementation check +var _ Documentable = (*documentable)(nil) + type documentable struct { Dir symbol string @@ -67,8 +64,8 @@ type documentable struct { pkgData *pkgData } -func (d *documentable) Document(opts ...DocumentOption) error { - o := &documentOptions{w: os.Stdout} +func (d *documentable) WriteDocumentation(w io.Writer, opts ...WriteDocumentationOption) error { + o := &writeDocOptions{w: w} for _, opt := range opts { opt(o) } diff --git a/gnovm/pkg/doc/doc_test.go b/gnovm/pkg/doc/doc_test.go index 93a986ca8a4..7b182985e2a 100644 --- a/gnovm/pkg/doc/doc_test.go +++ b/gnovm/pkg/doc/doc_test.go @@ -144,7 +144,7 @@ func TestDocument(t *testing.T) { tt := []struct { name string d *documentable - opts []DocumentOption + opts []WriteDocumentationOption contains []string }{ {"base", &documentable{Dir: dir}, nil, []string{"func Crypto", "!Crypto symbol", "func NewRand", "!unexp", "type Flag", "!Name"}}, @@ -155,19 +155,19 @@ func TestDocument(t *testing.T) { { "tpUnexp", &documentable{Dir: dir, symbol: "Rand"}, - []DocumentOption{WithUnexported(true)}, + []WriteDocumentationOption{WithUnexported(true)}, []string{"type Rand", "comment1", "!func Crypto", "unexp ", "comment4", "!Has unexported"}, }, { "symUnexp", &documentable{Dir: dir, symbol: "unexp"}, - []DocumentOption{WithUnexported(true)}, + []WriteDocumentationOption{WithUnexported(true)}, []string{"var unexp", "!type Rand", "!comment1", "!comment4", "!func Crypto", "!Has unexported"}, }, { "fieldUnexp", &documentable{Dir: dir, symbol: "Rand", accessible: "unexp"}, - []DocumentOption{WithUnexported(true)}, + []WriteDocumentationOption{WithUnexported(true)}, []string{"type Rand", "!comment1", "comment4", "!func Crypto", "elided", "!Has unexported"}, }, } @@ -177,7 +177,7 @@ func TestDocument(t *testing.T) { tc := tc t.Run(tc.name, func(t *testing.T) { buf.Reset() - err := tc.d.Document(append(tc.opts, WithWriter(buf))...) + err := tc.d.WriteDocumentation(buf, tc.opts...) require.NoError(t, err) s := buf.String() for _, c := range tc.contains { diff --git a/gnovm/pkg/doc/pkg.go b/gnovm/pkg/doc/pkg.go index 70771dc5d58..6f115057d71 100644 --- a/gnovm/pkg/doc/pkg.go +++ b/gnovm/pkg/doc/pkg.go @@ -183,7 +183,7 @@ func typeExprString(expr ast.Expr) string { return "" } -func (pkg *pkgData) docPackage(opts *documentOptions) (*ast.Package, *doc.Package, error) { +func (pkg *pkgData) docPackage(opts *writeDocOptions) (*ast.Package, *doc.Package, error) { // largely taken from go/doc.NewFromFiles source // Collect .gno files in a map for ast.NewPackage. diff --git a/gnovm/pkg/doc/print.go b/gnovm/pkg/doc/print.go index e811d2709eb..88c3b4c393b 100644 --- a/gnovm/pkg/doc/print.go +++ b/gnovm/pkg/doc/print.go @@ -36,7 +36,7 @@ type pkgPrinter struct { constructor map[*doc.Func]bool // Constructors. fs *token.FileSet // Needed for printing. buf pkgBuffer - opt *documentOptions + opt *writeDocOptions importPath string // this is set when an error should be returned up the call chain; From 444eaf78f27c89be53dc6fe9c9c929dd16600f1b Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Tue, 18 Apr 2023 21:52:44 +0200 Subject: [PATCH 16/24] add doc test --- gnovm/cmd/gno/doc_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 gnovm/cmd/gno/doc_test.go diff --git a/gnovm/cmd/gno/doc_test.go b/gnovm/cmd/gno/doc_test.go new file mode 100644 index 00000000000..3eb90e2a329 --- /dev/null +++ b/gnovm/cmd/gno/doc_test.go @@ -0,0 +1,29 @@ +package main + +import "testing" + +func TestGnoDoc(t *testing.T) { + tc := []testMainCase{ + { + args: []string{"doc", "io.Writer"}, + stdoutShouldContain: "Writer is the interface that wraps", + }, + { + args: []string{"doc", "avl"}, + stdoutShouldContain: "func NewNode", + }, + { + args: []string{"doc", "-u", "avl.Node"}, + stdoutShouldContain: "node *Node", + }, + { + args: []string{"doc", "dkfdkfkdfjkdfj"}, + errShouldContain: "package not found", + }, + { + args: []string{"doc", "There.Are.Too.Many.Dots"}, + errShouldContain: "invalid arguments", + }, + } + testMainCaseRun(t, tc) +} From 77cc14962cde66766288c513be818120d42b4815 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Wed, 19 Apr 2023 15:48:00 +0200 Subject: [PATCH 17/24] address changes requested from code review --- gnovm/cmd/gno/doc.go | 17 ++++++++++-- gnovm/pkg/doc/dirs.go | 6 +++- gnovm/pkg/doc/doc.go | 58 ++++++++++++++------------------------- gnovm/pkg/doc/doc_test.go | 10 +++---- gnovm/pkg/doc/pkg.go | 4 +-- gnovm/pkg/doc/print.go | 24 ++++++++-------- 6 files changed, 59 insertions(+), 60 deletions(-) diff --git a/gnovm/cmd/gno/doc.go b/gnovm/cmd/gno/doc.go index ca24b118aec..d29bcbf4dc6 100644 --- a/gnovm/cmd/gno/doc.go +++ b/gnovm/cmd/gno/doc.go @@ -13,6 +13,7 @@ type docCfg struct { all bool src bool unexported bool + short bool rootDir string } @@ -53,6 +54,13 @@ func (c *docCfg) RegisterFlags(fs *flag.FlagSet) { "show unexported symbols as well as exported", ) + fs.BoolVar( + &c.short, + "short", + false, + "show a one line representation for each symbol", + ) + fs.StringVar( &c.rootDir, "root-dir", @@ -76,9 +84,12 @@ func execDoc(cfg *docCfg, args []string, io *commands.IO) error { } err = res.WriteDocumentation( io.Out, - doc.WithShowAll(cfg.all), - doc.WithSource(cfg.src), - doc.WithUnexported(cfg.unexported), + &doc.WriteDocumentationOptions{ + ShowAll: cfg.all, + Source: cfg.src, + Unexported: cfg.unexported, + Short: false, + }, ) if err != nil { return err diff --git a/gnovm/pkg/doc/dirs.go b/gnovm/pkg/doc/dirs.go index fa53a9fe2f3..6294ff86841 100644 --- a/gnovm/pkg/doc/dirs.go +++ b/gnovm/pkg/doc/dirs.go @@ -1,4 +1,5 @@ -// Mostly copied from go source at tip, commit d922c0a. +// Mostly copied from go source at tip, cmd/doc/dirs.go. +// https://github.com/golang/go/blob/d922c0a8f5035b0533eb6e912ffd7b85487e3942/src/cmd/doc/dirs.go // // Copyright 2015 The Go Authors. All rights reserved. @@ -32,6 +33,8 @@ type Dirs struct { // NewDirs begins scanning the given stdlibs directory. func NewDirs(dirs ...string) *Dirs { d := &Dirs{ + // prealloc to 256 -- this will generaly be a few hundred dirs + // so starting off like this spares us log2(128) extra allocations in the general case. hist: make([]Dir, 0, 256), scan: make(chan Dir), } @@ -131,6 +134,7 @@ func (d *Dirs) bfsWalkRoot(root string) { // exactly, it will be returned as first. func (d *Dirs) findPackage(name string) []Dir { d.Reset() + // in general this will be either 0, 1 or a small number. candidates := make([]Dir, 0, 4) for dir, ok := d.Next(); ok; dir, ok = d.Next() { // want either exact matches or suffixes diff --git a/gnovm/pkg/doc/doc.go b/gnovm/pkg/doc/doc.go index cc6426ccdf1..d6698d1948d 100644 --- a/gnovm/pkg/doc/doc.go +++ b/gnovm/pkg/doc/doc.go @@ -18,40 +18,24 @@ import ( "go.uber.org/multierr" ) -// WriteDocumentationOption is used to pass options to the [Documentable].WriteDocumentation. -type WriteDocumentationOption func(s *writeDocOptions) - -type writeDocOptions struct { - all bool - src bool - unexported bool - short bool - w io.Writer -} - -// WithShowAll shows all symbols when displaying documentation about a package. -func WithShowAll(b bool) WriteDocumentationOption { - return func(s *writeDocOptions) { s.all = b } -} - -// WithSource shows source when documenting a symbol. -func WithSource(b bool) WriteDocumentationOption { - return func(s *writeDocOptions) { s.src = b } -} - -// WithUnexported shows unexported symbols as well as exported. -func WithUnexported(b bool) WriteDocumentationOption { - return func(s *writeDocOptions) { s.unexported = b } -} - -// WithShort shows a one-line representation for each symbol. -func WithShort(b bool) WriteDocumentationOption { - return func(s *writeDocOptions) { s.short = b } +// WriteDocumentationOptions represents the possible options when requesting +// documentation through Documentable. +type WriteDocumentationOptions struct { + // ShowAll shows all symbols when displaying documentation about a package. + ShowAll bool + // Source shows the source code when documenting a symbol. + Source bool + // Unexported shows unexported symbols as well as exported. + Unexported bool + // Short shows a one-line representation for each symbol. + Short bool + + w io.Writer } // Documentable is a package, symbol, or accessible which can be documented. type Documentable interface { - WriteDocumentation(w io.Writer, opts ...WriteDocumentationOption) error + WriteDocumentation(w io.Writer, opts *WriteDocumentationOptions) error } // static implementation check @@ -64,17 +48,17 @@ type documentable struct { pkgData *pkgData } -func (d *documentable) WriteDocumentation(w io.Writer, opts ...WriteDocumentationOption) error { - o := &writeDocOptions{w: w} - for _, opt := range opts { - opt(o) +func (d *documentable) WriteDocumentation(w io.Writer, o *WriteDocumentationOptions) error { + if o == nil { + o = &WriteDocumentationOptions{} } + o.w = w var err error // pkgData may already be initialised if we already had to look to see // if it had the symbol we wanted; otherwise initialise it now. if d.pkgData == nil { - d.pkgData, err = newPkgData(d.Dir, o.unexported) + d.pkgData, err = newPkgData(d.Dir, o.Unexported) if err != nil { return err } @@ -92,7 +76,7 @@ func (d *documentable) WriteDocumentation(w io.Writer, opts ...WriteDocumentatio pkg.Consts = append(pkg.Consts, typ.Consts...) pkg.Vars = append(pkg.Vars, typ.Vars...) pkg.Funcs = append(pkg.Funcs, typ.Funcs...) - if o.unexported || token.IsExported(typ.Name) { + if o.Unexported || token.IsExported(typ.Name) { for _, value := range typ.Consts { typedValue[value] = true } @@ -133,7 +117,7 @@ func (d *documentable) output(pp *pkgPrinter) (err error) { switch { case d.symbol == "" && d.accessible == "": - if pp.opt.all { + if pp.opt.ShowAll { pp.allDoc() return } diff --git a/gnovm/pkg/doc/doc_test.go b/gnovm/pkg/doc/doc_test.go index 7b182985e2a..13042264124 100644 --- a/gnovm/pkg/doc/doc_test.go +++ b/gnovm/pkg/doc/doc_test.go @@ -144,7 +144,7 @@ func TestDocument(t *testing.T) { tt := []struct { name string d *documentable - opts []WriteDocumentationOption + opts *WriteDocumentationOptions contains []string }{ {"base", &documentable{Dir: dir}, nil, []string{"func Crypto", "!Crypto symbol", "func NewRand", "!unexp", "type Flag", "!Name"}}, @@ -155,19 +155,19 @@ func TestDocument(t *testing.T) { { "tpUnexp", &documentable{Dir: dir, symbol: "Rand"}, - []WriteDocumentationOption{WithUnexported(true)}, + &WriteDocumentationOptions{Unexported: true}, []string{"type Rand", "comment1", "!func Crypto", "unexp ", "comment4", "!Has unexported"}, }, { "symUnexp", &documentable{Dir: dir, symbol: "unexp"}, - []WriteDocumentationOption{WithUnexported(true)}, + &WriteDocumentationOptions{Unexported: true}, []string{"var unexp", "!type Rand", "!comment1", "!comment4", "!func Crypto", "!Has unexported"}, }, { "fieldUnexp", &documentable{Dir: dir, symbol: "Rand", accessible: "unexp"}, - []WriteDocumentationOption{WithUnexported(true)}, + &WriteDocumentationOptions{Unexported: true}, []string{"type Rand", "!comment1", "comment4", "!func Crypto", "elided", "!Has unexported"}, }, } @@ -177,7 +177,7 @@ func TestDocument(t *testing.T) { tc := tc t.Run(tc.name, func(t *testing.T) { buf.Reset() - err := tc.d.WriteDocumentation(buf, tc.opts...) + err := tc.d.WriteDocumentation(buf, tc.opts) require.NoError(t, err) s := buf.String() for _, c := range tc.contains { diff --git a/gnovm/pkg/doc/pkg.go b/gnovm/pkg/doc/pkg.go index 6f115057d71..814d3540eff 100644 --- a/gnovm/pkg/doc/pkg.go +++ b/gnovm/pkg/doc/pkg.go @@ -183,7 +183,7 @@ func typeExprString(expr ast.Expr) string { return "" } -func (pkg *pkgData) docPackage(opts *writeDocOptions) (*ast.Package, *doc.Package, error) { +func (pkg *pkgData) docPackage(opts *WriteDocumentationOptions) (*ast.Package, *doc.Package, error) { // largely taken from go/doc.NewFromFiles source // Collect .gno files in a map for ast.NewPackage. @@ -204,7 +204,7 @@ func (pkg *pkgData) docPackage(opts *writeDocOptions) (*ast.Package, *doc.Packag // go doc time.Sunday // from finding the symbol. This is why we always have AllDecls. mode := doc.AllDecls - if opts.src { + if opts.Source { mode |= doc.PreserveAST } diff --git a/gnovm/pkg/doc/print.go b/gnovm/pkg/doc/print.go index 88c3b4c393b..4726c2251d5 100644 --- a/gnovm/pkg/doc/print.go +++ b/gnovm/pkg/doc/print.go @@ -36,7 +36,7 @@ type pkgPrinter struct { constructor map[*doc.Func]bool // Constructors. fs *token.FileSet // Needed for printing. buf pkgBuffer - opt *writeDocOptions + opt *WriteDocumentationOptions importPath string // this is set when an error should be returned up the call chain; @@ -46,7 +46,7 @@ type pkgPrinter struct { func (pkg *pkgPrinter) isExported(name string) bool { // cmd/doc uses a global here, so we change this to be a method. - return pkg.opt.unexported || token.IsExported(name) + return pkg.opt.Unexported || token.IsExported(name) } func (pkg *pkgPrinter) ToText(w io.Writer, text, prefix, codePrefix string) { @@ -106,13 +106,13 @@ func (pkg *pkgPrinter) newlines(n int) { } } -// emit prints the node. If pkg.opt.src is true, it ignores the provided comment, +// emit prints the node. If pkg.opt.Source is true, it ignores the provided comment, // assuming the comment is in the node itself. Otherwise, the go/doc package // clears the stuff we don't want to print anyway. It's a bit of a magic trick. func (pkg *pkgPrinter) emit(comment string, node ast.Node) { if node != nil { var arg any = node - if pkg.opt.src { + if pkg.opt.Source { // Need an extra little dance to get internal comments to appear. arg = &printer.CommentedNode{ Node: node, @@ -123,7 +123,7 @@ func (pkg *pkgPrinter) emit(comment string, node ast.Node) { if err != nil { log.Fatal(err) } - if comment != "" && !pkg.opt.src { + if comment != "" && !pkg.opt.Source { pkg.newlines(1) pkg.ToText(&pkg.buf, comment, indent, indent+indent) pkg.newlines(2) // Blank line after comment to separate from next item. @@ -396,12 +396,12 @@ func (pkg *pkgPrinter) allDoc() { // packageDoc prints the docs for the package (package doc plus one-liners of the rest). func (pkg *pkgPrinter) packageDoc() { pkg.Printf("") // Trigger the package clause; we know the package exists. - if !pkg.opt.short { + if !pkg.opt.Short { pkg.ToText(&pkg.buf, pkg.doc.Doc, "", indent) pkg.newlines(1) } - if !pkg.opt.short { + if !pkg.opt.Short { pkg.newlines(2) // Guarantee blank line before the components. } @@ -409,14 +409,14 @@ func (pkg *pkgPrinter) packageDoc() { pkg.valueSummary(pkg.doc.Vars, false) pkg.funcSummary(pkg.doc.Funcs, false) pkg.typeSummary() - if !pkg.opt.short { + if !pkg.opt.Short { pkg.bugs() } } // packageClause prints the package clause. func (pkg *pkgPrinter) packageClause() { - if pkg.opt.short { + if pkg.opt.Short { return } @@ -638,7 +638,7 @@ func (pkg *pkgPrinter) valueDoc(value *doc.Value, printed map[*ast.GenDecl]bool) } for _, ident := range vspec.Names { - if pkg.opt.src || pkg.isExported(ident.Name) { + if pkg.opt.Source || pkg.isExported(ident.Name) { if vspec.Type == nil && vspec.Values == nil && typ != nil { // This a standalone identifier, as in the case of iota usage. // Thus, assume the type comes from the previous type. @@ -675,7 +675,7 @@ func (pkg *pkgPrinter) typeDoc(typ *doc.Type) { pkg.emit(typ.Doc, decl) pkg.newlines(2) // Show associated methods, constants, etc. - if pkg.opt.all { + if pkg.opt.ShowAll { printed := make(map[*ast.GenDecl]bool) // We can use append here to print consts, then vars. Ditto for funcs and methods. values := typ.Consts @@ -710,7 +710,7 @@ func (pkg *pkgPrinter) typeDoc(typ *doc.Type) { // structs and methods from interfaces (unless the unexported flag is set or we // are asked to show the original source). func (pkg *pkgPrinter) trimUnexportedElems(spec *ast.TypeSpec) { - if pkg.opt.unexported || pkg.opt.src { + if pkg.opt.Unexported || pkg.opt.Source { return } switch typ := spec.Type.(type) { From 23eb5caca1bef20b48871545c8ae0a0bba9501c0 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Wed, 19 Apr 2023 15:53:17 +0200 Subject: [PATCH 18/24] fix typo --- gnovm/pkg/doc/dirs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnovm/pkg/doc/dirs.go b/gnovm/pkg/doc/dirs.go index 6294ff86841..af649bfc130 100644 --- a/gnovm/pkg/doc/dirs.go +++ b/gnovm/pkg/doc/dirs.go @@ -33,7 +33,7 @@ type Dirs struct { // NewDirs begins scanning the given stdlibs directory. func NewDirs(dirs ...string) *Dirs { d := &Dirs{ - // prealloc to 256 -- this will generaly be a few hundred dirs + // prealloc to 256 -- this will generally be a few hundred dirs // so starting off like this spares us log2(128) extra allocations in the general case. hist: make([]Dir, 0, 256), scan: make(chan Dir), From ed6b21ec382c4d5fbc5268794d34fc4ff8e1580d Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Tue, 9 May 2023 19:21:42 +0200 Subject: [PATCH 19/24] code review changes --- gnovm/cmd/gno/doc.go | 6 +----- gnovm/pkg/doc/doc.go | 37 ++++++++++++++++++++----------------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/gnovm/cmd/gno/doc.go b/gnovm/cmd/gno/doc.go index d29bcbf4dc6..39b87bf701f 100644 --- a/gnovm/cmd/gno/doc.go +++ b/gnovm/cmd/gno/doc.go @@ -82,7 +82,7 @@ func execDoc(cfg *docCfg, args []string, io *commands.IO) error { if err != nil { io.Printfln("warning: error parsing some candidate packages:\n%v", err) } - err = res.WriteDocumentation( + return res.WriteDocumentation( io.Out, &doc.WriteDocumentationOptions{ ShowAll: cfg.all, @@ -91,8 +91,4 @@ func execDoc(cfg *docCfg, args []string, io *commands.IO) error { Short: false, }, ) - if err != nil { - return err - } - return nil } diff --git a/gnovm/pkg/doc/doc.go b/gnovm/pkg/doc/doc.go index d6698d1948d..a7a36115532 100644 --- a/gnovm/pkg/doc/doc.go +++ b/gnovm/pkg/doc/doc.go @@ -76,18 +76,19 @@ func (d *documentable) WriteDocumentation(w io.Writer, o *WriteDocumentationOpti pkg.Consts = append(pkg.Consts, typ.Consts...) pkg.Vars = append(pkg.Vars, typ.Vars...) pkg.Funcs = append(pkg.Funcs, typ.Funcs...) - if o.Unexported || token.IsExported(typ.Name) { - for _, value := range typ.Consts { - typedValue[value] = true - } - for _, value := range typ.Vars { - typedValue[value] = true - } - for _, fun := range typ.Funcs { - // We don't count it as a constructor bound to the type - // if the type itself is not exported. - constructor[fun] = true - } + if !o.Unexported && !token.IsExported(typ.Name) { + continue + } + for _, value := range typ.Consts { + typedValue[value] = true + } + for _, value := range typ.Vars { + typedValue[value] = true + } + for _, fun := range typ.Funcs { + // We don't count it as a constructor bound to the type + // if the type itself is not exported. + constructor[fun] = true } } @@ -142,6 +143,7 @@ var fpAbs = filepath.Abs // ResolveDocumentable returns a Documentable from the given arguments. // Refer to the documentation of gnodev doc for the formats accepted (in general // the same as the go doc command). +// An warning error may be returned even if documentation was resolved. func ResolveDocumentable(dirs *Dirs, args []string, unexported bool) (Documentable, error) { parsed, ok := parseArgs(args) if !ok { @@ -212,12 +214,13 @@ func resolveDocumentable(dirs *Dirs, parsed docArgs, unexported bool) (Documenta continue } for _, sym := range pd.symbols { - if matchFunc(sym) { - doc.Dir = candidate - doc.pkgData = pd - // match found. return this as documentable. - return doc, multierr.Combine(errs...) + if !matchFunc(sym) { + continue } + doc.Dir = candidate + doc.pkgData = pd + // match found. return this as documentable. + return doc, multierr.Combine(errs...) } } return nil, multierr.Append( From e0dc07cf5e55f0a3b7eb549ae3533b82c7893496 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Tue, 9 May 2023 20:25:08 +0200 Subject: [PATCH 20/24] unexport dirs --- gnovm/cmd/gno/doc.go | 2 +- gnovm/pkg/doc/dirs.go | 52 ++++++++++++++++++-------------------- gnovm/pkg/doc/dirs_test.go | 28 ++++++++++---------- gnovm/pkg/doc/doc.go | 20 ++++++++------- gnovm/pkg/doc/doc_test.go | 50 ++++++++++++++++++------------------ gnovm/pkg/doc/pkg.go | 4 +-- 6 files changed, 77 insertions(+), 79 deletions(-) diff --git a/gnovm/cmd/gno/doc.go b/gnovm/cmd/gno/doc.go index 39b87bf701f..c54e289cd67 100644 --- a/gnovm/cmd/gno/doc.go +++ b/gnovm/cmd/gno/doc.go @@ -74,7 +74,7 @@ func execDoc(cfg *docCfg, args []string, io *commands.IO) error { if cfg.rootDir == "" { cfg.rootDir = guessRootDir() } - dirs := doc.NewDirs(filepath.Join(cfg.rootDir, "gnovm/stdlibs"), filepath.Join(cfg.rootDir, "examples")) + dirs := []string{filepath.Join(cfg.rootDir, "gnovm/stdlibs"), filepath.Join(cfg.rootDir, "examples")} res, err := doc.ResolveDocumentable(dirs, args, cfg.unexported) if res == nil { return err diff --git a/gnovm/pkg/doc/dirs.go b/gnovm/pkg/doc/dirs.go index af649bfc130..c6f90d167e4 100644 --- a/gnovm/pkg/doc/dirs.go +++ b/gnovm/pkg/doc/dirs.go @@ -1,5 +1,4 @@ -// Mostly copied from go source at tip, cmd/doc/dirs.go. -// https://github.com/golang/go/blob/d922c0a8f5035b0533eb6e912ffd7b85487e3942/src/cmd/doc/dirs.go +// Mostly copied from go source at tip, commit d922c0a. // // Copyright 2015 The Go Authors. All rights reserved. @@ -13,43 +12,41 @@ import ( "strings" ) -// A Dir describes a directory holding code by specifying +// A bfsDir describes a directory holding code by specifying // the expected import path and the file system directory. -type Dir struct { +type bfsDir struct { importPath string // import path for that dir dir string // file system directory } -// Dirs is a structure for scanning the directory tree. +// dirs is a structure for scanning the directory tree. // Its Next method returns the next Go source directory it finds. // Although it can be used to scan the tree multiple times, it // only walks the tree once, caching the data it finds. -type Dirs struct { - scan chan Dir // Directories generated by walk. - hist []Dir // History of reported Dirs. - offset int // Counter for Next. +type bfsDirs struct { + scan chan bfsDir // Directories generated by walk. + hist []bfsDir // History of reported Dirs. + offset int // Counter for Next. } -// NewDirs begins scanning the given stdlibs directory. -func NewDirs(dirs ...string) *Dirs { - d := &Dirs{ - // prealloc to 256 -- this will generally be a few hundred dirs - // so starting off like this spares us log2(128) extra allocations in the general case. - hist: make([]Dir, 0, 256), - scan: make(chan Dir), +// newDirs begins scanning the given stdlibs directory. +func newDirs(dirs ...string) *bfsDirs { + d := &bfsDirs{ + hist: make([]bfsDir, 0, 256), + scan: make(chan bfsDir), } go d.walk(dirs) return d } // Reset puts the scan back at the beginning. -func (d *Dirs) Reset() { +func (d *bfsDirs) Reset() { d.offset = 0 } // Next returns the next directory in the scan. The boolean // is false when the scan is done. -func (d *Dirs) Next() (Dir, bool) { +func (d *bfsDirs) Next() (bfsDir, bool) { if d.offset < len(d.hist) { dir := d.hist[d.offset] d.offset++ @@ -57,7 +54,7 @@ func (d *Dirs) Next() (Dir, bool) { } dir, ok := <-d.scan if !ok { - return Dir{}, false + return bfsDir{}, false } d.hist = append(d.hist, dir) d.offset++ @@ -65,7 +62,7 @@ func (d *Dirs) Next() (Dir, bool) { } // walk walks the trees in the given roots. -func (d *Dirs) walk(roots []string) { +func (d *bfsDirs) walk(roots []string) { for _, root := range roots { d.bfsWalkRoot(root) } @@ -73,8 +70,8 @@ func (d *Dirs) walk(roots []string) { } // bfsWalkRoot walks a single directory hierarchy in breadth-first lexical order. -// Each Gno source directory it finds is delivered on d.scan. -func (d *Dirs) bfsWalkRoot(root string) { +// Each Go source directory it finds is delivered on d.scan. +func (d *bfsDirs) bfsWalkRoot(root string) { root = filepath.Clean(root) // this is the queue of directories to examine in this pass. @@ -122,7 +119,7 @@ func (d *Dirs) bfsWalkRoot(root string) { if len(dir) > len(root) { importPath = filepath.ToSlash(dir[len(root)+1:]) } - d.scan <- Dir{importPath, dir} + d.scan <- bfsDir{importPath, dir} } } } @@ -132,10 +129,9 @@ func (d *Dirs) bfsWalkRoot(root string) { // name as a suffix (which may be a package name or a fully-qualified path). // returns a list of possible directories. If a directory's import path matched // exactly, it will be returned as first. -func (d *Dirs) findPackage(name string) []Dir { +func (d *bfsDirs) findPackage(name string) []bfsDir { d.Reset() - // in general this will be either 0, 1 or a small number. - candidates := make([]Dir, 0, 4) + candidates := make([]bfsDir, 0, 4) for dir, ok := d.Next(); ok; dir, ok = d.Next() { // want either exact matches or suffixes if dir.importPath == name || strings.HasSuffix(dir.importPath, "/"+name) { @@ -156,11 +152,11 @@ func (d *Dirs) findPackage(name string) []Dir { // findDir determines if the given absdir is present in the Dirs. // If not, the nil slice is returned. It returns always at most one dir. -func (d *Dirs) findDir(absdir string) []Dir { +func (d *bfsDirs) findDir(absdir string) []bfsDir { d.Reset() for dir, ok := d.Next(); ok; dir, ok = d.Next() { if dir.dir == absdir { - return []Dir{dir} + return []bfsDir{dir} } } return nil diff --git a/gnovm/pkg/doc/dirs_test.go b/gnovm/pkg/doc/dirs_test.go index 99d3425b076..a7c4926a8c8 100644 --- a/gnovm/pkg/doc/dirs_test.go +++ b/gnovm/pkg/doc/dirs_test.go @@ -9,33 +9,33 @@ import ( "github.com/stretchr/testify/require" ) -func newDirs(t *testing.T) (string, *Dirs) { +func tNewDirs(t *testing.T) (string, *bfsDirs) { t.Helper() p, err := filepath.Abs("./testdata/dirs") require.NoError(t, err) - return p, NewDirs(p) + return p, newDirs(p) } func TestDirs_findPackage(t *testing.T) { - abs, d := newDirs(t) + abs, d := tNewDirs(t) tt := []struct { name string - res []Dir + res []bfsDir }{ - {"rand", []Dir{ + {"rand", []bfsDir{ {importPath: "rand", dir: filepath.Join(abs, "rand")}, {importPath: "crypto/rand", dir: filepath.Join(abs, "crypto/rand")}, {importPath: "math/rand", dir: filepath.Join(abs, "math/rand")}, }}, - {"crypto/rand", []Dir{ + {"crypto/rand", []bfsDir{ {importPath: "crypto/rand", dir: filepath.Join(abs, "crypto/rand")}, }}, - {"math", []Dir{ + {"math", []bfsDir{ {importPath: "math", dir: filepath.Join(abs, "math")}, }}, - {"ath", []Dir{}}, - {"/math", []Dir{}}, - {"", []Dir{}}, + {"ath", []bfsDir{}}, + {"/math", []bfsDir{}}, + {"", []bfsDir{}}, } for _, tc := range tt { tc := tc @@ -47,16 +47,16 @@ func TestDirs_findPackage(t *testing.T) { } func TestDirs_findDir(t *testing.T) { - abs, d := newDirs(t) + abs, d := tNewDirs(t) tt := []struct { name string in string - res []Dir + res []bfsDir }{ - {"rand", filepath.Join(abs, "rand"), []Dir{ + {"rand", filepath.Join(abs, "rand"), []bfsDir{ {importPath: "rand", dir: filepath.Join(abs, "rand")}, }}, - {"crypto/rand", filepath.Join(abs, "crypto/rand"), []Dir{ + {"crypto/rand", filepath.Join(abs, "crypto/rand"), []bfsDir{ {importPath: "crypto/rand", dir: filepath.Join(abs, "crypto/rand")}, }}, // ignored (dir name testdata), so should not return anything. diff --git a/gnovm/pkg/doc/doc.go b/gnovm/pkg/doc/doc.go index a7a36115532..76950dc30e1 100644 --- a/gnovm/pkg/doc/doc.go +++ b/gnovm/pkg/doc/doc.go @@ -42,7 +42,7 @@ type Documentable interface { var _ Documentable = (*documentable)(nil) type documentable struct { - Dir + bfsDir symbol string accessible string pkgData *pkgData @@ -58,7 +58,7 @@ func (d *documentable) WriteDocumentation(w io.Writer, o *WriteDocumentationOpti // pkgData may already be initialised if we already had to look to see // if it had the symbol we wanted; otherwise initialise it now. if d.pkgData == nil { - d.pkgData, err = newPkgData(d.Dir, o.Unexported) + d.pkgData, err = newPkgData(d.bfsDir, o.Unexported) if err != nil { return err } @@ -144,16 +144,18 @@ var fpAbs = filepath.Abs // Refer to the documentation of gnodev doc for the formats accepted (in general // the same as the go doc command). // An warning error may be returned even if documentation was resolved. -func ResolveDocumentable(dirs *Dirs, args []string, unexported bool) (Documentable, error) { +func ResolveDocumentable(dirs []string, args []string, unexported bool) (Documentable, error) { + d := newDirs(dirs...) + parsed, ok := parseArgs(args) if !ok { return nil, fmt.Errorf("commands/doc: invalid arguments: %v", args) } - return resolveDocumentable(dirs, parsed, unexported) + return resolveDocumentable(d, parsed, unexported) } -func resolveDocumentable(dirs *Dirs, parsed docArgs, unexported bool) (Documentable, error) { - var candidates []Dir +func resolveDocumentable(dirs *bfsDirs, parsed docArgs, unexported bool) (Documentable, error) { + var candidates []bfsDir // if we have a candidate package name, search dirs for a dir that matches it. // prefer directories whose import path match precisely the package @@ -179,9 +181,9 @@ func resolveDocumentable(dirs *Dirs, parsed docArgs, unexported bool) (Documenta parsed = docArgs{pkg: ".", sym: parsed.pkg, acc: parsed.sym} return resolveDocumentable(dirs, parsed, unexported) } - // we wanted documentation about a package, and we found one! + // we wanted documentabfsDirn about a package, and we found one! if parsed.sym == "" { - return &documentable{Dir: candidates[0]}, nil + return &documentable{bfsDir: candidates[0]}, nil } // we also have a symbol, and maybe accessible. @@ -217,7 +219,7 @@ func resolveDocumentable(dirs *Dirs, parsed docArgs, unexported bool) (Documenta if !matchFunc(sym) { continue } - doc.Dir = candidate + doc.bfsDir = candidate doc.pkgData = pd // match found. return this as documentable. return doc, multierr.Combine(errs...) diff --git a/gnovm/pkg/doc/doc_test.go b/gnovm/pkg/doc/doc_test.go index 13042264124..1cccb4106f7 100644 --- a/gnovm/pkg/doc/doc_test.go +++ b/gnovm/pkg/doc/doc_test.go @@ -15,8 +15,8 @@ func TestResolveDocumentable(t *testing.T) { p, err := os.Getwd() require.NoError(t, err) path := func(s string) string { return filepath.Join(p, "testdata/integ", s) } - dirs := NewDirs(path("")) - getDir := func(p string) Dir { return dirs.findDir(path(p))[0] } + dirs := newDirs(path("")) + getDir := func(p string) bfsDir { return dirs.findDir(path(p))[0] } pdata := func(p string, unexp bool) *pkgData { pd, err := newPkgData(getDir(p), unexp) require.NoError(t, err) @@ -30,65 +30,65 @@ func TestResolveDocumentable(t *testing.T) { expect Documentable errContains string }{ - {"package", []string{"crypto/rand"}, false, &documentable{Dir: getDir("crypto/rand")}, ""}, - {"dir", []string{"./testdata/integ/crypto/rand"}, false, &documentable{Dir: getDir("crypto/rand")}, ""}, - {"dirAbs", []string{path("crypto/rand")}, false, &documentable{Dir: getDir("crypto/rand")}, ""}, + {"package", []string{"crypto/rand"}, false, &documentable{bfsDir: getDir("crypto/rand")}, ""}, + {"dir", []string{"./testdata/integ/crypto/rand"}, false, &documentable{bfsDir: getDir("crypto/rand")}, ""}, + {"dirAbs", []string{path("crypto/rand")}, false, &documentable{bfsDir: getDir("crypto/rand")}, ""}, // test_notapkg exists in local dir and also path("test_notapkg"). // ResolveDocumentable should first try local dir, and seeing as it is not a valid dir, try searching it as a package. - {"dirLocalMisleading", []string{"test_notapkg"}, false, &documentable{Dir: getDir("test_notapkg")}, ""}, + {"dirLocalMisleading", []string{"test_notapkg"}, false, &documentable{bfsDir: getDir("test_notapkg")}, ""}, { "normalSymbol", []string{"crypto/rand.Flag"}, false, - &documentable{Dir: getDir("crypto/rand"), symbol: "Flag", pkgData: pdata("crypto/rand", false)}, "", + &documentable{bfsDir: getDir("crypto/rand"), symbol: "Flag", pkgData: pdata("crypto/rand", false)}, "", }, { "normalAccessible", []string{"crypto/rand.Generate"}, false, - &documentable{Dir: getDir("crypto/rand"), symbol: "Generate", pkgData: pdata("crypto/rand", false)}, "", + &documentable{bfsDir: getDir("crypto/rand"), symbol: "Generate", pkgData: pdata("crypto/rand", false)}, "", }, { "normalSymbolUnexp", []string{"crypto/rand.unexp"}, true, - &documentable{Dir: getDir("crypto/rand"), symbol: "unexp", pkgData: pdata("crypto/rand", true)}, "", + &documentable{bfsDir: getDir("crypto/rand"), symbol: "unexp", pkgData: pdata("crypto/rand", true)}, "", }, { "normalAccessibleFull", []string{"crypto/rand.Rand.Name"}, false, - &documentable{Dir: getDir("crypto/rand"), symbol: "Rand", accessible: "Name", pkgData: pdata("crypto/rand", false)}, "", + &documentable{bfsDir: getDir("crypto/rand"), symbol: "Rand", accessible: "Name", pkgData: pdata("crypto/rand", false)}, "", }, { "disambiguate", []string{"rand.Flag"}, false, - &documentable{Dir: getDir("crypto/rand"), symbol: "Flag", pkgData: pdata("crypto/rand", false)}, "", + &documentable{bfsDir: getDir("crypto/rand"), symbol: "Flag", pkgData: pdata("crypto/rand", false)}, "", }, { "disambiguate2", []string{"rand.Crypto"}, false, - &documentable{Dir: getDir("crypto/rand"), symbol: "Crypto", pkgData: pdata("crypto/rand", false)}, "", + &documentable{bfsDir: getDir("crypto/rand"), symbol: "Crypto", pkgData: pdata("crypto/rand", false)}, "", }, { "disambiguate3", []string{"rand.Normal"}, false, - &documentable{Dir: getDir("rand"), symbol: "Normal", pkgData: pdata("rand", false)}, "", + &documentable{bfsDir: getDir("rand"), symbol: "Normal", pkgData: pdata("rand", false)}, "", }, { "disambiguate4", // just "rand" should use the directory that matches it exactly. []string{"rand"}, false, - &documentable{Dir: getDir("rand")}, "", + &documentable{bfsDir: getDir("rand")}, "", }, { "wdSymbol", []string{"WdConst"}, false, - &documentable{Dir: getDir("wd"), symbol: "WdConst", pkgData: pdata("wd", false)}, "", + &documentable{bfsDir: getDir("wd"), symbol: "WdConst", pkgData: pdata("wd", false)}, "", }, {"errInvalidArgs", []string{"1", "2", "3"}, false, nil, "invalid arguments: [1 2 3]"}, @@ -110,7 +110,7 @@ func TestResolveDocumentable(t *testing.T) { fpAbs = func(s string) (string, error) { return filepath.Clean(filepath.Join(path("wd"), s)), nil } defer func() { fpAbs = filepath.Abs }() } - result, err := ResolveDocumentable(dirs, tc.args, tc.unexp) + result, err := ResolveDocumentable([]string{path("")}, tc.args, tc.unexp) // we use stripFset because d.pkgData.fset contains sync/atomic values, // which in turn makes reflect.DeepEqual compare the two sync.Atomic values. assert.Equal(t, stripFset(tc.expect), stripFset(result), "documentables should match") @@ -136,7 +136,7 @@ func TestDocument(t *testing.T) { // Document. abspath, err := filepath.Abs("./testdata/integ/crypto/rand") require.NoError(t, err) - dir := Dir{ + dir := bfsDir{ importPath: "crypto/rand", dir: abspath, } @@ -147,26 +147,26 @@ func TestDocument(t *testing.T) { opts *WriteDocumentationOptions contains []string }{ - {"base", &documentable{Dir: dir}, nil, []string{"func Crypto", "!Crypto symbol", "func NewRand", "!unexp", "type Flag", "!Name"}}, - {"func", &documentable{Dir: dir, symbol: "crypto"}, nil, []string{"Crypto symbol", "func Crypto", "!func NewRand", "!type Flag"}}, - {"funcWriter", &documentable{Dir: dir, symbol: "NewWriter"}, nil, []string{"func NewWriter() io.Writer", "!func Crypto"}}, - {"tp", &documentable{Dir: dir, symbol: "Rand"}, nil, []string{"type Rand", "comment1", "!func Crypto", "!unexp ", "!comment4", "Has unexported"}}, - {"tpField", &documentable{Dir: dir, symbol: "Rand", accessible: "Value"}, nil, []string{"type Rand", "!comment1", "comment2", "!func Crypto", "!unexp", "elided"}}, + {"base", &documentable{bfsDir: dir}, nil, []string{"func Crypto", "!Crypto symbol", "func NewRand", "!unexp", "type Flag", "!Name"}}, + {"func", &documentable{bfsDir: dir, symbol: "crypto"}, nil, []string{"Crypto symbol", "func Crypto", "!func NewRand", "!type Flag"}}, + {"funcWriter", &documentable{bfsDir: dir, symbol: "NewWriter"}, nil, []string{"func NewWriter() io.Writer", "!func Crypto"}}, + {"tp", &documentable{bfsDir: dir, symbol: "Rand"}, nil, []string{"type Rand", "comment1", "!func Crypto", "!unexp ", "!comment4", "Has unexported"}}, + {"tpField", &documentable{bfsDir: dir, symbol: "Rand", accessible: "Value"}, nil, []string{"type Rand", "!comment1", "comment2", "!func Crypto", "!unexp", "elided"}}, { "tpUnexp", - &documentable{Dir: dir, symbol: "Rand"}, + &documentable{bfsDir: dir, symbol: "Rand"}, &WriteDocumentationOptions{Unexported: true}, []string{"type Rand", "comment1", "!func Crypto", "unexp ", "comment4", "!Has unexported"}, }, { "symUnexp", - &documentable{Dir: dir, symbol: "unexp"}, + &documentable{bfsDir: dir, symbol: "unexp"}, &WriteDocumentationOptions{Unexported: true}, []string{"var unexp", "!type Rand", "!comment1", "!comment4", "!func Crypto", "!Has unexported"}, }, { "fieldUnexp", - &documentable{Dir: dir, symbol: "Rand", accessible: "unexp"}, + &documentable{bfsDir: dir, symbol: "Rand", accessible: "unexp"}, &WriteDocumentationOptions{Unexported: true}, []string{"type Rand", "!comment1", "comment4", "!func Crypto", "elided", "!Has unexported"}, }, diff --git a/gnovm/pkg/doc/pkg.go b/gnovm/pkg/doc/pkg.go index 814d3540eff..71e1a50f299 100644 --- a/gnovm/pkg/doc/pkg.go +++ b/gnovm/pkg/doc/pkg.go @@ -13,7 +13,7 @@ import ( type pkgData struct { name string - dir Dir + dir bfsDir fset *token.FileSet files []*ast.File testFiles []*ast.File @@ -35,7 +35,7 @@ type symbolData struct { typ byte } -func newPkgData(dir Dir, unexported bool) (*pkgData, error) { +func newPkgData(dir bfsDir, unexported bool) (*pkgData, error) { files, err := os.ReadDir(dir.dir) if err != nil { return nil, fmt.Errorf("commands/doc: open %q: %w", dir.dir, err) From 5f874a4a8d9ccc0d463ef51b354826ee749077d1 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Wed, 10 May 2023 21:59:40 +0200 Subject: [PATCH 21/24] some typos, some error logging --- gnovm/pkg/doc/doc.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/gnovm/pkg/doc/doc.go b/gnovm/pkg/doc/doc.go index 76950dc30e1..46b0f13da83 100644 --- a/gnovm/pkg/doc/doc.go +++ b/gnovm/pkg/doc/doc.go @@ -11,6 +11,7 @@ import ( "go/doc" "go/token" "io" + "log" "os" "path/filepath" "strings" @@ -143,7 +144,8 @@ var fpAbs = filepath.Abs // ResolveDocumentable returns a Documentable from the given arguments. // Refer to the documentation of gnodev doc for the formats accepted (in general // the same as the go doc command). -// An warning error may be returned even if documentation was resolved. +// An error may be returned even if documentation was resolved in case some +// packages in dirs could not be parsed correctly. func ResolveDocumentable(dirs []string, args []string, unexported bool) (Documentable, error) { d := newDirs(dirs...) @@ -160,11 +162,17 @@ func resolveDocumentable(dirs *bfsDirs, parsed docArgs, unexported bool) (Docume // if we have a candidate package name, search dirs for a dir that matches it. // prefer directories whose import path match precisely the package if s, err := os.Stat(parsed.pkg); err == nil && s.IsDir() { - // expand to full path + // expand to full path - fpAbs is filepath.Abs except in test absVal, err := fpAbs(parsed.pkg) if err == nil { candidates = dirs.findDir(absVal) + } else { + // this is very rare - generally syscall failure or os.Getwd failing + log.Printf("warning: could not determine abs path: %v", err) } + } else if err != nil && !os.IsNotExist(err) { + // also quite rare, generally will be permission errors (in reading cwd) + log.Printf("warning: tried showing documentation for directory %q, error: %v", parsed.pkg, err) } // arg is either not a dir, or if it matched a local dir it was not // valid (ie. not scanned by dirs). try parsing as a package @@ -210,8 +218,8 @@ func resolveDocumentable(dirs *bfsDirs, parsed docArgs, unexported bool) (Docume for _, candidate := range candidates { pd, err := newPkgData(candidate, unexported) if err != nil { - // silently ignore errors - - // likely invalid AST in a file. + // report errors as warning, but don't fail because of them + // likely ast/parsing errors. errs = append(errs, err) continue } @@ -259,7 +267,7 @@ func parseArgs(args []string) (docArgs, bool) { slash := strings.LastIndexByte(args[0], '/') if args[0] == "." || args[0] == ".." || (slash != -1 && args[0][slash+1:] == "..") { - // special handling for common ., .. and ./.. + // special handling for common ., .. and /.. // these will generally work poorly if you try to use the one-argument // syntax to access a symbol/accessible. return docArgs{pkg: args[0]}, true From d817e92afe3731818420bd63f295eb79d2fa65cb Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Wed, 10 May 2023 22:12:03 +0200 Subject: [PATCH 22/24] better and more consistent error handling --- gnovm/pkg/doc/doc.go | 15 +++++++++++++-- gnovm/pkg/doc/print.go | 20 +++++++++++++------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/gnovm/pkg/doc/doc.go b/gnovm/pkg/doc/doc.go index 46b0f13da83..2bc179116b4 100644 --- a/gnovm/pkg/doc/doc.go +++ b/gnovm/pkg/doc/doc.go @@ -111,10 +111,21 @@ func (d *documentable) WriteDocumentation(w io.Writer, o *WriteDocumentationOpti func (d *documentable) output(pp *pkgPrinter) (err error) { defer func() { - pp.flush() - if err == nil { + // handle the case of errFatal. + // this will have been generated by pkg.Fatalf, so get the error + // from pp.err. + e := recover() + if e != nil && e != errFatal { + panic(e) + } + + flushErr := pp.flush() + if pp.err == nil { err = pp.err } + if flushErr != nil { + err = multierr.Combine(err, fmt.Errorf("error flushing: %w", err)) + } }() switch { diff --git a/gnovm/pkg/doc/print.go b/gnovm/pkg/doc/print.go index 4726c2251d5..7ac1742c62f 100644 --- a/gnovm/pkg/doc/print.go +++ b/gnovm/pkg/doc/print.go @@ -9,6 +9,7 @@ package doc import ( "bufio" "bytes" + "errors" "fmt" "go/ast" "go/doc" @@ -39,8 +40,9 @@ type pkgPrinter struct { opt *WriteDocumentationOptions importPath string - // this is set when an error should be returned up the call chain; - // ie. it is set in the places where the cmd/go code would panic. + // this is set when an error should be returned up the call chain. + // it is set together with a panic(errFatal), so it can be checked easily + // when calling recover. err error } @@ -77,24 +79,28 @@ func (pb *pkgBuffer) packageClause() { } } +var errFatal = errors.New("pkg/doc: pkgPrinter.Fatalf called") + // in cmd/go, pkg.Fatalf is like log.Fatalf, but panics so it can be recovered in the // main do function, so it doesn't cause an exit. Allows testing to work // without running a subprocess. // For our purposes, we store the error in .err - the caller knows about this and will check it. func (pkg *pkgPrinter) Fatalf(format string, args ...any) { pkg.err = fmt.Errorf(format, args...) + panic(errFatal) } func (pkg *pkgPrinter) Printf(format string, args ...any) { fmt.Fprintf(&pkg.buf, format, args...) } -func (pkg *pkgPrinter) flush() { +func (pkg *pkgPrinter) flush() error { _, err := pkg.opt.w.Write(pkg.buf.Bytes()) if err != nil { - log.Fatal(err) + return err } pkg.buf.Reset() // Not needed, but it's a flush. + return nil } var newlineBytes = []byte("\n\n") // We never ask for more than 2. @@ -121,7 +127,7 @@ func (pkg *pkgPrinter) emit(comment string, node ast.Node) { } err := format.Node(&pkg.buf, pkg.fs, arg) if err != nil { - log.Fatal(err) + pkg.Fatalf("%v", err) } if comment != "" && !pkg.opt.Source { pkg.newlines(1) @@ -764,7 +770,7 @@ func (pkg *pkgPrinter) trimUnexportedFields(fields *ast.FieldList, isInterface b } if names == nil && !constraint { // Can only happen if AST is incorrect. Safe to continue with a nil list. - log.Print("invalid program: unexpected type for embedded field") + log.Print("warning: invalid program: unexpected type for embedded field") } } // Trims if any is unexported. Good enough in practice. @@ -857,7 +863,7 @@ func (pkg *pkgPrinter) printMethodDoc(symbol, method string) bool { inter.Methods.List, methods = methods, inter.Methods.List err := format.Node(&pkg.buf, pkg.fs, inter) if err != nil { - log.Fatal(err) + pkg.Fatalf("%v", err) } pkg.newlines(1) // Restore the original methods. From d481161c5f38515a6764b35bce18c347bc22c27d Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Thu, 11 May 2023 15:48:11 +0200 Subject: [PATCH 23/24] use errors.Is for comparison --- gnovm/pkg/doc/doc.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gnovm/pkg/doc/doc.go b/gnovm/pkg/doc/doc.go index 2bc179116b4..cecd97f53d9 100644 --- a/gnovm/pkg/doc/doc.go +++ b/gnovm/pkg/doc/doc.go @@ -6,6 +6,7 @@ package doc import ( + "errors" "fmt" "go/ast" "go/doc" @@ -115,7 +116,8 @@ func (d *documentable) output(pp *pkgPrinter) (err error) { // this will have been generated by pkg.Fatalf, so get the error // from pp.err. e := recover() - if e != nil && e != errFatal { + ee, ok := e.(error) + if e != nil && ok && errors.Is(ee, errFatal) { panic(e) } From a2965cc8cd1cf95699e477e1e68d78cf8f02b93f Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Fri, 12 May 2023 09:37:57 +0200 Subject: [PATCH 24/24] empty commit to trigger github workflow