From 69da5df254dde62ddc9df3a22cbea35f6383bb04 Mon Sep 17 00:00:00 2001 From: Rodin Date: Fri, 15 May 2026 04:47:59 +0000 Subject: [PATCH] feat(#141): add validate-docmap subcommand Adds 'review-bot validate-docmap' for CI hard-fail on docmap coverage gaps. Usage: git diff --name-only origin/main HEAD | \ review-bot validate-docmap --docmap .review-bot/doc-map.yml --repo-root . Flags: --docmap (required) path to doc-map YAML file --repo-root (optional, default '.') root for resolving docs: paths Two checks, both always run: 1. Coverage: every stdin file must match at least one paths: glob. 2. Stale docs: every docs: entry must exist on disk under --repo-root. Exit codes: 0=clean, 1=failures found, 2=usage/parse error. Tests cover: clean pass, uncovered file, stale doc, both failures, empty stdin, blank-line stdin, and duplicate docs: deduplication. --- cmd/review-bot/main.go | 2 + cmd/review-bot/validatedocmap.go | 139 ++++++++++++ cmd/review-bot/validatedocmap_test.go | 296 ++++++++++++++++++++++++++ 3 files changed, 437 insertions(+) create mode 100644 cmd/review-bot/validatedocmap.go create mode 100644 cmd/review-bot/validatedocmap_test.go diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index e80fd0c..1ee0380 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -64,6 +64,8 @@ func main() { switch os.Args[1] { case "validate-url": os.Exit(runValidateURL(os.Args[2:])) + case "validate-docmap": + os.Exit(runValidateDocmap(os.Args[2:])) } } diff --git a/cmd/review-bot/validatedocmap.go b/cmd/review-bot/validatedocmap.go new file mode 100644 index 0000000..41fac7b --- /dev/null +++ b/cmd/review-bot/validatedocmap.go @@ -0,0 +1,139 @@ +package main + +import ( + "bufio" + "flag" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "gitea.weiker.me/rodin/review-bot/review" +) + +// runValidateDocmap implements the `review-bot validate-docmap` subcommand. +// +// It reads changed file paths from stdin (one per line, as produced by +// `git diff --name-only`), parses a doc-map YAML file, and performs two checks: +// +// 1. Coverage check: every changed file must be matched by at least one +// paths: glob in the docmap. Fails if any file is uncovered. +// +// 2. Stale-docs check: every docs: entry in the docmap must exist on disk +// (relative to --repo-root). Fails if any path is missing. +// +// Both checks always run — all failures are reported before exiting. +// +// Exit codes: +// +// 0 — clean (all files covered, all docs exist) +// 1 — one or more coverage or stale-doc failures +// 2 — usage error, missing flag, or YAML parse error +func runValidateDocmap(args []string) int { + fs := flag.NewFlagSet("validate-docmap", flag.ContinueOnError) + fs.SetOutput(errWriter) + + docmapFlag := fs.String("docmap", "", "Path to doc-map YAML file (required)") + repoRootFlag := fs.String("repo-root", ".", "Repo root for resolving docs: paths (default: cwd)") + + if err := fs.Parse(args); err != nil { + // flag.ContinueOnError already wrote the error to errWriter. + return 2 + } + + if *docmapFlag == "" { + fmt.Fprintln(errWriter, "Error: --docmap is required") + fmt.Fprintln(errWriter, "") + fmt.Fprintln(errWriter, "usage: review-bot validate-docmap --docmap [--repo-root ]") + fmt.Fprintln(errWriter, " Changed files are read from stdin, one per line.") + fmt.Fprintln(errWriter, " Example: git diff --name-only origin/main HEAD | review-bot validate-docmap --docmap .review-bot/doc-map.yml") + return 2 + } + + // Parse docmap YAML. + cfg, err := review.ParseDocMapConfig(*docmapFlag) + if err != nil { + fmt.Fprintf(errWriter, "Error: failed to parse docmap %q: %v\n", *docmapFlag, err) + return 2 + } + + // Read changed files from stdin. + changedFiles, err := readLines(os.Stdin) + if err != nil { + fmt.Fprintf(errWriter, "Error: failed to read stdin: %v\n", err) + return 2 + } + + failed := false + + // --- Check 1: Coverage --- + var uncovered []string + for _, f := range changedFiles { + if !review.FileCoveredByDocMap(cfg, f) { + uncovered = append(uncovered, f) + } + } + if len(uncovered) > 0 { + failed = true + fmt.Fprintln(errWriter, "ERROR: changed files with no docmap coverage:") + for _, f := range uncovered { + fmt.Fprintf(errWriter, " %s\n", f) + } + } + + // --- Check 2: Stale docs --- + repoRoot := filepath.Clean(*repoRootFlag) + staleDocs := checkStaleDocs(cfg, repoRoot) + if len(staleDocs) > 0 { + failed = true + fmt.Fprintln(errWriter, "ERROR: stale docmap docs: entries (paths do not exist):") + for _, d := range staleDocs { + fmt.Fprintf(errWriter, " %s\n", d) + } + } + + if failed { + return 1 + } + + fmt.Fprintln(outWriter, "OK: docmap is valid") + return 0 +} + +// checkStaleDocs returns deduplicated docs: entries that do not exist under repoRoot. +func checkStaleDocs(cfg *review.DocMapConfig, repoRoot string) []string { + seen := make(map[string]struct{}) + var stale []string + + for _, mapping := range cfg.Mappings { + for _, docPath := range mapping.Docs { + if docPath == "" { + continue + } + if _, ok := seen[docPath]; ok { + continue + } + seen[docPath] = struct{}{} + + fullPath := filepath.Join(repoRoot, filepath.FromSlash(docPath)) + if _, err := os.Stat(fullPath); err != nil { + stale = append(stale, docPath) + } + } + } + return stale +} + +// readLines reads all non-empty trimmed lines from r. +func readLines(r io.Reader) ([]string, error) { + scanner := bufio.NewScanner(r) + var lines []string + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line != "" { + lines = append(lines, line) + } + } + return lines, scanner.Err() +} diff --git a/cmd/review-bot/validatedocmap_test.go b/cmd/review-bot/validatedocmap_test.go new file mode 100644 index 0000000..b19ec4f --- /dev/null +++ b/cmd/review-bot/validatedocmap_test.go @@ -0,0 +1,296 @@ +package main + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +// makeDocmapYAML writes a YAML string to a temp file and returns its path. +func makeDocmapYAML(t *testing.T, content string) string { + t.Helper() + f, err := os.CreateTemp(t.TempDir(), "doc-map-*.yml") + if err != nil { + t.Fatalf("CreateTemp: %v", err) + } + defer f.Close() + if _, err := f.WriteString(content); err != nil { + t.Fatalf("WriteString: %v", err) + } + return f.Name() +} + +// makeDocFile creates a file (and any parent dirs) at the given path relative to dir. +func makeDocFile(t *testing.T, dir, rel string) { + t.Helper() + full := filepath.Join(dir, rel) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(full, []byte("# doc\n"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } +} + +// captureOutput redirects outWriter/errWriter to buffers for the duration of f. +func captureOutput(f func()) (stdout, stderr string) { + var outBuf, errBuf bytes.Buffer + origOut, origErr := outWriter, errWriter + outWriter = &outBuf + errWriter = &errBuf + defer func() { + outWriter = origOut + errWriter = origErr + }() + f() + return outBuf.String(), errBuf.String() +} + +func TestRunValidateDocmap_Clean(t *testing.T) { + dir := t.TempDir() + makeDocFile(t, dir, "docs/foo.md") + + docmap := makeDocmapYAML(t, ` +mappings: + - paths: + - "lib/foo/**" + docs: + - docs/foo.md +`) + + var code int + stdout, _ := captureOutput(func() { + code = runValidateDocmap([]string{ + "--docmap", docmap, + "--repo-root", dir, + }) + }) + // Provide stdin via the global os.Stdin would require process tricks. + // The implementation reads os.Stdin directly; for tests we need a different approach. + // See TestRunValidateDocmap_WithStdin_* below which use a helper that replaces os.Stdin. + // This case: empty stdin (no changed files) → clean. + if code != 0 { + t.Errorf("expected exit 0 for clean (empty stdin), got %d", code) + } + if !strings.Contains(stdout, "OK") { + t.Errorf("expected 'OK' in stdout, got %q", stdout) + } +} + +func TestRunValidateDocmap_MissingDocmapFlag(t *testing.T) { + var code int + _, stderr := captureOutput(func() { + code = runValidateDocmap([]string{}) + }) + if code != 2 { + t.Errorf("expected exit 2 for missing --docmap, got %d", code) + } + if !strings.Contains(stderr, "--docmap") { + t.Errorf("expected --docmap in stderr, got %q", stderr) + } +} + +func TestRunValidateDocmap_BadYAML(t *testing.T) { + docmap := makeDocmapYAML(t, "mappings: [{{invalid") + var code int + _, stderr := captureOutput(func() { + code = runValidateDocmap([]string{"--docmap", docmap}) + }) + if code != 2 { + t.Errorf("expected exit 2 for bad YAML, got %d", code) + } + if !strings.Contains(stderr, "failed to parse") { + t.Errorf("expected parse error in stderr, got %q", stderr) + } +} + +func TestRunValidateDocmap_StaleDocs(t *testing.T) { + dir := t.TempDir() + // docs/foo.md does NOT exist on disk. + + docmap := makeDocmapYAML(t, ` +mappings: + - paths: + - "lib/foo/**" + docs: + - docs/foo.md +`) + + var code int + _, stderr := captureOutput(func() { + code = runValidateDocmap([]string{ + "--docmap", docmap, + "--repo-root", dir, + }) + }) + if code != 1 { + t.Errorf("expected exit 1 for stale docs, got %d", code) + } + if !strings.Contains(stderr, "docs/foo.md") { + t.Errorf("expected stale path in stderr, got %q", stderr) + } + if !strings.Contains(stderr, "stale docmap") { + t.Errorf("expected 'stale docmap' in stderr, got %q", stderr) + } +} + +// stdinValidateDocmap runs runValidateDocmap with a synthetic stdin. +func stdinValidateDocmap(t *testing.T, stdinContent string, args []string) (code int, stdout, stderr string) { + t.Helper() + // Write stdin content to a temp file and redirect os.Stdin. + f, err := os.CreateTemp(t.TempDir(), "stdin-*") + if err != nil { + t.Fatalf("CreateTemp for stdin: %v", err) + } + defer f.Close() + if _, err := f.WriteString(stdinContent); err != nil { + t.Fatalf("WriteString for stdin: %v", err) + } + if _, err := f.Seek(0, 0); err != nil { + t.Fatalf("Seek for stdin: %v", err) + } + + origStdin := os.Stdin + os.Stdin = f + defer func() { os.Stdin = origStdin }() + + stdout, stderr = captureOutput(func() { + code = runValidateDocmap(args) + }) + return +} + +func TestRunValidateDocmap_UncoveredFile(t *testing.T) { + dir := t.TempDir() + makeDocFile(t, dir, "docs/foo.md") + + docmap := makeDocmapYAML(t, ` +mappings: + - paths: + - "lib/foo/**" + docs: + - docs/foo.md +`) + + code, _, stderr := stdinValidateDocmap(t, + "lib/bar/uncovered.ex\n", + []string{"--docmap", docmap, "--repo-root", dir}, + ) + if code != 1 { + t.Errorf("expected exit 1 for uncovered file, got %d", code) + } + if !strings.Contains(stderr, "lib/bar/uncovered.ex") { + t.Errorf("expected uncovered file in stderr, got %q", stderr) + } + if !strings.Contains(stderr, "no docmap coverage") { + t.Errorf("expected 'no docmap coverage' in stderr, got %q", stderr) + } +} + +func TestRunValidateDocmap_BothFailures(t *testing.T) { + dir := t.TempDir() + // docs/foo.md intentionally missing + + docmap := makeDocmapYAML(t, ` +mappings: + - paths: + - "lib/foo/**" + docs: + - docs/foo.md +`) + + code, _, stderr := stdinValidateDocmap(t, + "lib/bar/uncovered.ex\n", + []string{"--docmap", docmap, "--repo-root", dir}, + ) + if code != 1 { + t.Errorf("expected exit 1 for both failures, got %d", code) + } + if !strings.Contains(stderr, "no docmap coverage") { + t.Errorf("expected coverage error in stderr, got %q", stderr) + } + if !strings.Contains(stderr, "stale docmap") { + t.Errorf("expected stale-docs error in stderr, got %q", stderr) + } +} + +func TestRunValidateDocmap_EmptyStdin(t *testing.T) { + dir := t.TempDir() + makeDocFile(t, dir, "docs/foo.md") + + docmap := makeDocmapYAML(t, ` +mappings: + - paths: + - "lib/foo/**" + docs: + - docs/foo.md +`) + + code, stdout, _ := stdinValidateDocmap(t, + "", + []string{"--docmap", docmap, "--repo-root", dir}, + ) + if code != 0 { + t.Errorf("expected exit 0 for empty stdin, got %d", code) + } + if !strings.Contains(stdout, "OK") { + t.Errorf("expected 'OK' in stdout, got %q", stdout) + } +} + +func TestRunValidateDocmap_BlankLinesSkipped(t *testing.T) { + dir := t.TempDir() + makeDocFile(t, dir, "docs/foo.md") + + docmap := makeDocmapYAML(t, ` +mappings: + - paths: + - "lib/foo/**" + docs: + - docs/foo.md +`) + + // stdin with only blank lines → effectively empty, should be clean + code, stdout, _ := stdinValidateDocmap(t, + "\n \n\n", + []string{"--docmap", docmap, "--repo-root", dir}, + ) + if code != 0 { + t.Errorf("expected exit 0 for blank-only stdin, got %d", code) + } + if !strings.Contains(stdout, "OK") { + t.Errorf("expected 'OK' in stdout for blank-only stdin, got %q", stdout) + } +} + +func TestRunValidateDocmap_DuplicateDocsDeduped(t *testing.T) { + dir := t.TempDir() + // docs/shared.md intentionally missing — but it appears in TWO mappings. + // Should appear only once in stale list. + + docmap := makeDocmapYAML(t, ` +mappings: + - paths: + - "lib/foo/**" + docs: + - docs/shared.md + - paths: + - "lib/bar/**" + docs: + - docs/shared.md +`) + + code, _, stderr := stdinValidateDocmap(t, + "", + []string{"--docmap", docmap, "--repo-root", dir}, + ) + if code != 1 { + t.Errorf("expected exit 1 for stale doc, got %d", code) + } + count := strings.Count(stderr, "docs/shared.md") + if count != 1 { + t.Errorf("expected docs/shared.md to appear exactly once in stderr (deduplicated), got %d occurrences: %q", count, stderr) + } +}