diff --git a/.github/workflows/go-ci.yml b/.github/workflows/go-ci.yml index c8466aa..8b44203 100644 --- a/.github/workflows/go-ci.yml +++ b/.github/workflows/go-ci.yml @@ -52,6 +52,16 @@ on: required: false type: number default: 0 + doc-coverage-file: + description: "Documentation file doccov checks (e.g. 'docs/API.md' for the split layout; default README.md)." + required: false + type: string + default: "README.md" + doc-coverage-config: + description: "Also require base-generated config accessors (get_/set_) to be documented." + required: false + type: boolean + default: false secrets: CODECOV_TOKEN: required: false @@ -123,7 +133,7 @@ jobs: # 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 . + run: go run github.com/1set/meta/doccov@master -readme "${{ inputs.doc-coverage-file }}" ${{ inputs.doc-coverage-config && '-config' || '' }} . - name: Test run: make ci # Test-coverage gate (ratchet): covcheck parses the coverage.txt that diff --git a/README.md b/README.md index 9931888..2a65813 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,8 @@ gates run on the floor Linux leg. | `cov-min` | number | `0` | **covcheck** test-coverage floor (`0` = no gate) | | `footprint` | bool | `false` | run **footprint** (badge + bloat gate) | | `footprint-max-mb` | number | `0` | fail if the stripped footprint delta exceeds this many MB | +| `doc-coverage-file` | string | `README.md` | doc file doccov checks (e.g. `docs/API.md` for the split layout) | +| `doc-coverage-config` | bool | `false` | also gate the `base`-generated `get_`/`set_` config accessors | Pin the `@` for supply-chain safety; bump it when this workflow changes. diff --git a/doccov/README.md b/doccov/README.md index 7340f49..b8ce0e0 100644 --- a/doccov/README.md +++ b/doccov/README.md @@ -21,8 +21,9 @@ Flags: | Flag | Default | Meaning | |------|---------|---------| -| `-readme` | `README.md` | documentation file to check | -| `-ignore` | (empty) | comma-separated builtin names to exclude (deprecated/internal-but-registered) | +| `-readme` | `README.md` | documentation file to check (e.g. `docs/API.md` for the split layout) | +| `-ignore` | (empty) | comma-separated names to exclude (deprecated/internal-but-registered) | +| `-config` | off | also require the `base`-generated config accessors (`get_`/`set_`) to be documented | ## How it decides @@ -30,6 +31,13 @@ Flags: 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. +- **Config accessors** (`-config`) = a separate group. `base` auto-generates a + `get_`/`set_` pair for every config option (secret options get only + `set_`). doccov derives them from the `configKey = ""` constants and + the `gen[Secret]ConfigOption(configKey, …)` declarations (a declaration line + containing "Secret" — `genSecretConfigOption` or a chained `.SetSecret(true)` — + marks the option secret). Reported and gated as its own group, so the docs can + keep a separate *Configuration* section. - 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). @@ -45,7 +53,9 @@ jobs: uses: 1set/meta/.github/workflows/go-ci.yml@ with: go-floor: "1.20" - doc-coverage: true # turn on the doccov gate + doc-coverage: true # turn on the doccov gate + # doc-coverage-file: docs/API.md # split layout: check the API reference + # doc-coverage-config: true # also gate the get_/set_ config accessors secrets: inherit ``` diff --git a/doccov/main.go b/doccov/main.go index c1465f7..4673478 100644 --- a/doccov/main.go +++ b/doccov/main.go @@ -44,6 +44,7 @@ import ( func main() { readme := flag.String("readme", "README.md", "documentation file to check") ignore := flag.String("ignore", "", "comma-separated builtin names to exclude") + checkConfig := flag.Bool("config", false, "also require the base-generated config accessors (get_/set_) to be documented") flag.Parse() dir := "." @@ -51,19 +52,25 @@ func main() { dir = flag.Arg(0) } - if err := run(dir, *readme, splitCSV(*ignore)); err != nil { + if err := run(dir, *readme, splitCSV(*ignore), *checkConfig); err != nil { fmt.Fprintln(os.Stderr, "doccov: "+err.Error()) os.Exit(1) } } -func run(dir, readmeName string, ignore map[string]bool) error { +func run(dir, readmeName string, ignore map[string]bool, checkConfig 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") + var accessors []string + if checkConfig { + if accessors, err = scanConfig(dir); err != nil { + return err + } + } + if len(surface) == 0 && len(accessors) == 0 { + fmt.Println("doccov: no starlark.NewBuiltin calls or config options found; nothing to check") return nil } @@ -74,21 +81,39 @@ func run(dir, readmeName string, ignore map[string]bool) error { } documented := backtickWords(string(data)) + missBuiltins := undocumented(surface, documented, ignore) + missConfig := undocumented(accessors, documented, ignore) + + fmt.Printf("doccov: %s — builtins %d/%d documented, config accessors %d/%d documented\n", + readmeName, + len(surface)-len(missBuiltins), len(surface), + len(accessors)-len(missConfig), len(accessors)) + + var errs []string + if len(missBuiltins) > 0 { + errs = append(errs, "undocumented builtins: "+strings.Join(missBuiltins, ", ")) + } + if len(missConfig) > 0 { + errs = append(errs, "undocumented config accessors: "+strings.Join(missConfig, ", ")) + } + if len(errs) > 0 { + return fmt.Errorf("in %s: %s", readmeName, strings.Join(errs, "; ")) + } + return nil +} + +// undocumented returns the sorted names from want that are neither ignored nor +// present (as a backtick word) in documented. +func undocumented(want []string, documented, ignore map[string]bool) []string { var missing []string - for _, name := range surface { + for _, name := range want { 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 + return missing } // scanSurface returns the sorted, de-duplicated set of script-facing builtin @@ -137,6 +162,70 @@ func scanSurface(dir string) ([]string, error) { return out, nil } +var ( + configKeyConst = regexp.MustCompile(`(configKey\w+)\s*=\s*"([^"]+)"`) + configDeclLine = regexp.MustCompile(`ConfigOption\(\s*(configKey\w+)`) +) + +// scanConfig returns the sorted config-accessor builtin names that `base` +// auto-generates for a module's config options: set_ for every option, +// plus get_ for non-secret options. It is convention-based (best-effort): +// it reads the `configKey = ""` constants and the +// gen[Secret]ConfigOption(configKey, …) declarations; a declaration line +// containing "Secret" (genSecretConfigOption or a chained .SetSecret(true)) +// marks that option secret, so it gets no get_ accessor. Returns nil if the +// module follows neither convention. +func scanConfig(dir string) ([]string, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return nil, err + } + keyValue := map[string]string{} // configKey ident -> option name + declared := map[string]bool{} // configKey ident -> registered as an option + secret := map[string]bool{} // configKey ident -> secret (set_ only) + for _, e := range entries { + name := e.Name() + if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { + continue + } + data, err := os.ReadFile(filepath.Join(dir, name)) + if err != nil { + return nil, err + } + text := string(data) + for _, m := range configKeyConst.FindAllStringSubmatch(text, -1) { + keyValue[m[1]] = m[2] + } + for _, line := range strings.Split(text, "\n") { + m := configDeclLine.FindStringSubmatch(line) + if m == nil { + continue + } + declared[m[1]] = true + if strings.Contains(line, "Secret") { + secret[m[1]] = true + } + } + } + set := map[string]bool{} + for ident := range declared { + val, ok := keyValue[ident] + if !ok { + continue + } + set["set_"+val] = true + if !secret[ident] { + set["get_"+val] = 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. diff --git a/doccov/main_test.go b/doccov/main_test.go index 1eac246..d6cfe29 100644 --- a/doccov/main_test.go +++ b/doccov/main_test.go @@ -6,6 +6,8 @@ package main import ( "go/parser" + "os" + "path/filepath" "strings" "testing" ) @@ -63,13 +65,13 @@ func TestBacktickWords(t *testing.T) { } func TestRunGood(t *testing.T) { - if err := run("testdata/good", "README.md", nil); err != nil { + if err := run("testdata/good", "README.md", nil, false); err != nil { t.Fatalf("good fixture should pass, got: %v", err) } } func TestRunBad(t *testing.T) { - err := run("testdata/bad", "README.md", nil) + err := run("testdata/bad", "README.md", nil, false) if err == nil { t.Fatal("bad fixture should fail: gamma is undocumented") } @@ -79,19 +81,49 @@ func TestRunBad(t *testing.T) { } func TestRunBadIgnored(t *testing.T) { - if err := run("testdata/bad", "README.md", map[string]bool{"gamma": true}); err != nil { + if err := run("testdata/bad", "README.md", map[string]bool{"gamma": true}, false); 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 { + if err := run("testdata/empty", "README.md", nil, false); err != nil { t.Fatalf("a repo with no builtins must not fail, got: %v", err) } } +func TestScanConfig(t *testing.T) { + got, err := scanConfig("testdata/config") + if err != nil { + t.Fatalf("scanConfig: %v", err) + } + // width non-secret -> get_+set_ ; token secret -> set_ only + want := "get_width,set_token,set_width" + if strings.Join(got, ",") != want { + t.Fatalf("accessors = %v, want %s", got, want) + } +} + +func TestRunConfigGate(t *testing.T) { + dir := t.TempDir() + // copy the config fixture's go file + a README documenting only some accessors + src, _ := os.ReadFile("testdata/config/mod.go") + if err := os.WriteFile(filepath.Join(dir, "mod.go"), src, 0o644); err != nil { + t.Fatal(err) + } + // README documents get_width/set_width but NOT set_token + os.WriteFile(filepath.Join(dir, "README.md"), []byte("# c\n`get_width` `set_width`\n"), 0o644) + if err := run(dir, "README.md", nil, false); err != nil { + t.Fatalf("without -config, missing accessors must not fail: %v", err) + } + err := run(dir, "README.md", nil, true) + if err == nil || !strings.Contains(err.Error(), "set_token") { + t.Fatalf("with -config, undocumented set_token must fail, got: %v", err) + } +} + func TestRunMissingReadme(t *testing.T) { - if err := run("testdata/good", "NOPE.md", nil); err == nil { + if err := run("testdata/good", "NOPE.md", nil, false); err == nil { t.Fatal("a missing documentation file should fail") } } diff --git a/doccov/testdata/config/mod.go b/doccov/testdata/config/mod.go new file mode 100644 index 0000000..4531028 --- /dev/null +++ b/doccov/testdata/config/mod.go @@ -0,0 +1,10 @@ +package config + +const ( + configKeyWidth = "width" + configKeyToken = "token" +) + +// non-secret -> get_width + set_width ; secret -> set_token only +var _ = genConfigOption(configKeyWidth, "width", 0) +var _ = genSecretConfigOption(configKeyToken, "token", "")