diff --git a/dev/adr_lint.py b/dev/adr_lint.py index 6ff62e6..1e51b08 100644 --- a/dev/adr_lint.py +++ b/dev/adr_lint.py @@ -11,16 +11,23 @@ * carry no lifecycle field (Status, Date, Last reviewed, Deciders) and no ``## Lifecycle`` footer; * not be a tombstone: no superseded, deprecated, rejected or obsolete marker (the audit's DOC-04 rule); * hold the four template sections, in order: Context, Decision, Alternatives Considered, Consequences; -* have a row in the root index, :data:`INDEX`, which links it by path. +* have a row, a link by path, in each index of :data:`INDEXES` that covers its folder: the root index covers every + folder, and the engine's architecture page covers the engine's. -The index must not link a numbered ADR that does not exist. Exit 0 when all hold, 1 when one fails (each failure is -printed), 2 on a usage error. ``make adr-lint`` runs it inside ``make check``. Standard library only. +An index must not link a numbered ADR that does not exist. No Markdown page may name an ADR that does not exist: a +path such as ``adrs/0004-slug.md`` must resolve in an ADR folder, and "ADR 0004" must be a number some folder uses. +Generated history (``CHANGELOG.md``), fixtures, scratch and the greenfield templates (whose ADR numbers belong to a +new project) are not read. The number check cannot tell folders apart: "ADR 0001" passes while any folder has one. + +Exit 0 when all hold, 1 when one fails (each failure is printed), 2 on a usage error. ``make adr-lint`` runs it +inside ``make check``. Standard library only. """ from __future__ import annotations import argparse import importlib +import os import re import sys from pathlib import Path @@ -40,12 +47,24 @@ "audit/engine/docs/architecture/adrs", ) INDEX = "docs/architecture/adrs/README.md" +ENGINE_INDEX = "audit/engine/docs/architecture/README.md" +#: Each index and the ADR folders it must list in full. +INDEXES: tuple[tuple[str, tuple[str, ...]], ...] = ( + (INDEX, ADR_FOLDERS), + (ENGINE_INDEX, ("audit/engine/docs/architecture/adrs",)), +) SECTIONS = ("Context", "Decision", "Alternatives Considered", "Consequences") ADR_FILE = re.compile(r"^(\d{4})-[a-z0-9-]+\.md$") LIFECYCLE_FIELD = re.compile(r"^[ \t>*_-]*(status|date|last reviewed|deciders)[ \t*_]*:", re.IGNORECASE | re.MULTILINE) LIFECYCLE_FOOTER = re.compile(r"^#{1,6}\s*lifecycle\b", re.IGNORECASE | re.MULTILINE) LINK = re.compile(r"\]\(([^)#\s]+)(?:#[^)]*)?\)") +ADR_NUMBER_MENTION = re.compile(r"\bADR[- ]?(\d{1,4})\b") +ADR_PATH_MENTION = re.compile(r"(?:adrs|decisions)/(\d{4}-[A-Za-z0-9._-]+\.md)") +#: Directories never read for mentions: scratch (off-limits), generated output, fixtures and the greenfield templates. +SKIP_DIRS = {".git", "node_modules", ".venv", "artefacts", "fixtures", "testdata", "dist", "__pycache__"} +SKIP_PREFIXES = ("greenfield/templates/",) +SKIP_FILES = {"CHANGELOG.md"} def adr_files(root: Path) -> list[Path]: @@ -85,13 +104,63 @@ def lint_adr(path: Path, text: str) -> list[str]: return problems -def index_links(root: Path) -> set[Path]: - """The files the root index links to, resolved.""" - index = root / INDEX +def index_links(index: Path) -> set[Path]: + """The files ``index`` links to, resolved.""" text = index.read_text(encoding="utf-8") return {(index.parent / target).resolve() for target in LINK.findall(text) if "://" not in target} +def lint_index(root: Path, index_rel: str, folders: tuple[str, ...], adrs: list[Path]) -> list[str]: + """``index_rel`` links every ADR in ``folders``, and no numbered ADR there that does not exist.""" + index = root / index_rel + covered = {(root / folder).resolve() for folder in folders} + if not index.is_file(): + holds = any(path.resolve().parent in covered for path in adrs) + return [f"{index_rel}: the ADR index is missing"] if holds else [] + linked = index_links(index) + failures = [ + f"{path.relative_to(root).as_posix()}:1: no row in {index_rel}" + for path in adrs + if path.resolve().parent in covered and path.resolve() not in linked + ] + for target in sorted(linked): + if target.parent in covered and ADR_FILE.match(target.name) and not target.is_file(): + failures.append(f"{index_rel}: links {target.relative_to(root.resolve()).as_posix()}, which does not exist") + return failures + + +def markdown_pages(root: Path) -> list[Path]: + """The Markdown pages read for ADR mentions; skipped folders are pruned before the walk enters them.""" + pages: list[Path] = [] + for directory, subdirs, files in os.walk(root): + rel_dir = Path(directory).relative_to(root).as_posix() + subdirs[:] = sorted( + d for d in subdirs if d not in SKIP_DIRS and not f"{rel_dir}/{d}/".lstrip("./").startswith(SKIP_PREFIXES) + ) + pages += [Path(directory) / name for name in sorted(files) if name.endswith(".md") and name not in SKIP_FILES] + return pages + + +def lint_mentions(root: Path, adrs: list[Path]) -> list[str]: + """Every mention of an ADR number or path that no ADR folder holds.""" + numbers = {path.name[:4] for path in adrs} + names = {path.name for path in adrs} + indexes = {index_rel for index_rel, _ in INDEXES} + failures: list[str] = [] + for page in markdown_pages(root): + rel = page.relative_to(root).as_posix() + # An index's ADR paths are its rows, which lint_index checks; report each dead row once. + path_mention = None if rel in indexes else ADR_PATH_MENTION + for number, line in enumerate(page.read_text(encoding="utf-8", errors="replace").splitlines(), 1): + for match in ADR_NUMBER_MENTION.finditer(line): + if match.group(1).zfill(4) not in numbers: + failures.append(f"{rel}:{number}: names {match.group(0)}, which no ADR folder holds") + for match in path_mention.finditer(line) if path_mention else (): + if match.group(1) not in names: + failures.append(f"{rel}:{number}: names {match.group(1)}, which no ADR folder holds") + return failures + + def lint(root: Path) -> list[str]: """Every failure under ``root``, each as ``:: ``.""" failures: list[str] = [] @@ -99,18 +168,9 @@ def lint(root: Path) -> list[str]: for path in adrs: rel = path.relative_to(root).as_posix() failures += [f"{rel}:{problem}" for problem in lint_adr(path, path.read_text(encoding="utf-8"))] - - if not (root / INDEX).is_file(): - return failures + [f"{INDEX}: the ADR index is missing"] - linked = index_links(root) - for path in adrs: - if path.resolve() not in linked: - failures.append(f"{path.relative_to(root).as_posix()}:1: no row in {INDEX}") - folders = {(root / folder).resolve() for folder in ADR_FOLDERS} - for target in sorted(linked): - if target.parent in folders and ADR_FILE.match(target.name) and not target.is_file(): - failures.append(f"{INDEX}: links {target.relative_to(root.resolve()).as_posix()}, which does not exist") - return failures + for index_rel, folders in INDEXES: + failures += lint_index(root, index_rel, folders, adrs) + return failures + lint_mentions(root, adrs) def main(argv: Sequence[str] | None = None) -> int: diff --git a/dev/tests/test_adr_lint.py b/dev/tests/test_adr_lint.py index f0a8a6e..dffedd1 100644 --- a/dev/tests/test_adr_lint.py +++ b/dev/tests/test_adr_lint.py @@ -83,7 +83,9 @@ def test_sections_out_of_order_fail(self) -> None: ) -class LintTree(unittest.TestCase): +class TreeCase(unittest.TestCase): + """Builds a throwaway repository from a mapping of paths to text.""" + def tree(self, files: dict[str, str]) -> Path: tmp = tempfile.TemporaryDirectory() self.addCleanup(tmp.cleanup) @@ -94,6 +96,8 @@ def tree(self, files: dict[str, str]) -> Path: path.write_text(text, encoding="utf-8") return root + +class LintTree(TreeCase): def test_an_indexed_set_in_two_folders_passes(self) -> None: root = self.tree( { @@ -131,6 +135,75 @@ def test_main_returns_two_for_a_missing_root(self) -> None: self.assertEqual(adr_lint.main([str(REPO / "no-such-dir")]), 2) +ENGINE_ADR = "audit/engine/docs/architecture/adrs/0001-use-one-thing.md" +ROOT_ROW = "[0001](../../../audit/engine/docs/architecture/adrs/0001-use-one-thing.md)\n" + + +class EngineIndex(TreeCase): + def test_an_engine_adr_listed_in_both_indexes_passes(self) -> None: + root = self.tree( + {ENGINE_ADR: GOOD, adr_lint.INDEX: ROOT_ROW, adr_lint.ENGINE_INDEX: "[0001](adrs/0001-use-one-thing.md)\n"} + ) + self.assertEqual(adr_lint.lint(root), []) + + def test_an_engine_adr_missing_from_the_engine_index_fails(self) -> None: + root = self.tree({ENGINE_ADR: GOOD, adr_lint.INDEX: ROOT_ROW, adr_lint.ENGINE_INDEX: "No rows.\n"}) + self.assertEqual(adr_lint.lint(root), [f"{ENGINE_ADR}:1: no row in {adr_lint.ENGINE_INDEX}"]) + + def test_the_engine_index_linking_a_deleted_adr_fails(self) -> None: + engine_index = "[0001](adrs/0001-use-one-thing.md) [0002](adrs/0002-gone.md)\n" + root = self.tree({ENGINE_ADR: GOOD, adr_lint.INDEX: ROOT_ROW, adr_lint.ENGINE_INDEX: engine_index}) + self.assertEqual( + adr_lint.lint(root), + [f"{adr_lint.ENGINE_INDEX}: links audit/engine/docs/architecture/adrs/0002-gone.md, which does not exist"], + ) + + def test_a_missing_engine_index_fails_only_when_the_engine_has_adrs(self) -> None: + root = self.tree({ENGINE_ADR: GOOD, adr_lint.INDEX: ROOT_ROW}) + self.assertEqual(adr_lint.lint(root), [f"{adr_lint.ENGINE_INDEX}: the ADR index is missing"]) + root = self.tree( + { + "spec/decisions/0001-use-one-thing.md": GOOD, + adr_lint.INDEX: "[0001](../../../spec/decisions/0001-use-one-thing.md)\n", + } + ) + self.assertEqual(adr_lint.lint(root), []) + + +class Mentions(TreeCase): + def base(self, extra: dict[str, str]) -> Path: + files = { + "docs/architecture/adrs/0001-use-one-thing.md": GOOD, + adr_lint.INDEX: "[0001](0001-use-one-thing.md)\n", + } + return self.tree({**files, **extra}) + + def test_a_mention_of_an_existing_adr_passes(self) -> None: + root = self.base({"docs/page.md": "See ADR 0001, ADR-0001 and ADR 1, and adrs/0001-use-one-thing.md.\n"}) + self.assertEqual(adr_lint.lint(root), []) + + def test_a_number_no_folder_holds_fails(self) -> None: + root = self.base({"docs/page.md": "Intro.\nSee ADR 0007 for the old rule.\n"}) + self.assertEqual(adr_lint.lint(root), ["docs/page.md:2: names ADR 0007, which no ADR folder holds"]) + + def test_a_path_to_a_deleted_adr_fails(self) -> None: + root = self.base({"README.md": "Read `docs/architecture/adrs/0003-old-rule.md`.\n"}) + self.assertEqual(adr_lint.lint(root), ["README.md:1: names 0003-old-rule.md, which no ADR folder holds"]) + + def test_history_scratch_fixtures_and_greenfield_are_not_read(self) -> None: + stale = "ADR 0009 was removed.\n" + root = self.base( + { + "CHANGELOG.md": stale, + "artefacts/notes.md": stale, + "audit/engine/tests/fixtures/x/README.md": stale, + "dev/dist/skills/x.md": stale, + "greenfield/templates/docs/x.md": stale, + } + ) + self.assertEqual(adr_lint.lint(root), []) + + class ThisRepository(unittest.TestCase): def test_every_adr_here_follows_the_policy(self) -> None: out = io.StringIO() diff --git a/docs/ADR_POLICY.md b/docs/ADR_POLICY.md index 2012c32..ea235aa 100644 --- a/docs/ADR_POLICY.md +++ b/docs/ADR_POLICY.md @@ -37,7 +37,8 @@ Every ADR folder follows it: the root `docs/architecture/adrs/`, `spec/decisions 11. **Read only the ADRs you need.** Use the index summaries to pick the ADRs that govern the folder you change. `make adr-lint`, part of `make check`, checks the mechanical rules in every ADR folder. An ADR has no lifecycle -field or footer and no retired-ADR marker. It holds the four sections, and the root index has its row. +field or footer and no retired-ADR marker. It holds the four sections. The root index has its row, and so does +the engine's index for an engine ADR. No Markdown page names an ADR number or ADR path that no folder holds. ## Where else this applies diff --git a/docs/STYLE_GUIDE.md b/docs/STYLE_GUIDE.md index 302225d..0dbbb03 100644 --- a/docs/STYLE_GUIDE.md +++ b/docs/STYLE_GUIDE.md @@ -98,7 +98,7 @@ Paths are relative to `docs/`. | `dev/prose_check.py` | Sentence length, vague words, diagrams on explanation pages | Nowhere: advisory warnings and notices | | `dev/mermaid_check.py` | Each Mermaid block renders | The files you pass to `make docs-mermaid` | | `dev/recipe_lint.py` | Links resolve inside the repository | Recipes, `docs/` and the root pages | -| `dev/adr_lint.py` | Every ADR follows [`ADR_POLICY.md`](ADR_POLICY.md) and has a row in the root index | The five ADR folders | +| `dev/adr_lint.py` | Every ADR follows [`ADR_POLICY.md`](ADR_POLICY.md) and is indexed; no page names a missing ADR | The ADR folders, their indexes and every Markdown page | `DOCS_LINT_PATHS` is a ratchet: a page joins once it is clean, and the list only grows. Outside the list, the total count of Markdown issues may not go up (`.quality-baseline.json`). When you edit a page outside the list, fix diff --git a/docs/reference/dev-tooling.md b/docs/reference/dev-tooling.md index dfa3890..6203046 100644 --- a/docs/reference/dev-tooling.md +++ b/docs/reference/dev-tooling.md @@ -17,7 +17,7 @@ and where its version is pinned. The rules for the gates are in [`AGENTS.md`](.. | coverage.py | Test coverage below the floor in `.coveragerc` | `make coverage` (in CI: `coverage-part`, then `coverage-combine SHARDED=1`) | `test-coverage`, `coverage`; unsharded in `nightly.yml` | `dev/tools/pyproject.toml` | | jsonschema | The tools' schema validator disagrees with the reference package | `make crosscheck` | `crosscheck` | `dev/tools/pyproject.toml` | | `dev/recipe_lint.py` | Broken links, bad `SKILL.md` frontmatter, a recipe `SKILL.md` that does not link the shared user prompt, oversize files, unknown tools, check IDs and requirement IDs | `make recipe-lint` | `static` | This repository | -| `dev/adr_lint.py` | An ADR with a lifecycle field, a lifecycle footer or a retired-ADR marker, a missing template section, or no row in the [ADR index](../architecture/adrs/README.md); an index row to a missing ADR | `make adr-lint` | `static` | This repository | +| `dev/adr_lint.py` | An ADR with a lifecycle field, a lifecycle footer or a retired-ADR marker, a missing template section, or no row in the [ADR index](../architecture/adrs/README.md) (and, for an engine ADR, the engine's index); an index row to a missing ADR; a page that names an ADR number or path no folder holds | `make adr-lint` | `static` | This repository | | `dev/build_masters.py` | An [audit master list](audit-master.md) or [implement master list](implement-master.md) that no longer matches the catalogue, the check files or the playbooks | `make masters-check`; `make update-masters` rewrites both pages | `static` | This repository | | `dev/eval/generate_fixture_expected.py` | An eval-expected file (`dev/eval/expected/fixtures/`) that no longer matches a fresh engine run on its fixture | `make eval-expected-check`; the script without `--check` rewrites the files | `static` | This repository | | actionlint | Workflow mistakes: syntax, expressions, action inputs, shellcheck on `run:` blocks | `make workflow-lint` | `static` | `dev/tools/actionlint/go.mod` |