From 16ed3796ac7c387fbc209aae1e4d80303f2a2f32 Mon Sep 17 00:00:00 2001 From: Kevin Tang <73975146+vt128@users.noreply.github.com> Date: Mon, 15 Jun 2026 03:05:21 +0800 Subject: [PATCH] [feat] doccov: README<->builtin doc-consistency gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds doccov, a static gate that fails when a module exposes a Starlark builtin (starlark.NewBuiltin) that is not documented in its README — so docs cannot silently drift behind the code. - doccov/main.go: AST-scan non-test *.go for NewBuiltin first-arg names (handles "module.fn" and ModuleName+".fn"), check each appears as a backtick word in README.md; exit non-zero on omission. Stdlib only. - doccov/main_test.go + testdata: surface extraction, backtick coverage, pass/fail/no-builtins/ignore/missing-readme cases. - go.mod: meta becomes module github.com/1set/meta (go 1.19), so the tool is runnable as `go run github.com/1set/meta/doccov@master .`. - go-ci.yml: opt-in `doc-coverage` input + a gating step on the checks leg (fetched with `go run`, like govulncheck). Off by default; core libs unaffected. - selftest.yml: a job that runs `go test -race ./doccov/`. Validated against real modules: sqlite passes (29/29); markdown and qrcode each surface a genuine undocumented builtin (custom_converter, pure_ascii). Co-Authored-By: Claude Opus 4.8 --- .github/workflows/go-ci.yml | 12 ++ .github/workflows/selftest.yml | 14 +++ doccov/README.md | 53 +++++++++ doccov/main.go | 193 +++++++++++++++++++++++++++++++ doccov/main_test.go | 97 ++++++++++++++++ doccov/testdata/bad/README.md | 3 + doccov/testdata/bad/mod.go | 6 + doccov/testdata/empty/README.md | 1 + doccov/testdata/empty/mod.go | 3 + doccov/testdata/good/README.md | 3 + doccov/testdata/good/mod.go | 6 + doccov/testdata/good/mod_test.go | 5 + go.mod | 3 + 13 files changed, 399 insertions(+) create mode 100644 doccov/README.md create mode 100644 doccov/main.go create mode 100644 doccov/main_test.go create mode 100644 doccov/testdata/bad/README.md create mode 100644 doccov/testdata/bad/mod.go create mode 100644 doccov/testdata/empty/README.md create mode 100644 doccov/testdata/empty/mod.go create mode 100644 doccov/testdata/good/README.md create mode 100644 doccov/testdata/good/mod.go create mode 100644 doccov/testdata/good/mod_test.go create mode 100644 go.mod diff --git a/.github/workflows/go-ci.yml b/.github/workflows/go-ci.yml index ba7da5f..35db27a 100644 --- a/.github/workflows/go-ci.yml +++ b/.github/workflows/go-ci.yml @@ -32,6 +32,11 @@ on: required: false type: string default: "." + doc-coverage: + description: "Run the doccov README<->builtin consistency gate (starpkg domain modules)." + required: false + type: boolean + default: false secrets: CODECOV_TOKEN: required: false @@ -97,6 +102,13 @@ jobs: run: | go mod tidy git diff --exit-code -- go.mod go.sum + # Doc coverage (opt-in): every starlark.NewBuiltin a module exposes to + # scripts must be documented in its README. The gate lives in meta and is + # fetched with `go run` like govulncheck below; enable it via the + # doc-coverage input on starpkg domain modules. + - name: Doc coverage + if: ${{ fromJSON(env.IS_CHECKS) && inputs.doc-coverage }} + run: go run github.com/1set/meta/doccov@master . - name: Test run: make ci # govulncheck is informational (continue-on-error): it surfaces dependency diff --git a/.github/workflows/selftest.yml b/.github/workflows/selftest.yml index a4243a9..514177c 100644 --- a/.github/workflows/selftest.yml +++ b/.github/workflows/selftest.yml @@ -19,3 +19,17 @@ jobs: go-floor: "1.19" working-directory: "selftest" secrets: inherit + + # Tests the doccov gate itself (meta's own tool, in the root module). The + # doc-coverage step in go-ci.yml fetches this via `go run`, so it must stay green. + doccov: + name: doccov tool + runs-on: ubuntu-22.04 + permissions: + contents: read + steps: + - uses: actions/checkout@v5 + - uses: actions/setup-go@v6 + with: + go-version: "1.19.x" + - run: go test -race ./doccov/ diff --git a/doccov/README.md b/doccov/README.md new file mode 100644 index 0000000..7340f49 --- /dev/null +++ b/doccov/README.md @@ -0,0 +1,53 @@ +# doccov + +The documentation-consistency gate for the Star\* ecosystem. + +A Star\* module exposes its script-facing API as Starlark builtins, each built +with `starlark.NewBuiltin("", fn)`. `doccov` statically scans a module's Go +source for those builtins and **fails if any of them is not documented in the +module's README** — so the docs can never silently drift behind the code. + +## Usage + +```bash +# from a module directory +go run github.com/1set/meta/doccov@master . + +# elsewhere / a specific path +go run github.com/1set/meta/doccov@master path/to/module +``` + +Flags: + +| Flag | Default | Meaning | +|------|---------|---------| +| `-readme` | `README.md` | documentation file to check | +| `-ignore` | (empty) | comma-separated builtin names to exclude (deprecated/internal-but-registered) | + +## How it decides + +- **Surface** = the first string argument of every `starlark.NewBuiltin(...)` call + in the non-test (`*.go`, excluding `*_test.go`) files at the top of the directory. + A qualified name `"module.fn"` (and the `ModuleName + ".fn"` form) is reduced to `fn`. +- **Documented** = the name appears as a word inside a backtick span of the README. +- It checks for **omission**, not accuracy — a wrong description is a review concern. +- Exit status is non-zero on an undocumented builtin or a missing README; it is + **zero when no `starlark.NewBuiltin` calls are found** (the repo does not opt in). + +## In CI + +The reusable workflow `1set/meta/.github/workflows/go-ci.yml` runs this as an +opt-in gate. A starpkg domain module enables it in its caller: + +```yaml +jobs: + ci: + uses: 1set/meta/.github/workflows/go-ci.yml@ + with: + go-floor: "1.20" + doc-coverage: true # turn on the doccov gate + secrets: inherit +``` + +GoDoc completeness (a doc comment on every exported symbol) is a separate concern, +covered by `revive`'s `exported` rule in the same workflow's Analyze step. diff --git a/doccov/main.go b/doccov/main.go new file mode 100644 index 0000000..c1465f7 --- /dev/null +++ b/doccov/main.go @@ -0,0 +1,193 @@ +// Command doccov is the documentation-consistency gate for the Star* ecosystem. +// +// A Star* module exposes its script-facing API as Starlark builtins, each +// constructed with starlark.NewBuiltin("", fn). doccov statically scans a +// module's Go source for those builtins and fails if any of them is not +// documented in the module's README — so the docs can never silently drift +// behind the code. +// +// Usage: +// +// doccov [flags] [dir] # dir defaults to the current directory +// go run github.com/1set/meta/doccov@ . +// +// Flags: +// +// -readme documentation file to check (default "README.md") +// -ignore a,b,c builtin names to exclude (deprecated/internal-but-registered) +// +// It scans only non-test *.go files in dir (top level), so test-only builtins do +// not count as public surface. A builtin name of the form "module.fn" is reduced +// to "fn" before the README is checked. A symbol counts as documented when it +// appears as a word inside any backtick span in the README; doccov guards against +// omission, not against an inaccurate description (that is a review concern). +// +// Exit status is non-zero when a builtin is undocumented or the README is +// missing; it is zero when no starlark.NewBuiltin calls are found (the repo does +// not opt into this convention). +package main + +import ( + "flag" + "fmt" + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "regexp" + "sort" + "strconv" + "strings" +) + +func main() { + readme := flag.String("readme", "README.md", "documentation file to check") + ignore := flag.String("ignore", "", "comma-separated builtin names to exclude") + flag.Parse() + + dir := "." + if flag.NArg() > 0 { + dir = flag.Arg(0) + } + + if err := run(dir, *readme, splitCSV(*ignore)); err != nil { + fmt.Fprintln(os.Stderr, "doccov: "+err.Error()) + os.Exit(1) + } +} + +func run(dir, readmeName string, ignore map[string]bool) error { + surface, err := scanSurface(dir) + if err != nil { + return err + } + if len(surface) == 0 { + fmt.Println("doccov: no starlark.NewBuiltin calls found; nothing to check") + return nil + } + + readmePath := filepath.Join(dir, readmeName) + data, err := os.ReadFile(readmePath) + if err != nil { + return fmt.Errorf("cannot read %s: %v", readmePath, err) + } + documented := backtickWords(string(data)) + + var missing []string + for _, name := range surface { + if ignore[name] || documented[name] { + continue + } + missing = append(missing, name) + } + sort.Strings(missing) + + fmt.Printf("doccov: %d script-facing builtins, %d documented, %d missing\n", + len(surface), len(surface)-len(missing), len(missing)) + if len(missing) > 0 { + return fmt.Errorf("undocumented in %s: %s", readmeName, strings.Join(missing, ", ")) + } + return nil +} + +// scanSurface returns the sorted, de-duplicated set of script-facing builtin +// names declared in the non-test Go files at the top level of dir. +func scanSurface(dir string) ([]string, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return nil, err + } + + set := map[string]bool{} + fset := token.NewFileSet() + for _, e := range entries { + name := e.Name() + if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { + continue + } + f, err := parser.ParseFile(fset, filepath.Join(dir, name), nil, 0) + if err != nil { + return nil, fmt.Errorf("parse %s: %v", name, err) + } + ast.Inspect(f, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok || len(call.Args) == 0 { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "NewBuiltin" { + return true + } + if pkg, ok := sel.X.(*ast.Ident); !ok || pkg.Name != "starlark" { + return true + } + if lit := stringLit(call.Args[0]); lit != "" { + set[shortName(lit)] = true + } + return true + }) + } + + out := make([]string, 0, len(set)) + for k := range set { + out = append(out, k) + } + sort.Strings(out) + return out, nil +} + +// stringLit extracts a string constant from a builtin's first argument. It +// resolves a plain literal ("module.fn") and the common "ModuleName + \".fn\"" +// concatenation, returning the literal portion. +func stringLit(e ast.Expr) string { + switch v := e.(type) { + case *ast.BasicLit: + if v.Kind == token.STRING { + if s, err := strconv.Unquote(v.Value); err == nil { + return s + } + } + case *ast.BinaryExpr: + if l := stringLit(v.X); l != "" { + return l + } + return stringLit(v.Y) + } + return "" +} + +// shortName reduces a qualified builtin name ("module.fn") to its final segment. +func shortName(name string) string { + if i := strings.LastIndex(name, "."); i >= 0 { + return name[i+1:] + } + return name +} + +var ( + backtickSpan = regexp.MustCompile("`[^`]+`") + wordToken = regexp.MustCompile(`[A-Za-z_][A-Za-z0-9_]*`) +) + +// backtickWords collects every identifier word appearing inside a backtick span +// of the document. +func backtickWords(doc string) map[string]bool { + out := map[string]bool{} + for _, span := range backtickSpan.FindAllString(doc, -1) { + for _, w := range wordToken.FindAllString(span, -1) { + out[w] = true + } + } + return out +} + +func splitCSV(s string) map[string]bool { + out := map[string]bool{} + for _, part := range strings.Split(s, ",") { + if p := strings.TrimSpace(part); p != "" { + out[p] = true + } + } + return out +} diff --git a/doccov/main_test.go b/doccov/main_test.go new file mode 100644 index 0000000..1eac246 --- /dev/null +++ b/doccov/main_test.go @@ -0,0 +1,97 @@ +// Tests for the doccov gate, grouped by goal: +// - surface extraction (scanSurface, stringLit, shortName): which builtins are found +// - README coverage (backtickWords): which names count as documented +// - end-to-end (run): pass / fail / no-builtins / ignore behaviour +package main + +import ( + "go/parser" + "strings" + "testing" +) + +func TestScanSurface(t *testing.T) { + got, err := scanSurface("testdata/good") + if err != nil { + t.Fatalf("scanSurface: %v", err) + } + want := []string{"alpha", "beta"} // "good.alpha" -> "alpha"; "test_only" excluded (_test.go) + if strings.Join(got, ",") != strings.Join(want, ",") { + t.Fatalf("surface = %v, want %v", got, want) + } +} + +func TestStringLitFromExpr(t *testing.T) { + // stringLit must resolve a plain literal and the "ModuleName + \".fn\"" form. + cases := map[string]string{ + `"connect"`: "connect", + `"sqlite.connect"`: "sqlite.connect", + `ModuleName + ".connect"`: ".connect", + } + for src, want := range cases { + expr, err := parser.ParseExpr(src) + if err != nil { + t.Fatalf("ParseExpr(%q): %v", src, err) + } + if got := stringLit(expr); got != want { + t.Errorf("stringLit(%q) = %q, want %q", src, got, want) + } + } +} + +func TestShortName(t *testing.T) { + for in, want := range map[string]string{ + "connect": "connect", + "sqlite.connect": "connect", + ".connect": "connect", + } { + if got := shortName(in); got != want { + t.Errorf("shortName(%q) = %q, want %q", in, got, want) + } + } +} + +func TestBacktickWords(t *testing.T) { + doc := "Use `alpha` and `db.beta(x)`; plain gamma is not in backticks." + words := backtickWords(doc) + if !words["alpha"] || !words["beta"] || !words["db"] { + t.Errorf("expected alpha/beta/db in backticks, got %v", words) + } + if words["gamma"] { + t.Errorf("gamma is outside backticks and must not count as documented") + } +} + +func TestRunGood(t *testing.T) { + if err := run("testdata/good", "README.md", nil); err != nil { + t.Fatalf("good fixture should pass, got: %v", err) + } +} + +func TestRunBad(t *testing.T) { + err := run("testdata/bad", "README.md", nil) + if err == nil { + t.Fatal("bad fixture should fail: gamma is undocumented") + } + if !strings.Contains(err.Error(), "gamma") { + t.Fatalf("error should name the undocumented builtin, got: %v", err) + } +} + +func TestRunBadIgnored(t *testing.T) { + if err := run("testdata/bad", "README.md", map[string]bool{"gamma": true}); err != nil { + t.Fatalf("ignoring gamma should pass, got: %v", err) + } +} + +func TestRunNoBuiltins(t *testing.T) { + if err := run("testdata/empty", "README.md", nil); err != nil { + t.Fatalf("a repo with no builtins must not fail, got: %v", err) + } +} + +func TestRunMissingReadme(t *testing.T) { + if err := run("testdata/good", "NOPE.md", nil); err == nil { + t.Fatal("a missing documentation file should fail") + } +} diff --git a/doccov/testdata/bad/README.md b/doccov/testdata/bad/README.md new file mode 100644 index 0000000..106887c --- /dev/null +++ b/doccov/testdata/bad/README.md @@ -0,0 +1,3 @@ +# bad + +Only `alpha` is documented here. diff --git a/doccov/testdata/bad/mod.go b/doccov/testdata/bad/mod.go new file mode 100644 index 0000000..c296ee3 --- /dev/null +++ b/doccov/testdata/bad/mod.go @@ -0,0 +1,6 @@ +package bad + +import "go.starlark.net/starlark" + +var _ = starlark.NewBuiltin("bad.alpha", nil) +var _ = starlark.NewBuiltin("bad.gamma", nil) diff --git a/doccov/testdata/empty/README.md b/doccov/testdata/empty/README.md new file mode 100644 index 0000000..1bb8bf6 --- /dev/null +++ b/doccov/testdata/empty/README.md @@ -0,0 +1 @@ +# empty diff --git a/doccov/testdata/empty/mod.go b/doccov/testdata/empty/mod.go new file mode 100644 index 0000000..1eb5e74 --- /dev/null +++ b/doccov/testdata/empty/mod.go @@ -0,0 +1,3 @@ +package empty + +func helper() {} diff --git a/doccov/testdata/good/README.md b/doccov/testdata/good/README.md new file mode 100644 index 0000000..f1f666f --- /dev/null +++ b/doccov/testdata/good/README.md @@ -0,0 +1,3 @@ +# good + +Exposes `alpha` and `beta` to scripts. diff --git a/doccov/testdata/good/mod.go b/doccov/testdata/good/mod.go new file mode 100644 index 0000000..d412725 --- /dev/null +++ b/doccov/testdata/good/mod.go @@ -0,0 +1,6 @@ +package good + +import "go.starlark.net/starlark" + +var _ = starlark.NewBuiltin("good.alpha", nil) +var _ = starlark.NewBuiltin("beta", nil) diff --git a/doccov/testdata/good/mod_test.go b/doccov/testdata/good/mod_test.go new file mode 100644 index 0000000..d6dbe35 --- /dev/null +++ b/doccov/testdata/good/mod_test.go @@ -0,0 +1,5 @@ +package good + +import "go.starlark.net/starlark" + +var _ = starlark.NewBuiltin("test_only", nil) diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..43200f0 --- /dev/null +++ b/go.mod @@ -0,0 +1,3 @@ +module github.com/1set/meta + +go 1.19