Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion .github/workflows/go-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `@<sha>` for supply-chain safety; bump it when this workflow changes.

Expand Down
16 changes: 13 additions & 3 deletions doccov/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,23 @@ 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_<key>`) to be documented |

## 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.
- **Config accessors** (`-config`) = a separate group. `base` auto-generates a
`get_<key>`/`set_<key>` pair for every config option (secret options get only
`set_`). doccov derives them from the `configKey<X> = "<name>"` constants and
the `gen[Secret]ConfigOption(configKey<X>, …)` 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).
Expand All @@ -45,7 +53,9 @@ jobs:
uses: 1set/meta/.github/workflows/go-ci.yml@<pin>
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
```

Expand Down
113 changes: 101 additions & 12 deletions doccov/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,26 +44,33 @@ 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_<key>) to be documented")
flag.Parse()

dir := "."
if flag.NArg() > 0 {
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
}

Expand All @@ -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
Expand Down Expand Up @@ -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_<name> for every option,
// plus get_<name> for non-secret options. It is convention-based (best-effort):
// it reads the `configKey<X> = "<name>"` constants and the
// gen[Secret]ConfigOption(configKey<X>, …) 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.
Expand Down
42 changes: 37 additions & 5 deletions doccov/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ package main

import (
"go/parser"
"os"
"path/filepath"
"strings"
"testing"
)
Expand Down Expand Up @@ -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")
}
Expand All @@ -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")
}
}
10 changes: 10 additions & 0 deletions doccov/testdata/config/mod.go
Original file line number Diff line number Diff line change
@@ -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", "")
Loading