diff --git a/docs/console/utilities.md b/docs/console/utilities.md index 8123305..8fcc31c 100644 --- a/docs/console/utilities.md +++ b/docs/console/utilities.md @@ -42,6 +42,6 @@ summer docs:sync summer docs:serve ``` -`docs:build` fails, and writes nothing, when a page names an identifier that does not exist, links to a missing page or anchor, shows a command that neither `summer` nor an application binary has, or has a `src=` code block that differs from its source. It also fails on a Go code block (including `golang`) without a `src=` reference, a `src=` code block inside a callout, blockquote or list item (`src=` blocks must be top-level), and a `src=` code block in a module README. After you change code that a page shows, run `summer docs:sync` to refresh the copies. +`docs:build` fails, and writes nothing, when a page names an identifier that does not exist, links to a missing page or anchor, shows a command that neither `summer` nor an application binary has, or has a `src=` code block that differs from its source. It also fails on a Go code block (including `golang`) without a `src=` reference, a `src=` code block inside a callout, blockquote or list item (`src=` blocks must be top-level), and a `src=` code block in a module README. A `src=` target must be code `go test ./...` compiles and runs, that is a file in the default build, inside a `Test` function or an `Example` with an `// Output:` comment, or a function one of them calls. After you change code that a page shows, run `summer docs:sync` to refresh the copies. `docs:serve` listens only on a loopback address unless you pass `--allow-remote`. Use it to preview search, which browsers block when you open the built files directly from disk. diff --git a/internal/docsite/check_commands.go b/internal/docsite/check_commands.go index 4060ab7..3b400ae 100644 --- a/internal/docsite/check_commands.go +++ b/internal/docsite/check_commands.go @@ -30,10 +30,6 @@ type Commands struct { // are checked as commands and highlighted with prompts. var shellLangs = []string{"sh", "shell", "bash", "console"} -// commandToken finds `summer ` and `./bin/ ` at the start -// of a shell command (after an optional "$ " prompt). -var commandToken = regexp.MustCompile(`^(?:\$\s+)?(summer|\./bin/[A-Za-z0-9._-]+)\s+(\S+)`) - // commandSeparators split one shell line into its commands. var commandSeparators = regexp.MustCompile(`&&|\|\||;|\|`) @@ -64,14 +60,13 @@ func (s *site) checkCommands(docs []parsedDoc) ([]Problem, error) { var problems []Problem check := func(d parsedDoc, line int, text string) { for _, cmd := range commandSeparators.Split(text, -1) { - m := commandToken.FindStringSubmatch(strings.TrimSpace(cmd)) - if m == nil || strings.HasPrefix(m[2], "-") { + name, isTool, ok := commandWord(cmd) + if !ok { continue } - name := m[2] - known := tool[name] - if m[1] != "summer" { - known = app[name] + known := app[name] + if isTool { + known = tool[name] } if !known { problems = append(problems, Problem{File: d.file, Line: line, Rule: "command", @@ -102,6 +97,91 @@ func (s *site) checkCommands(docs []parsedDoc) ([]Problem, error) { return problems, nil } +// commandWord finds the summer or application command word in one shell +// command. tool is true for summer and `go run ./cmd/summer`, false for +// ./bin/{app} and bin/{app}. ok is false when the line is not one of +// those programs or when flags leave no command word. Flag handling +// matches cobra's stripFlags for a root whose only flag is the bool +// --help / -h: "--" ends the search, --help and -h take no value, any +// other --name or two-character -x consumes the next token, and a token +// that already contains "=" is skipped alone. +func commandWord(cmd string) (name string, tool bool, ok bool) { + fields := strings.Fields(strings.TrimSpace(cmd)) + if len(fields) > 0 && fields[0] == "$" { + fields = fields[1:] + } + for len(fields) > 0 && isAssignment(fields[0]) { + fields = fields[1:] + } + switch { + case len(fields) == 0: + return "", false, false + case fields[0] == "summer": + tool = true + fields = fields[1:] + case len(fields) >= 3 && fields[0] == "go" && fields[1] == "run" && fields[2] == "./cmd/summer": + tool = true + fields = fields[3:] + case isAppBin(fields[0]): + fields = fields[1:] + default: + return "", false, false + } + for len(fields) > 0 { + s := fields[0] + switch { + case s == "--": + return "", tool, false + case strings.HasPrefix(s, "-") && strings.Contains(s, "="): + fields = fields[1:] + case s == "--help" || s == "-h": + fields = fields[1:] + case strings.HasPrefix(s, "--") || (strings.HasPrefix(s, "-") && len(s) == 2): + if len(fields) < 3 { + return "", tool, false + } + fields = fields[2:] + case strings.HasPrefix(s, "-"): + fields = fields[1:] + default: + return s, tool, true + } + } + return "", tool, false +} + +// isAssignment reports a leading VAR=value token: a name of letters, +// digits and underscores that does not start with a digit. +func isAssignment(tok string) bool { + name, _, ok := strings.Cut(tok, "=") + if !ok || name == "" || (name[0] >= '0' && name[0] <= '9') { + return false + } + for _, r := range name { + if r != '_' && (r < '0' || r > '9') && (r < 'A' || r > 'Z') && (r < 'a' || r > 'z') { + return false + } + } + return true +} + +// isAppBin reports ./bin/{app} and bin/{app}, with app in [A-Za-z0-9._-]. +func isAppBin(tok string) bool { + rest, ok := strings.CutPrefix(tok, "./bin/") + if !ok { + rest, ok = strings.CutPrefix(tok, "bin/") + } + if !ok || rest == "" || strings.Contains(rest, "/") { + return false + } + for _, r := range rest { + if r != '.' && r != '_' && r != '-' && (r < '0' || r > '9') && (r < 'A' || r > 'Z') && (r < 'a' || r > 'z') { + return false + } + } + return true +} + // exampleCommandNames returns the string-literal Name of every // bonfire.Command composite literal (including the elided elements of a // []bonfire.Command literal) in the non-test Go files under dir. It parses diff --git a/internal/docsite/check_identifiers.go b/internal/docsite/check_identifiers.go index 65f7ddb..45afa45 100644 --- a/internal/docsite/check_identifiers.go +++ b/internal/docsite/check_identifiers.go @@ -284,10 +284,10 @@ func isUpper(s string) bool { // optionally followed by one ".Member". var goDocIdent = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)?$`) -// goDoc is the fallback for an index miss: `go doc ./ ` run in -// the repository root, so a declaration the index does not model still -// counts. It runs without a shell, on an argument list whose query matches -// goDocIdent. +// goDoc is the fallback for an index miss: `go doc -c ./ ` +// run in the repository root, so a declaration the index does not model +// still counts. -c makes the match case-sensitive. It runs without a +// shell, on an argument list whose query matches goDocIdent. func (idx *identIndex) goDoc(dir, query string) bool { if !goDocIdent.MatchString(query) { return false @@ -298,7 +298,7 @@ func (idx *identIndex) goDoc(dir, query string) bool { } ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - cmd := exec.CommandContext(ctx, "go", "doc", "./"+dir, query) + cmd := exec.CommandContext(ctx, "go", "doc", "-c", "./"+dir, query) cmd.Dir = idx.root cmd.Stdout, cmd.Stderr = nil, nil ok := cmd.Run() == nil diff --git a/internal/docsite/snippet.go b/internal/docsite/snippet.go index 92d5b97..a318369 100644 --- a/internal/docsite/snippet.go +++ b/internal/docsite/snippet.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "go/ast" + "go/build" "go/doc" "go/parser" "go/token" @@ -14,6 +15,8 @@ import ( "path/filepath" "slices" "strings" + "unicode" + "unicode/utf8" ) // Ref is a src= reference in a code fence info string: a @@ -106,11 +109,8 @@ func Extract(root string, ref Ref) (string, error) { if err := checkRootModule(rootReal, filepath.Dir(real)); err != nil { return "", err } - if !isTest { - tests, _ := filepath.Glob(filepath.Join(filepath.Dir(real), "*_test.go")) - if len(tests) == 0 { - return "", snippetError("package has no _test.go files, so go test does not cover it") - } + if err := checkBuiltGo(ref.Path, real, isTest); err != nil { + return "", err } } if ref.Fragment == "" { @@ -161,6 +161,53 @@ func checkRefPath(p string) error { return nil } +// buildPackage loads dir the way go test ./... does: build.Default, so +// the host GOOS, GOARCH, CGO_ENABLED and release tags apply. +func buildPackage(dir string) (*build.Package, error) { + return build.ImportDir(dir, 0) +} + +// checkBuiltGo refuses a .go src= target go test ./... does not compile: +// a testdata or underscore directory, a file left out of the default +// build, or a non-test file in a package with no tests. +func checkBuiltGo(refPath, real string, isTest bool) error { + for _, seg := range strings.Split(refPath, "/") { + if seg == "testdata" || strings.HasPrefix(seg, "_") { + return snippetError("file is in a directory go test ./... skips (testdata or a _-prefixed directory)") + } + } + pkg, err := buildPackage(filepath.Dir(real)) + if err != nil { + var noGo *build.NoGoError + if errors.As(err, &noGo) { + return snippetError("file is excluded from the default build (build constraint or GOOS/GOARCH file-name suffix)") + } + return snippetError("cannot load package: " + firstLine(err.Error())) + } + base := filepath.Base(real) + listed := func(groups ...[]string) bool { + for _, g := range groups { + if slices.Contains(g, base) { + return true + } + } + return false + } + if isTest { + if !listed(pkg.TestGoFiles, pkg.XTestGoFiles) { + return snippetError("file is excluded from the default build (build constraint or GOOS/GOARCH file-name suffix)") + } + return nil + } + if !listed(pkg.GoFiles, pkg.CgoFiles) { + return snippetError("file is excluded from the default build (build constraint or GOOS/GOARCH file-name suffix)") + } + if len(pkg.TestGoFiles) == 0 && len(pkg.XTestGoFiles) == 0 { + return snippetError("package has no _test.go files, so go test does not cover it") + } + return nil +} + // checkRootModule refuses a Go source that sits in a nested module (a // directory between it and the root holds a go.mod): root go test does not // run it. @@ -246,11 +293,9 @@ func extractIdent(file string, raw []byte, name string, isTest bool) (string, er if funcKey(d) != name { continue } - if isTest { - if err := checkTestIdent(file, name, true); err != nil { - return "", err - } - } + // An Example with output is a root, so it is not also asked + // to be reachable from some other test. Without output, go + // test compiles it and never runs it. if d.Recv == nil && strings.HasPrefix(name, "Example") && isTest { if !exampleHasOutput(f, name) { return "", snippetError(name + " has no // Output: comment, so go test compiles it but never runs it") @@ -258,6 +303,11 @@ func extractIdent(file string, raw []byte, name string, isTest bool) (string, er body := string(raw[offset(d.Body.Lbrace)+1 : offset(d.Body.Rbrace)]) return strings.Join(dedent(strings.Split(body, "\n")), "\n"), nil } + if isTest { + if err := checkTestIdent(file, name, true); err != nil { + return "", err + } + } start := d.Pos() if d.Doc != nil { start = d.Doc.Pos() @@ -342,25 +392,43 @@ type testGraph struct { files map[string]*ast.File // by path } -func isRoot(d *ast.FuncDecl) bool { - return d.Recv == nil && (strings.HasPrefix(d.Name.Name, "Test") || strings.HasPrefix(d.Name.Name, "Example")) +// isTestName is the cmd/go rule for a test function: "Test" alone, or +// "Test" followed by a rune that is not a lowercase letter. Testable +// is not a test. Benchmark and Fuzz are not roots. +func isTestName(name string) bool { + rest, ok := strings.CutPrefix(name, "Test") + if !ok { + return false + } + if rest == "" { + return true + } + r, _ := utf8.DecodeRuneInString(rest) + return !unicode.IsLower(r) } func loadTestGraph(dir string) (*testGraph, error) { - paths, err := filepath.Glob(filepath.Join(dir, "*_test.go")) + pkg, err := buildPackage(dir) if err != nil { - return nil, fmt.Errorf("docsite: %w", err) + var noGo *build.NoGoError + if !errors.As(err, &noGo) { + return nil, snippetError("cannot load package: " + firstLine(err.Error())) + } + pkg = &build.Package{} } g := &testGraph{fset: token.NewFileSet(), funcs: map[string][]*ast.FuncDecl{}, reachable: map[string]bool{}, referenced: map[string]bool{}, files: map[string]*ast.File{}} byName := map[string][]string{} // plain or method name -> keys var queue []string - for _, p := range paths { - f, err := parser.ParseFile(g.fset, p, nil, 0) + var files []*ast.File + for _, name := range append(append([]string{}, pkg.TestGoFiles...), pkg.XTestGoFiles...) { + p := filepath.Join(dir, name) + f, err := parser.ParseFile(g.fset, p, nil, parser.ParseComments) if err != nil { return nil, snippetError("cannot parse " + filepath.Base(p) + ": " + firstLine(err.Error())) } g.files[p] = f + files = append(files, f) for _, decl := range f.Decls { d, ok := decl.(*ast.FuncDecl) if !ok { @@ -369,12 +437,22 @@ func loadTestGraph(dir string) (*testGraph, error) { key := funcKey(d) g.funcs[key] = append(g.funcs[key], d) byName[d.Name.Name] = append(byName[d.Name.Name], key) - if isRoot(d) && !g.reachable[key] { + if d.Recv == nil && isTestName(d.Name.Name) && !g.reachable[key] { g.reachable[key] = true queue = append(queue, key) } } } + for _, ex := range doc.Examples(files...) { + if ex.Output == "" && !ex.EmptyOutput { + continue + } + key := "Example" + ex.Name + if !g.reachable[key] { + g.reachable[key] = true + queue = append(queue, key) + } + } for len(queue) > 0 { key := queue[0] queue = queue[1:] @@ -401,7 +479,7 @@ func loadTestGraph(dir string) (*testGraph, error) { return g, nil } -const notRunMessage = "fragment is not inside a Test or Example function, or a function one of them calls, so go test does not run it" +const notRunMessage = "fragment is not inside a Test or Example function (an Example counts only with an // Output: comment), or a function one of them calls, so go test does not run it" // checkTestIdent requires a _test.go declaration to be run by go test: a // function reachable from a Test or Example, or a type, var or const that