diff --git a/.dependency-cruiser.cjs b/.dependency-cruiser.cjs index 1cabcc2..d7e1360 100644 --- a/.dependency-cruiser.cjs +++ b/.dependency-cruiser.cjs @@ -41,6 +41,35 @@ const AMBIENT = [ ]; const ambientPattern = `^(node:)?(${AMBIENT.join("|")})$`; +/** + * Packages an architecture decision already rejected, each with the record + * that rejected it. A denylist is cheaper than the argument a second time. + * + * @kubernetes/client-node the compiler renders, it does not apply: adapters + * build typed objects and one serializer owns the + * bytes (0067). + * ajv Zod is the one validator; a second one means two + * declarations of the same shape (0066). + * zod-to-json-schema Zod generates JSON Schema itself, from the input + * variant of each schema (0066). + * handlebars, ejs, text templating is the generation this compiler + * mustache, nunjucks replaces: objects in, one serializer out (0067). + */ +const DENIED = [ + "@kubernetes/client-node", + "ajv", + "zod-to-json-schema", + "handlebars", + "ejs", + "mustache", + "nunjucks", +]; +// The resolved path, not the specifier: an installed package resolves under +// node_modules/, and one that is merely written resolves to itself. +const deniedPattern = `^(node_modules/)?(${DENIED.map((name) => + name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"), +).join("|")})(/|$)`; + module.exports = { forbidden: [ { @@ -194,6 +223,27 @@ module.exports = { from: { path: "^src/", pathNot: "\\.test\\.ts$" }, to: { dependencyTypes: ["npm-dev"] }, }, + { + name: "no-denied-dependency", + severity: "error", + comment: + "A package an architecture decision already rejected. See the DENIED " + + "list above for which record rejected which, and " + + "docs/architecture-rules.md RULE-022 for the ledger row.", + from: { path: "^src/" }, + to: { path: deniedPattern }, + }, + { + name: "no-unresolvable-import", + severity: "error", + comment: + "A relative import that resolves to nothing is an edge no other rule " + + "can check: the graph cannot tell which ring it crossed. Bare " + + "specifiers are left to the package manager and the type checker, " + + "which say something more useful about a missing package.", + from: { path: "^src/" }, + to: { couldNotResolve: true, path: "^[.]" }, + }, { name: "not-to-deprecated-core", severity: "error", diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 29ae86f..7243875 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -127,6 +127,15 @@ - 'name': 'Requirements ledger' 'run': 'npm run lint:requirements' + # The rule ledger (docs/architecture-rules.md): every row resolves its + # enforcer, every enforced row names a fixture that asserts on its + # witness, every pending row carries a ticket and a reason, and every + # rule the two rule configurations name is claimed by exactly one + # enforced row. See + # docs/adr/architecture/0104-every-enforced-rule-has-an-id-a-row-and-a-fixture.md. + - 'name': 'Rule ledger' + 'run': 'npm run lint:rules' + # README.md and CONTRIBUTING.md, held to the repository they describe: # every named script exists, every backtick-quoted path exists, every # quoted coverage number matches vitest.config.ts, every quoted Node diff --git a/README.md b/README.md index 18ee136..20f556c 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ npm run verify # lint, format, typecheck, ADR contract, tests + coverage `npm run lint:adrs` alone runs the decision-record contract, and `npm test` runs the suite without enforcing coverage. `npm run test:coverage` (part of `npm run verify`) enforces the ratchet in `vitest.config.ts`: statements -97.94%, branches 90.72%, functions 100%, lines 97.76%. +98.31%, branches 92.43%, functions 100%, lines 98.19%. ## Conventions diff --git a/docs/adr/README.md b/docs/adr/README.md index f0020a1..43a80f3 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -51,7 +51,7 @@ which domain a decision lives in. | directory | holds | `normative:` pointers resolve against | linted | |---|---|---|---| | [`model/`](model/) | the v1 model: the layers, composition, derivation, the adapters | `spec/v1/` | yes | -| [`architecture/`](architecture/) | the compiler's own structure: layering, ports, error model, gates | `docs/architecture.md` | yes | +| [`architecture/`](architecture/) | the compiler's own structure: layering, ports, error model, gates | `docs/architecture.md`, `docs/architecture-rules.md` | yes | | [`deferred/`](deferred/README.md) | delivery mechanics and co-testing, defined separately | sections these chapters deliberately lack | no | ## Premises @@ -204,3 +204,4 @@ Decisions about the compiler's own structure, not about the model. Their | [0101](architecture/0101-coverage-is-a-ratchet.md) | Coverage is a ratchet: the thresholds sit on what the suite reaches, and only rise | settled | | [0102](architecture/0102-the-gate-grows-with-the-code.md) | A new gate's script and its CI job land in the same pull request, and a test proves the two stay matched | settled | | [0103](architecture/0103-a-behaviour-ledger-names-what-a-test-proves.md) | A behaviour ledger names every guarantee and the test that proves it, and a meta test holds the two together | settled | +| [0104](architecture/0104-every-enforced-rule-has-an-id-a-row-and-a-fixture.md) | Every enforced rule has an id, a ledger row and a fixture that proves it fires | settled | diff --git a/docs/adr/architecture/0104-every-enforced-rule-has-an-id-a-row-and-a-fixture.md b/docs/adr/architecture/0104-every-enforced-rule-has-an-id-a-row-and-a-fixture.md new file mode 100644 index 0000000..3413888 --- /dev/null +++ b/docs/adr/architecture/0104-every-enforced-rule-has-an-id-a-row-and-a-fixture.md @@ -0,0 +1,98 @@ +--- +tier: decision +status: proposed +claim: settled +date: 2026-09-14 +normative: docs/architecture-rules.md#rules +rests-on: ["0001"] +--- + +# Every enforced rule has an id, a ledger row and a fixture that proves it fires, and a rule not enforced yet says so with a reason + +## Rests on + +Every rule this repository enforces is enforced by a named thing on disk: a +rule in the dependency-cruiser ruleset, a rule id in the ESLint configuration, +a gate script, or a file that carries the rule itself. So a ledger of {id, +family, sentence, enforcer, fixture} rows can be checked by reading the tree +in both directions, rather than by trusting whoever last touched either side. +False if: a rule exists that no artifact on disk enforces, which would leave a +row with nothing to resolve to and no way to tell it from a rule quietly +deleted. Settled by: `scripts/lint-rules.ts` resolving every enforced row's +enforcer against the tree, and every rule the two rule configurations name +being claimed by exactly one enforced row, with none left over. + +## Why + +[0069](0069-boundaries-enforced-on-the-graph.md) put the layer boundaries on +the module graph because review is not the control that catches the +maintainer's own mistakes ([0001](../model/0001-estate-scale-and-ownership.md)). +That left a second-order gap, and it is the one this decision closes: a rule +can stop being enforced without anything going red. Delete a ruleset entry, +turn an ESLint rule off, rewrite a negative fixture past the case it was +written for, and the suite stays green, because a rule that no longer fires +breaks nothing. The coverage ratchet +([0101](0101-coverage-is-a-ratchet.md)) does not see it either: the lines still +run. + +The fix is the shape the other meta gates already take: state the rule as a +comparison over two things that exist on disk. Here the comparison runs both +ways, and the second direction is what makes it more than a document. Forwards, +a row must resolve: its enforcer must be configured, and its fixture must be a +real test that mentions the literal it asserts on, so deleting the case takes +the witness with it. Backwards, every rule the configurations name must be +claimed by exactly one **enforced** row, so a rule added without a row fails, +and so does a live rule quietly moved to the pending list. + +That backwards direction is the whole answer to the obvious objection, which is +that a pending list is where rules go to be forgotten. A row may be pending only +while nothing enforces it; the moment something does, pending stops being an +available answer. What is left is a pending row that names a ticket and a +reason and claims no fixture, which is a decision on the page rather than an +omission off it. + +The id is `RULE-NNN` and the family is a column rather than a prefix. The id is +what a failure quotes and what a grep finds, so it has to survive +reclassification: a family-prefixed id would become a lie the first time a rule +moved between families, and renumbering to fix that would break every citation +already written. `REQ-NNN` in +[the behaviour ledger](0103-a-behaviour-ledger-names-what-a-test-proves.md) is +a separate namespace on purpose: that ledger answers what a person can rely on, +this one answers what a tool enforces, and one rule can serve several +behaviours. + +## Alternatives + +| option | cost if taken | why rejected | +|---|---|---| +| No ledger; trust each rule's own negative fixture | Nothing to write or maintain | A fixture proves a rule fires while the fixture exists. Nothing notices the rule and its fixture leaving together, which is exactly how a ruleset decays | +| Fold the rules into `docs/requirements.md` | One document instead of two | A rule and a behaviour answer different questions, and one row cannot carry both an enforcer and a proof of a guarantee without one of them becoming decoration | +| A family-prefixed id, such as `LAY-004` | The family is readable in the id | Every reclassification breaks a citation, and the pressure is then to leave a rule in the wrong family rather than renumber. The family is a column, which can be revised for free | +| Pending rows in a separate file, out of the table | The enforced table stays short and clean | The point of a pending row is to be read beside the enforced ones. A second file is a list nobody opens | +| A severity column, so a rule can land as a warning first | A gentler path for a new rule | A warning in CI is a rule nobody enforces. A rule not worth failing a build for is a pending row with a reason, which is the same information without the false green | + +## Reversibility + +Undo cost today: deleting one document, one gate script and two test files. +Becomes irreversible once: never; the ledger describes this repository's own +tree, and nothing outside it reads either. + +## Consequences + +- Adding a rule to either rule configuration without a ledger row fails the + suite. Paid by whoever adds the rule, in the same pull request, which is + where the reason for it is freshest. +- An enforced row must name a fixture that asserts on a literal, so a rule + whose failure output nobody has ever looked at cannot be written down as + enforced. Paid once per rule. +- The ledger knows only about rules this repository names. The two presets + (`recommended` and `strictTypeChecked`) carry far more rules than any row + here, and they are one row between them; a preset silently narrowing in a + minor release is not something this gate would see. Paid by whoever upgrades + the linter. +- Most rows are pending today, because `src/` does not exist. That is the + ledger working: the vocabulary and the machinery land before the code they + will hold, so the compiler's first module meets them rather than prompting + them. Paid down by + [issue #30](https://github.com/JorisJonkers-dev/deploy-kit/issues/30), which + every pending row names. diff --git a/docs/adr/architecture/README.md b/docs/adr/architecture/README.md index 24a942b..558d32d 100644 --- a/docs/adr/architecture/README.md +++ b/docs/adr/architecture/README.md @@ -8,7 +8,9 @@ and no chapter of `spec/v1` depends on one. The consequence for the contract every ADR satisfies is one field: a `normative:` pointer here names a section of [`docs/architecture.md`](../../architecture.md), the normative document for code -structure, rather than a section of `spec/v1`. Everything else is unchanged +structure, or of [`docs/architecture-rules.md`](../../architecture-rules.md), +the ledger of rules that structure is held to, rather than a section of +`spec/v1`. Everything else is unchanged (frontmatter schema, a falsifiable claim per premise, `rests-on` naming premises only, the Alternatives table, the citation rule) and `scripts/lint-adrs.ts` enforces all of it here exactly as it does for the model, anchor check included. diff --git a/docs/architecture-rules.md b/docs/architecture-rules.md new file mode 100644 index 0000000..ae75e81 --- /dev/null +++ b/docs/architecture-rules.md @@ -0,0 +1,150 @@ +# Rule ledger + +This is not [`docs/requirements.md`](requirements.md). That ledger lists the +behaviours **a person depends on**: a sentence a contributor or a consumer can +rely on, keyed to the test that fails the moment it stops being true. This one +lists the rules **a tool enforces**: a dependency-cruiser check, an ESLint rule, +a gate script, each keyed to the enforcer that runs it and to the fixture that +proves it fires. A behaviour can rest on several rules, and one rule can serve +several behaviours, so the two ledgers stay separate on purpose, and their id +spaces (`REQ-NNN` and `RULE-NNN`) never overlap. + +A rule can stop being enforced without anything saying so: a +dependency-cruiser entry deleted, an ESLint rule turned off, a negative fixture +rewritten past the case it was written for. The suite stays green either way, +because a rule that no longer fires breaks nothing. +[`scripts/lint-rules.ts`](../scripts/lint-rules.ts) is what holds this document +honest: every row parses and carries a declared family; every enforced row's +enforcer resolves against the tree; every enforced row names a real test file +that mentions the literal its fixture asserts on; every pending row carries a +ticket and a reason and names no fixture; every rule this repository configures +by name is claimed by exactly one enforced row; the stated counts match what the +document holds; and every `RULE-NNN` cited anywhere in the tracked tree resolves +to a row here. + +## The id + +`RULE-NNN`, a flat estate-wide sequence, three digits, in a namespace of its +own so a grep for `RULE-017` finds the rule and a grep for `REQ-004` finds the +behaviour. The family is a **column, not a prefix**: a rule's family is a +reading aid that can be revised, and an id that encoded it would become a lie +the first time a rule was reclassified. The id is what a failure quotes, so it +has to outlive every opinion about where the rule belongs. + +## Families + +Eleven, the set [issue #21](https://github.com/JorisJonkers-dev/deploy-kit/issues/21) +named, each normalised to one word. None was added and none dropped: what this +repository's own enforcement changed is which families have members today +(`layering`, `purity`, `graph`, `dependencies`, `tests`, `toolchain` and +`gates`) and which are entirely pending until the compiler exists (`naming`, +`cli`, `registry`, `diagnostics`). A family declared here and used by no rule +fails the gate, so the taxonomy cannot grow entries nothing stands behind. + +| family | covers | +|---|---| +| layering | which ring may import which, on the module graph | +| purity | ambient capability: what a ring may reach for beyond its own imports | +| graph | the graph's shape: cycles, orphans, reachability, resolution | +| dependencies | what may be imported from outside the tree | +| naming | how modules, files and types are named | +| tests | what a test may do, and must do | +| toolchain | the compiler options, the lint presets, the coverage ratchet | +| cli | the outermost ring's own conventions | +| registry | the registries: adapters, and the estate-wide invariants | +| diagnostics | the shape of a failure and the codes it carries | +| gates | this repository's own documents and pipeline | + +## Rules + +This ledger holds **60** rules, **19** of them pending. + +A row is enforced or pending, never both. An enforced row names its enforcer as +`kind:value`: `depcruise:` a rule in +[`.dependency-cruiser.cjs`](../.dependency-cruiser.cjs), `eslint:` a rule id in +[`eslint.config.js`](../eslint.config.js), `npm:` a script in `package.json`, +`file:` a file that carries the rule itself. Its proof names the fixture and, +in backticks, the **witness**: the literal that fixture asserts on. Delete the +case and the witness goes with it, which is what makes the proof column +checkable rather than decorative. + +A pending row reads `pending (#NNN): reason`, or `pending (n/a): reason` when no +ticket will bring it because it waits on something outside this repository's +plan. It names no fixture: pending may never read as proven. And because every +rule this repository configures by name must be claimed by an **enforced** row, +moving a live rule to pending fails the gate rather than quietly retiring it. + +| id | family | the rule | enforced by | proved by | +|---|---|---|---|---| +| RULE-001 | layering | The domain imports nothing outside `src/domain/`: what it needs from the world arrives through a port it declares | `depcruise:domain-is-pure` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `domain-is-pure` | +| RULE-002 | layering | The domain never imports Zod: the authoring shape is not the model, and a mapper stands between them | `depcruise:domain-does-not-know-the-wire` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `domain-does-not-know-the-wire` | +| RULE-003 | layering | The wire layer parses and maps inward only: it renders nothing and orchestrates nothing | `depcruise:wire-maps-inward-only` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `wire-maps-inward-only` | +| RULE-004 | layering | The typed Kubernetes object model imports nothing from the compiler: it is a shape, not a participant | `depcruise:objects-are-data` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `objects-are-data` | +| RULE-005 | layering | No adapter imports another adapter; shared code goes to `src/adapters/shared/` | `depcruise:adapters-do-not-read-each-other` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `adapters-do-not-read-each-other` | +| RULE-006 | layering | A use-case takes ports, never a concrete infrastructure implementation | `depcruise:application-takes-ports-not-adapters` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `application-takes-ports-not-adapters` | +| RULE-007 | layering | Infrastructure implements ports: it does not orchestrate, parse or render | `depcruise:infrastructure-implements-ports-only` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `infrastructure-implements-ports-only` | +| RULE-008 | layering | Nothing inside imports the CLI ring | `depcruise:nothing-depends-on-the-cli` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `nothing-depends-on-the-cli` | +| RULE-009 | layering | Shipped code never imports a test file or anything under `dist/` | pending (#30): no `src/` exists yet, so the rule has nothing to constrain and no fixture tree can be shaped like the real one | pending | +| RULE-010 | purity | The domain reaches for no filesystem, network, clock, environment, process or crypto: hashing arrives through a port | `depcruise:domain-reads-nothing-ambient` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `domain-reads-nothing-ambient` | +| RULE-011 | purity | An adapter renders only: documents in, attributed Deliverables out, with no ambient read and no outward import | `depcruise:adapters-render-only` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `adapters-render-only` | +| RULE-012 | purity | Environment, clock, randomness, console, spawning and synchronous filesystem calls are allowed only in the infrastructure and CLI rings | pending (#30): needs a probe file per ring to prove it fires, and the rings do not exist until the first module lands | pending | +| RULE-013 | purity | Exiting the process and writing to stdout or stderr happen only in `src/cli/boundary.ts`, the one file excluded from coverage | pending (#30): the boundary file is the subject of its own decision record, which lands with the CLI ring | pending | +| RULE-014 | graph | No import cycle between modules | `depcruise:no-circular` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `no-circular` | +| RULE-015 | graph | No orphan module: every module but an entry point is imported by something | `depcruise:no-orphans` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `no-orphans` | +| RULE-016 | graph | Every module is reachable from an entry point, which is the half a coverage gate cannot see | `depcruise:unreachable-from-an-entry-point` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `unreachable-from-an-entry-point` | +| RULE-017 | graph | Every relative import in shipped code resolves to a file on disk; a bare specifier is the package manager's and the type checker's to answer for | `depcruise:no-unresolvable-import` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `no-unresolvable-import` | +| RULE-018 | graph | No cycle between directories, which a module-level cycle check cannot see | pending (#30): needs the collapsed-graph pass, and a directory layout with enough modules in it for the pass to mean anything | pending | +| RULE-019 | graph | No computed dynamic import: a specifier the graph cannot read is an edge no gate can check | pending (#30): the rule is only meaningful over shipped code, and the gates it would cover are static imports today | pending | +| RULE-020 | dependencies | Shipped code never imports a devDependency | `depcruise:no-dev-dependency-in-src` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `no-dev-dependency-in-src` | +| RULE-021 | dependencies | No deprecated Node builtin: a deprecated import is a migration already overdue | `depcruise:not-to-deprecated-core` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `not-to-deprecated-core` | +| RULE-022 | dependencies | No package an architecture decision already rejected: the Kubernetes client, a second JSON Schema validator, a Zod-to-JSON-Schema converter, or a text template engine | `depcruise:no-denied-dependency` | [test/boundary-contract.test.ts](../test/boundary-contract.test.ts) `no-denied-dependency` | +| RULE-023 | dependencies | A Node builtin is imported under its `node:` prefix | pending (#30): an ESLint rule over shipped code, which does not exist yet; the gates themselves already follow it by hand | pending | +| RULE-024 | naming | A module file is named in kebab-case | pending (#30): every file today is a gate or a test, and the rule lands with the lint that reads `src/` | pending | +| RULE-025 | naming | No `I`-prefixed interface name: the interface is the noun, and the implementation carries the adjective | pending (#30): needs a naming-convention rule over shipped declarations, which arrive with the first domain module | pending | +| RULE-026 | naming | A type-only import is written `import type`, so erasure is visible in the file rather than inferred | pending (#30): enforced by a type-aware rule over `src/`, and turned on with the first module it can read | pending | +| RULE-027 | tests | No committed `.only`: a focused test is a suite that is green for the wrong reason | `eslint:vitest/no-focused-tests` | [test/harness.test.ts](../test/harness.test.ts) `vitest/no-focused-tests` | +| RULE-028 | tests | No committed `.skip`: a disabled test proves nothing and reads as covered | `eslint:vitest/no-disabled-tests` | [test/harness.test.ts](../test/harness.test.ts) `vitest/no-disabled-tests` | +| RULE-029 | tests | No test without an assertion: a test that asserts nothing passes for ever | `eslint:vitest/expect-expect` | [test/harness.test.ts](../test/harness.test.ts) `vitest/expect-expect` | +| RULE-030 | tests | No fixed sleep through a timer global: slow when it passes, flaky when the machine is busy | `eslint:no-restricted-globals` | [test/harness.test.ts](../test/harness.test.ts) `no-restricted-globals` | +| RULE-031 | tests | No fixed sleep through the timers module either, which is the same defect under another import | `eslint:no-restricted-imports` | [test/harness.test.ts](../test/harness.test.ts) `no-restricted-imports` | +| RULE-032 | tests | No test reaches the network: the capability is removed, and using it fails the test that tried | `file:test/setup.ts` | [test/harness.test.ts](../test/harness.test.ts) `tried to reach` | +| RULE-033 | tests | A test never imports another test: a shared fixture belongs in `test/support/` | pending (#30): no test imports another today, and the rule wants a graph pass over `test/` that the boundary gate does not yet make | pending | +| RULE-034 | tests | The rendered tree is compared byte for byte against a committed golden tree, and a double render inside one process and in a fresh one agrees with it | pending (n/a): there is no renderer to render anything, and issue #21 puts the golden tree out of scope until one exists | pending | +| RULE-035 | toolchain | A TypeScript suppression carries a description and may never silence a whole file | `eslint:@typescript-eslint/ban-ts-comment` | [test/eslint-rules.test.ts](../test/eslint-rules.test.ts) `@typescript-eslint/ban-ts-comment` | +| RULE-036 | toolchain | No `any`: a value the compiler cannot describe is a check nobody runs | `eslint:@typescript-eslint/no-explicit-any` | [test/eslint-rules.test.ts](../test/eslint-rules.test.ts) `@typescript-eslint/no-explicit-any` | +| RULE-037 | toolchain | No unused binding, unless it is named with a leading underscore to say so | `eslint:@typescript-eslint/no-unused-vars` | [test/eslint-rules.test.ts](../test/eslint-rules.test.ts) `@typescript-eslint/no-unused-vars` | +| RULE-038 | toolchain | No object or nullable interpolated into a template literal: a number has one sensible string form, and those do not | `eslint:@typescript-eslint/restrict-template-expressions` | [test/eslint-rules.test.ts](../test/eslint-rules.test.ts) `@typescript-eslint/restrict-template-expressions` | +| RULE-039 | toolchain | The recommended and `strictTypeChecked` presets apply to every TypeScript file with type information, so a floating promise or an empty catch fails lint without this repository naming either rule | `file:eslint.config.js` | [test/eslint-rules.test.ts](../test/eslint-rules.test.ts) `@typescript-eslint/no-floating-promises` | +| RULE-040 | toolchain | Coverage is a ratchet over an explicit include list, and no ignore comment exempts a line from it | `file:vitest.config.ts` | [test/harness.test.ts](../test/harness.test.ts) `an ignore is slack nobody decided` | +| RULE-041 | toolchain | No default export outside a tool configuration file | pending (#30): the tool configs are the only modules with exports today, and they are the exception the rule carves out | pending | +| RULE-042 | toolchain | Shipped code is ESM, and the one CommonJS file is the dependency-cruiser configuration that cannot be anything else | pending (#30): stated by `type: module` and enforced by hand until a lint over `src/` can read it | pending | +| RULE-043 | toolchain | Generated artifacts are committed, and CI fails when regenerating one produces a diff | pending (#30): nothing generates anything yet; the rule lands with the first generator | pending | +| RULE-044 | cli | The CLI prints help on `--help` and `-h`, data on stdout and diagnostics on stderr, emits only data under `--json`, maps failures through one exit-code enum, honours `NO_COLOR`, and never prompts | pending (#30): the CLI ring does not exist, and each clause needs a process-level fixture to be worth a row of its own | pending | +| RULE-045 | registry | Every registered adapter satisfies the adapter port, attributes every Deliverable to itself, and renders deterministically | pending (#30): there is no registry and no adapter; the table-driven contract suite arrives with the first one | pending | +| RULE-046 | registry | Every estate-wide invariant is registered with its code, its spec anchor and its test, so an unregistered one is detectable rather than merely absent | pending (#30): the registry is a compiler module, and the enumeration it makes possible needs it to exist | pending | +| RULE-047 | diagnostics | Every diagnostic carries a code, a document path, a message and a non-empty hint, enforced by its type rather than by review | pending (#30): the `Diagnostic` type is the enforcement, and it lands with the error model | pending | +| RULE-048 | diagnostics | Every `E_` code the specification defines is exercised by a test or a negative example, or listed as pending with a reason, and a code used in the tree that the specification does not define fails | pending (#30): the reader over the spec chapters is that ticket's own deliverable, with 59 codes defined and 28 covered today | pending | +| RULE-049 | gates | Every decision record satisfies the frontmatter, register, citation and normative-anchor contract | `npm:lint:adrs` | [test/adr-lint-negative.test.ts](../test/adr-lint-negative.test.ts) `missing frontmatter block` | +| RULE-050 | gates | Every relative link and heading anchor in tracked Markdown resolves | `npm:lint:links` | [test/link-contract.test.ts](../test/link-contract.test.ts) `anchor #the-old-name not found` | +| RULE-051 | gates | Every rendered example object validates against its pinned Kubernetes or CRD schema | `npm:lint:manifests` | [test/manifest-contract.test.ts](../test/manifest-contract.test.ts) `no rendered examples found` | +| RULE-052 | gates | Every behaviour ledger row parses, names a real non-empty test, matches the stated count, and is cited by no dangling id | `npm:lint:requirements` | [test/requirements-lint-negative.test.ts](../test/requirements-lint-negative.test.ts) `ledger missing` | +| RULE-053 | gates | Every script, path, coverage number and Node version README.md and CONTRIBUTING.md name matches the repository | `npm:lint:docs` | [test/docs-contract.test.ts](../test/docs-contract.test.ts) `is not a script in package.json` | +| RULE-054 | gates | Every rule ledger row parses, resolves its enforcer, names a fixture that mentions its witness, and every configured rule is claimed by exactly one enforced row | `npm:lint:rules` | [test/rules-lint-negative.test.ts](../test/rules-lint-negative.test.ts) `ledger missing` | +| RULE-055 | gates | The npm package ships nothing outside `docs/adr/` and `spec/`, checked against what npm would really pack | `file:scripts/check-package-contents.ts` | [test/package-contents-contract.test.ts](../test/package-contents-contract.test.ts) `ship files outside docs` | +| RULE-056 | gates | A pull request title and every commit in it use a conventional-commit type release-please reads | `file:scripts/check-pr-title.ts` | [test/pr-title-contract.test.ts](../test/pr-title-contract.test.ts) `is not a Conventional Commit` | +| RULE-057 | gates | No pull request title, body or commit carries agent attribution: no trailer, no banner, no link back to a session | `file:scripts/check-pr-title.ts` | [test/pr-title-contract.test.ts](../test/pr-title-contract.test.ts) `google-labs-jules` | +| RULE-058 | gates | No em-dash enters tracked text outside `docs/mde/` and `CHANGELOG.md` | `file:test/emdash.test.ts` | [test/emdash.test.ts](../test/emdash.test.ts) `contains an em-dash` | +| RULE-059 | gates | A gate's npm script and the CI job that runs it land together, or the script is listed pending with a reason | `file:test/pipeline-wiring.test.ts` | [test/pipeline-wiring.test.ts](../test/pipeline-wiring.test.ts) `every script either runs in some workflow` | +| RULE-060 | gates | No committed secret matching the default gitleaks ruleset or this repository's own allowlist, checked locally by the same command CI runs | `npm:lint:secrets` | [test/secret-scan-contract.test.ts](../test/secret-scan-contract.test.ts) `secret scan: could not run` | + +## Considered and rejected + +Proposing one of these again starts from the argument, not from scratch. + +| rule | why not | +|---|---| +| A maximum file length, or a maximum function length | A number nobody can defend at the boundary. The rules that matter here are about edges in a graph, which have a truth value; a 41-line function does not | +| Inline suppressions for boundary rules, the way an `eslint-disable` comment works | An exemption a reviewer never sees is an exemption nobody decided. Where an exception is genuinely needed it belongs in the ruleset, named, with the reason beside it, which is the form every rule above already takes | +| One ESLint rule per layer, instead of the dependency-cruiser ruleset | ESLint sees one file at a time, so it can catch an import but never a cycle, an orphan or an unreachable subtree ([0069](adr/architecture/0069-boundaries-enforced-on-the-graph.md)). The fast local signal stays; the authority is the graph | +| Folding this ledger into `docs/requirements.md` | A rule and a behaviour answer different questions, and merging them would make the per-rule enforcer story and the per-behaviour test story compete for one row ([0103](adr/architecture/0103-a-behaviour-ledger-names-what-a-test-proves.md)) | +| A severity column, with `warn` as a value | A warning in CI is a rule nobody enforces. Every row above is an error, and a rule not worth failing a build for is a rule not worth a row | diff --git a/docs/architecture.md b/docs/architecture.md index ea3b83d..89dfc61 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -188,7 +188,7 @@ clean tree is untested: nothing proves it would fail. ## Gates -Thirteen gates hold the structure, and each exists because its absence has already +Fourteen gates hold the structure, and each exists because its absence has already cost something in the generation this compiler replaces. Each runs as its own CI job, aggregated by one required check that fails when any gate job fails, is cancelled, or is skipped @@ -207,18 +207,26 @@ proves the two never drift apart. | links | `npm run lint:links` | relative links and heading anchors across every tracked Markdown file | | manifests | `npm run lint:manifests` | every rendered example object against pinned Kubernetes and CRD schemas | | requirements | `npm run lint:requirements` | a behaviour ledger row that no longer parses, names a missing or empty test, drifts from its stated count, or is cited by an id no row carries | +| rules | `npm run lint:rules` | a [rule ledger](architecture-rules.md) row whose enforcer no longer exists, whose fixture no longer asserts on its witness, or that is pending with no ticket and no reason, and a rule the ruleset or the lint configuration enforces that no row claims | | docs | `npm run lint:docs` | a script, path, coverage number or Node version README.md or CONTRIBUTING.md name that no longer matches the repository | | tests | `npm run test:coverage` | behaviour, plus the coverage ratchet | | package contents | `node scripts/check-package-contents.ts` | `npm pack` shipping a file outside `docs/adr/` and `spec/`, the boundary the package's `files` field states but does not enforce on its own | | actionlint | a pinned `actionlint` binary | invalid workflow syntax, an undefined `${{ }}` expression, a shellcheck finding inside a `run:` step | | secret scan | `npm run lint:secrets` | a committed secret matching the default ruleset, or this repository's own allowlist entries | -Decisions, links, manifests, requirements and docs share one CI job, -`contracts`: all five check a document against a rule rather than code +Decisions, links, manifests, requirements, rules and docs share one CI job, +`contracts`: all six check a document against a rule rather than code against a graph. Boundaries runs alone as `architecture`, because it is the one gate that speaks for `docs/architecture.md` itself rather than for a document beside it. +Every rule these gates enforce is written down once, with a greppable id, in +the [rule ledger](architecture-rules.md) +([0104](adr/architecture/0104-every-enforced-rule-has-an-id-a-row-and-a-fixture.md)). +A row names the enforcer that runs the rule and the fixture that proves it +fires; a rule not enforced yet is listed as pending with a ticket and a reason, +and stops being allowed to say that the moment something enforces it. + Coverage is a ratchet ([0101](adr/architecture/0101-coverage-is-a-ratchet.md)). The thresholds in `vitest.config.ts` sit on what the suite reaches, over an explicit include list so a file no test reaches counts as zero, and they only diff --git a/docs/requirements.md b/docs/requirements.md index 7a40476..4a1fbe8 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -1,10 +1,9 @@ # Behaviour ledger -This is not `docs/architecture-rules.md` (issue -[#29](https://github.com/JorisJonkers-dev/deploy-kit/issues/29), not yet -landed). That ledger will list the rules a tool enforces: a dependency-cruiser -check, an ESLint message, a lint's error code, each keyed to its enforcer and -its severity. This one lists the behaviours a person depends on: a sentence a +This is not [`docs/architecture-rules.md`](architecture-rules.md). That ledger +lists the rules a tool enforces: a dependency-cruiser check, an ESLint rule, a +gate script, each keyed to its enforcer and to the fixture that proves it +fires. This one lists the behaviours a person depends on: a sentence a contributor or a consumer can rely on, keyed to the test that fails the moment it stops being true. A behaviour can rest on several rules, and one rule can serve several behaviours, so the two ledgers stay separate on purpose. @@ -20,7 +19,7 @@ test file and holds at least one test; ids are unique; the count this document states matches the number of rows it holds; and every id cited anywhere in the tracked tree resolves to a row here. -This ledger holds **13** rows. The compiler's behaviours join it as they land. +This ledger holds **14** rows. The compiler's behaviours join it as they land. | id | a contributor or a consumer can rely on | proved by | |---|---|---| @@ -37,3 +36,4 @@ This ledger holds **13** rows. The compiler's behaviours join it as they land. | REQ-011 | Every script, path, coverage number and Node version README.md and CONTRIBUTING.md name matches the repository they describe | [test/docs-contract.test.ts](../test/docs-contract.test.ts) | | REQ-012 | A pull request's title, body and every commit in it carry no agent attribution: no Co-Authored-By trailer naming a coding agent, no "generated with" banner naming one, no link back to an agent session | [test/pr-title-contract.test.ts](../test/pr-title-contract.test.ts) | | REQ-013 | `npm run verify` runs the same secret scan CI runs, failing on a committed secret rather than only after a push | [test/secret-scan-contract.test.ts](../test/secret-scan-contract.test.ts) | +| REQ-014 | Every rule this repository enforces has a ledger row with a greppable id and a fixture that proves it fires, and a rule not enforced yet is listed as pending with a ticket and a reason rather than dropped | [test/rules-contract.test.ts](../test/rules-contract.test.ts) | diff --git a/package.json b/package.json index 5e95b09..83d4e90 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "lint:manifests": "node scripts/lint-manifests.ts", "lint:boundaries": "node scripts/lint-boundaries.ts", "lint:requirements": "node scripts/lint-requirements.ts", + "lint:rules": "node scripts/lint-rules.ts", "lint:docs": "node scripts/lint-docs.ts", "lint:secrets": "node scripts/lint-secrets.ts", "format": "prettier --write .", @@ -26,7 +27,7 @@ "typecheck": "tsc --noEmit", "test": "vitest run", "test:coverage": "vitest run --coverage", - "verify": "npm run lint && npm run format:check && npm run typecheck && npm run lint:adrs && npm run lint:links && npm run lint:manifests && npm run lint:requirements && npm run lint:docs && npm run lint:secrets && npm run lint:boundaries && npm run test:coverage" + "verify": "npm run lint && npm run format:check && npm run typecheck && npm run lint:adrs && npm run lint:links && npm run lint:manifests && npm run lint:requirements && npm run lint:rules && npm run lint:docs && npm run lint:secrets && npm run lint:boundaries && npm run test:coverage" }, "devDependencies": { "@eslint/js": "10.0.1", diff --git a/scripts/lib/tracked.ts b/scripts/lib/tracked.ts new file mode 100644 index 0000000..118fc8c --- /dev/null +++ b/scripts/lib/tracked.ts @@ -0,0 +1,55 @@ +// The tracked tree, as text, and the dangling-citation scan over it. +// +// Two ledgers cite ids into the tree: docs/requirements.md (REQ-NNN) and +// docs/architecture-rules.md (RULE-NNN). Both need the same two things: every +// tracked file's text, and every id cited somewhere that no row declares. The +// scan is pure over a {path: content} map, so each gate's negative fixtures +// test it against a synthetic tree with no filesystem involved. +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +/** Every tracked file's text, keyed by its path relative to `root`. */ +export function trackedText(root: string): Record { + const files: Record = {}; + const tracked = execFileSync("git", ["ls-files", "-z"], { + cwd: root, + encoding: "utf8", + }).split("\0"); + for (const rel of tracked) { + if (rel === "") continue; + try { + files[rel] = readFileSync(join(root, rel), "utf8"); + } catch { + // A path that cannot be read as text (a symlink to nowhere, a + // directory entry) carries no citation either way. + } + } + return files; +} + +/** + * Every id matching `pattern` that a file cites and no row declares, reported + * once per file rather than once per occurrence, in path order. + */ +export function danglingCitations( + files: Readonly>, + ids: ReadonlySet, + pattern: RegExp, +): string[] { + const errors: string[] = []; + const reported = new Set(); + // Object keys are unique paths, so two entries are never equal: a strict + // less-than is enough to order them, with no equal case to fall through to. + const entries = Object.entries(files).sort(([a], [b]) => (a < b ? -1 : 1)); + for (const [rel, content] of entries) { + for (const match of content.matchAll(pattern)) { + const id = match[0]; + const key = `${rel}\0${id}`; + if (ids.has(id) || reported.has(key)) continue; + reported.add(key); + errors.push(`${rel} cites ${id}, which no row carries`); + } + } + return errors; +} diff --git a/scripts/lint-adrs.ts b/scripts/lint-adrs.ts index f2a8f0e..ff13d26 100644 --- a/scripts/lint-adrs.ts +++ b/scripts/lint-adrs.ts @@ -27,11 +27,17 @@ interface AdrFile { const REPOSITORY = join(import.meta.dirname, ".."); // One directory per decision domain, and the domain decides which normative -// root a pointer must resolve against. `deferred` is parked direction work: it +// roots a pointer may resolve against. `deferred` is parked direction work: it // is not linted, and its pointers name sections spec/v1 deliberately lacks. +// +// An architecture ADR may point at either normative document for code: the +// structure itself, or the ledger of rules that structure is held to. const DOMAINS = [ - { domain: "model", normativeRoot: "spec/v1/" }, - { domain: "architecture", normativeRoot: "docs/architecture.md" }, + { domain: "model", normativeRoots: ["spec/v1/"] }, + { + domain: "architecture", + normativeRoots: ["docs/architecture.md", "docs/architecture-rules.md"], + }, ] as const; const SECTIONS = [ @@ -92,12 +98,12 @@ export function lintAdrs(root: string): AdrLintResult { for (const stray of listing(adrDir).filter(isAdrName)) err(stray, "ADR outside a domain directory"); - const files = DOMAINS.flatMap(({ domain, normativeRoot }) => + const files = DOMAINS.flatMap(({ domain, normativeRoots }) => listing(join(adrDir, domain)) .filter(isAdrName) .map((name) => ({ domain, - normativeRoot, + normativeRoots, name, rel: posix.join(domain, name), })), @@ -139,7 +145,7 @@ export function lintAdrs(root: string): AdrLintResult { const premises = new Set(); const decisions: { readonly rel: string; readonly restsOn: string[] }[] = []; - for (const { domain, normativeRoot, name, rel } of files) { + for (const { domain, normativeRoots, name, rel } of files) { const text = readFileSync(join(adrDir, domain, name), "utf8"); const block = /^---\n([\s\S]*?)\n---\n/.exec(text); if (!block) { @@ -242,8 +248,11 @@ export function lintAdrs(root: string): AdrLintResult { // -- the normative target and its anchor exist, under this domain's root if (normative) { const [target = "", anchor] = normative.split("#"); - if (!target.startsWith(normativeRoot)) - err(rel, `normative target '${target}' is outside '${normativeRoot}'`); + if (!normativeRoots.some((root) => target.startsWith(root))) + err( + rel, + `normative target '${target}' is outside '${normativeRoots.join(", ")}'`, + ); const anchors = anchorsOf(join(root, target)); if (anchors === null) err(rel, `normative target '${target}' does not exist`); diff --git a/scripts/lint-requirements.ts b/scripts/lint-requirements.ts index 78377a8..ca60df9 100644 --- a/scripts/lint-requirements.ts +++ b/scripts/lint-requirements.ts @@ -20,11 +20,11 @@ // // A library first, like the other gates: tests call lintRequirements() in // -process, and `node scripts/lint-requirements.ts [root]` is the command. -import { execFileSync } from "node:child_process"; import { readFileSync } from "node:fs"; import { join } from "node:path"; import { isEntrypoint } from "./lib/entrypoint.ts"; import { processOutput, type GateOutput } from "./lib/output.ts"; +import { danglingCitations, trackedText } from "./lib/tracked.ts"; /** One row of the ledger: an id, its guarantee, and the test that proves it. */ export interface RequirementRow { @@ -93,48 +93,15 @@ export function parseRequirements(text: string): { } /** - * Every id cited outside the row that declares it, across the tracked tree, - * that no row carries. Pure over a {rel: content} map, so it is testable - * against a synthetic tree the way emdash.test.ts's offendersIn() is. + * Every REQ id cited outside the row that declares it, across the tracked + * tree, that no row carries. Pure over a {rel: content} map, so it is + * testable against a synthetic tree the way emdash.test.ts's offendersIn() is. */ export function citationErrors( files: Readonly>, ids: ReadonlySet, ): string[] { - const errors: string[] = []; - const reported = new Set(); - // Object keys are unique paths, so two entries are never equal: a strict - // less-than is enough to order them, with no equal case to fall through to. - const entries = Object.entries(files).sort(([a], [b]) => (a < b ? -1 : 1)); - for (const [rel, content] of entries) { - for (const match of content.matchAll(CITATION)) { - const id = match[0]; - const key = `${rel}\0${id}`; - if (ids.has(id) || reported.has(key)) continue; - reported.add(key); - errors.push(`${rel} cites ${id}, which no row carries`); - } - } - return errors; -} - -/** Every tracked file's text, keyed by its path relative to `root`. */ -function trackedText(root: string): Record { - const files: Record = {}; - const tracked = execFileSync("git", ["ls-files", "-z"], { - cwd: root, - encoding: "utf8", - }).split("\0"); - for (const rel of tracked) { - if (rel === "") continue; - try { - files[rel] = readFileSync(join(root, rel), "utf8"); - } catch { - // A path that cannot be read as text (a symlink to nowhere, a - // directory entry) carries no citation either way. - } - } - return files; + return danglingCitations(files, ids, CITATION); } /** Lint the behaviour ledger under `root`. */ diff --git a/scripts/lint-rules.ts b/scripts/lint-rules.ts new file mode 100644 index 0000000..fb13077 --- /dev/null +++ b/scripts/lint-rules.ts @@ -0,0 +1,466 @@ +// The rule ledger, docs/architecture-rules.md: what keeps it honest. +// +// docs/architecture-rules.md holds one row per rule the compiler and this +// repository are held to: a greppable id, its family, the sentence, the +// enforcer that runs it, and the fixture that proves it fires. This is what +// notices when a row and the tree stop agreeing. +// +// The checks, in the order a reader meets them: +// +// - every row parses, its id is unique and shaped RULE-NNN, and its family +// is one the document's own Families table declares (and every declared +// family is used, so the taxonomy cannot grow dead entries); +// - an enforced row's enforcer resolves: a dependency-cruiser rule name +// that .dependency-cruiser.cjs configures, an ESLint rule id that +// eslint.config.js configures, an npm script package.json defines, or a +// file that exists; +// - an enforced row names a fixture: a real, non-empty test file that +// mentions the witness (the literal the fixture asserts on), so a row +// whose fixture was deleted or rewritten past its rule fails here; +// - a pending row carries a ticket and a reason, and names no fixture, so +// "pending" can never read as proven; +// - the converse: every dependency-cruiser rule and every ESLint rule this +// repository configures by name is claimed by exactly one enforced row. +// A rule enforced with no row fails, and so does a rule quietly moved to +// pending while its configuration still runs. That is what stops pending +// becoming where rules go to be forgotten; +// - the stated totals (rows, and how many are pending) match what the +// document holds, and every RULE-NNN cited in the tracked tree resolves. +// +// This is not docs/requirements.md. That ledger lists the behaviours a person +// depends on, keyed to the test that fails when one stops being true; this +// one lists the rules a tool enforces, keyed to the enforcer that runs them. +// A behaviour can rest on several rules and a rule can serve several +// behaviours, so the two ledgers stay separate and their id spaces do not +// overlap. +// +// A library first, like the other gates: tests call lintRules() in-process, +// and `node scripts/lint-rules.ts [root]` is the command. +import { existsSync, readFileSync } from "node:fs"; +import { join } from "node:path"; +import { isEntrypoint } from "./lib/entrypoint.ts"; +import { processOutput, type GateOutput } from "./lib/output.ts"; +import { danglingCitations, trackedText } from "./lib/tracked.ts"; + +/** One row of the ledger. */ +export interface RuleRow { + readonly id: string; + readonly family: string; + readonly sentence: string; + readonly enforcement: Enforcement; + /** The fixture's path, relative to the repository root, or null when pending. */ + readonly fixture: string | null; + /** The literal the fixture asserts on, or null when pending. */ + readonly witness: string | null; +} + +/** What one run found: the rows it read, how many are pending, and every violation. */ +export interface RulesLintResult { + readonly rows: number; + readonly pending: number; + readonly errors: readonly string[]; +} + +const REPOSITORY = join(import.meta.dirname, ".."); +const LEDGER = join("docs", "architecture-rules.md"); + +const CITATION = /\bRULE-\d{3}\b/g; +const STATED_ROWS = /this ledger holds \*\*(\d+)\*\* rules?\b/i; +const STATED_PENDING = /\*\*(\d+)\*\* of them pending\b/i; + +/** The enforcer kinds a row may name, each resolvable against the tree. */ +const KINDS = ["depcruise", "eslint", "npm", "file"] as const; +type Kind = (typeof KINDS)[number]; + +const isKind = (candidate: string): candidate is Kind => + (KINDS as readonly string[]).includes(candidate); + +/** How a row says it is enforced: by a named enforcer, or not yet at all. */ +export type Enforcement = + | { readonly kind: Kind; readonly value: string } + | { + readonly kind: "pending"; + readonly ticket: string; + readonly reason: string; + }; + +/** The shortest reason that says anything: shorter than this is a shrug. */ +const MIN_REASON = 20; + +// A row is five cells: the id, its family, the rule, its enforcer, its proof. +const ROW = + /^\|\s*(RULE-\d{3})\s*\|\s*([a-z]+)\s*\|\s*(.+?)\s*\|\s*(.+?)\s*\|\s*(.+?)\s*\|$/; +// `kind:value`, backticked so the document renders it as the token it is. +const ENFORCER = /^`([a-z]+):(.+)`$/; +// pending (#29): a reason, or pending (n/a): a reason, for a rule no ticket +// will bring because it waits on something outside this repository's plan. +const PENDING = /^pending \((#\d+|n\/a)\):\s*(.*)$/; +// A link to the fixture, then the literal that fixture asserts on. +const PROOF = /^\[([^\]]+)\]\(([^)]+)\)\s+`([^`]+)`$/; +// A Families table row: `| family | what it covers |`. +const FAMILY = /^\|\s*([a-z]+)\s*\|\s*(.+?)\s*\|$/; + +/** Every line that opens like a ledger row, matched or not. */ +function candidateLines(text: string): string[] { + return text.split("\n").filter((line) => /^\|\s*RULE-\d{3}\s*\|/.test(line)); +} + +/** The families the document's own Families table declares, in order. */ +export function parseFamilies(text: string): string[] { + const section = text.split("\n## Families\n")[1]?.split("\n## ")[0] ?? ""; + const families: string[] = []; + for (const line of section.split("\n")) { + const match = FAMILY.exec(line); + const family = match?.[1]; + if (family === undefined || family === "family") continue; + families.push(family); + } + return families; +} + +/** Read one row's enforcer cell, or null when it is neither form. */ +function parseEnforcement(cell: string): Enforcement | null { + const enforcer = ENFORCER.exec(cell); + const kind = enforcer?.[1]; + const value = enforcer?.[2]; + if (kind !== undefined && value !== undefined && isKind(kind)) + return { kind, value }; + const pending = PENDING.exec(cell); + if (pending?.[1] !== undefined) + return { kind: "pending", ticket: pending[1], reason: pending[2] ?? "" }; + return null; +} + +/** + * Parse the ledger body into rows. Pure and synchronous, so a malformed row + * is testable against a string with no filesystem involved. A line that opens + * like a row but does not match it is reported and dropped, not guessed at. + */ +export function parseRules(text: string): { + readonly rows: readonly RuleRow[]; + readonly errors: readonly string[]; +} { + const rows: RuleRow[] = []; + const errors: string[] = []; + for (const line of candidateLines(text)) { + const match = ROW.exec(line); + const id = match?.[1]; + const family = match?.[2]; + const sentence = match?.[3]; + const enforcerCell = match?.[4]; + const proofCell = match?.[5]; + if ( + id === undefined || + family === undefined || + sentence === undefined || + enforcerCell === undefined || + proofCell === undefined + ) { + errors.push(`malformed row: ${line}`); + continue; + } + + const enforcement = parseEnforcement(enforcerCell); + if (enforcement === null) { + errors.push( + `${id}: enforcer '${enforcerCell}' is neither a backticked ` + + `kind:value nor a 'pending (#NNN): reason' entry`, + ); + continue; + } + + const proof = PROOF.exec(proofCell); + const linkText = proof?.[1]; + const href = proof?.[2]; + const witness = proof?.[3]; + if (enforcement.kind === "pending") { + if (proofCell !== "pending") + errors.push( + `${id}: is pending, so its proof cell must read 'pending', not '${proofCell}'`, + ); + rows.push({ + id, + family, + sentence, + enforcement, + fixture: null, + witness: null, + }); + continue; + } + if (linkText === undefined || href === undefined || witness === undefined) { + errors.push( + `${id}: is enforced, so its proof cell must be a linked fixture and a ` + + `backticked witness, not '${proofCell}'`, + ); + continue; + } + if (linkText !== href.replace(/^\.\.\//, "")) + errors.push( + `${id}: link text '${linkText}' does not match its target '${href}'`, + ); + rows.push({ + id, + family, + sentence, + enforcement, + fixture: linkText, + witness, + }); + } + return { rows, errors }; +} + +/** The rule names `.dependency-cruiser.cjs` configures under `forbidden`. */ +export function dependencyCruiserRules(text: string): string[] { + // `\bname:` and not `name:`, so the `fileName:` of the tsConfig option is + // not read as a rule: there is no word boundary inside `fileName`. + return [...text.matchAll(/\bname:\s*"([^"]+)"/g)].map( + (match) => match[1] as string, + ); +} + +/** + * The rule ids `eslint.config.js` configures by name and leaves on. A quoted + * key whose value is an options array or a severity is a rule; `"off"` is a + * rule this repository deliberately does not run, and a quoted key with any + * other value (`"ts-expect-error": "allow-with-description"`) is an option of + * one, not a rule of its own. + */ +export function eslintRules(text: string): string[] { + const rules: string[] = []; + for (const match of text.matchAll( + /(?:^|[\s{,])"([^"]+)":\s*(\[|"error"|"warn"|"off")/gm, + )) { + if (match[2] === '"off"') continue; + rules.push(match[1] as string); + } + return rules; +} + +/** The npm scripts `package.json` defines, or none when it cannot be read. */ +function packageScripts(root: string): Set { + try { + const pkg = JSON.parse( + readFileSync(join(root, "package.json"), "utf8"), + ) as { scripts?: Record }; + return new Set(Object.keys(pkg.scripts ?? {})); + } catch { + return new Set(); + } +} + +/** The text of `rel` under `root`, or null when there is no such file. */ +function read(root: string, rel: string): string | null { + try { + return readFileSync(join(root, rel), "utf8"); + } catch { + return null; + } +} + +/** Check one enforced row's enforcer against what the tree configures. */ +function enforcerErrors( + row: RuleRow, + configured: { + readonly depcruise: ReadonlySet; + readonly eslint: ReadonlySet; + readonly npm: ReadonlySet; + }, + root: string, +): string[] { + const enforcement = row.enforcement; + if (enforcement.kind === "pending") return []; + const value = enforcement.value; + switch (enforcement.kind) { + case "depcruise": + return configured.depcruise.has(value) + ? [] + : [ + `${row.id}: names dependency-cruiser rule '${value}', which ` + + `.dependency-cruiser.cjs does not configure`, + ]; + case "eslint": + return configured.eslint.has(value) + ? [] + : [ + `${row.id}: names ESLint rule '${value}', which eslint.config.js ` + + `does not configure`, + ]; + case "npm": + return configured.npm.has(value) + ? [] + : [ + `${row.id}: names npm script '${value}', which package.json does ` + + `not define`, + ]; + default: + return existsSync(join(root, value)) + ? [] + : [`${row.id}: names file '${value}', which does not exist`]; + } +} + +/** Check one row's fixture: it exists, holds a test, and mentions the witness. */ +function fixtureErrors(row: RuleRow, root: string): string[] { + const { fixture, witness } = row; + if (fixture === null || witness === null) return []; + const content = read(root, fixture); + if (content === null) + return [`${row.id}: names fixture '${fixture}', which does not exist`]; + const errors: string[] = []; + if (!fixture.endsWith(".test.ts")) + errors.push(`${row.id}: fixture '${fixture}' is not a test file`); + if (!/\b(?:test|it)\s*\(/.test(content)) + errors.push(`${row.id}: fixture '${fixture}' holds no test`); + if (!content.includes(witness)) + errors.push( + `${row.id}: fixture '${fixture}' never mentions '${witness}', so ` + + `nothing in it proves this rule fires`, + ); + return errors; +} + +/** + * The converse check: a configuration this repository runs today, against the + * rows that claim it. A configured rule no enforced row claims is either a + * rule nobody wrote down or a row that has quietly been moved to pending; + * both fail here, which is what keeps the pending list from becoming a place + * rules go to be forgotten. + */ +function converseErrors( + configured: readonly string[], + rows: readonly RuleRow[], + kind: "depcruise" | "eslint", + source: string, +): string[] { + const errors: string[] = []; + for (const name of configured) { + const claimed = rows.filter( + (row) => row.enforcement.kind === kind && row.enforcement.value === name, + ); + if (claimed.length === 0) + errors.push( + `${source}: rule '${name}' is enforced, and no ledger row claims it; ` + + `a row that calls it pending does not count`, + ); + else if (claimed.length > 1) + errors.push( + `${source}: rule '${name}' is claimed by ${String(claimed.length)} ` + + `rows (${claimed.map((row) => row.id).join(", ")}); one enforcer, one row`, + ); + } + return errors; +} + +/** Lint the rule ledger under `root`. */ +export function lintRules(root: string): RulesLintResult { + const text = read(root, LEDGER); + if (text === null) + return { rows: 0, pending: 0, errors: [`${LEDGER}: ledger missing`] }; + + const { rows, errors: parseErrors } = parseRules(text); + const errors = [...parseErrors]; + const pending = rows.filter( + (row) => row.enforcement.kind === "pending", + ).length; + + // -- families: a row names a declared one, and every declared one is used + const families = parseFamilies(text); + if (families.length === 0) errors.push(`${LEDGER}: no Families table`); + const declared = new Set(families); + const used = new Set(rows.map((row) => row.family)); + for (const row of rows) + if (!declared.has(row.family)) + errors.push( + `${row.id}: family '${row.family}' is not declared in the Families table`, + ); + for (const family of families) + if (!used.has(family)) + errors.push( + `${LEDGER}: family '${family}' is declared and no rule uses it`, + ); + + // -- ids are unique + const byId = new Map(); + for (const row of rows) { + const seen = byId.get(row.id); + if (seen) + errors.push(`${row.id}: id used twice, also for '${seen.sentence}'`); + else byId.set(row.id, row); + } + + const depcruiseText = read(root, ".dependency-cruiser.cjs"); + const eslintText = read(root, "eslint.config.js"); + if (depcruiseText === null) + errors.push(`${LEDGER}: .dependency-cruiser.cjs is missing`); + if (eslintText === null) + errors.push(`${LEDGER}: eslint.config.js is missing`); + const depcruise = dependencyCruiserRules(depcruiseText ?? ""); + const eslint = eslintRules(eslintText ?? ""); + const configured = { + depcruise: new Set(depcruise), + eslint: new Set(eslint), + npm: packageScripts(root), + }; + + for (const row of rows) { + errors.push(...enforcerErrors(row, configured, root)); + errors.push(...fixtureErrors(row, root)); + if (row.enforcement.kind === "pending") { + if (row.enforcement.reason.length < MIN_REASON) + errors.push( + `${row.id}: is pending with no reason; a pending row says why it is ` + + `not enforced yet`, + ); + } + } + + errors.push( + ...converseErrors(depcruise, rows, "depcruise", ".dependency-cruiser.cjs"), + ); + errors.push(...converseErrors(eslint, rows, "eslint", "eslint.config.js")); + + // -- the stated totals match what the document holds + const statedRows = STATED_ROWS.exec(text)?.[1]; + if (statedRows === undefined) + errors.push(`${LEDGER}: does not state how many rules it holds`); + else if (Number(statedRows) !== rows.length) + errors.push( + `${LEDGER}: states ${statedRows} rules, holds ${String(rows.length)}`, + ); + + const statedPending = STATED_PENDING.exec(text)?.[1]; + if (statedPending === undefined) + errors.push(`${LEDGER}: does not state how many rules are pending`); + else if (Number(statedPending) !== pending) + errors.push( + `${LEDGER}: states ${statedPending} pending, holds ${String(pending)}`, + ); + + errors.push( + ...danglingCitations(trackedText(root), new Set(byId.keys()), CITATION), + ); + + return { rows: rows.length, pending, errors }; +} + +/** Lint the tree named by argv[0], or this repository, and say what was found. */ +export function main( + argv: readonly string[], + output: GateOutput = processOutput, +): number { + const { rows, pending, errors } = lintRules(argv[0] ?? REPOSITORY); + if (errors.length > 0) { + output.err( + `rules lint: ${errors.length} error(s)\n` + + errors.map((e) => ` - ${e}\n`).join(""), + ); + return 1; + } + output.out(`rules lint: ${rows} rules clean, ${pending} pending\n`); + return 0; +} + +if (isEntrypoint(import.meta.url, process.argv[1])) + process.exitCode = main(process.argv.slice(2)); diff --git a/test/adr-lint-negative.test.ts b/test/adr-lint-negative.test.ts index bf126de..6cf9c41 100644 --- a/test/adr-lint-negative.test.ts +++ b/test/adr-lint-negative.test.ts @@ -92,6 +92,13 @@ function tree( join(root, "docs", "architecture.md"), "# Architecture\n\n## Layers\n\nText.\n", ); + // The second normative document an architecture ADR may point at. It + // deliberately carries only one heading, so a pointer at a section it lacks + // is the case the anchor check has to catch. + writeFileSync( + join(root, "docs", "architecture-rules.md"), + "# Rule ledger\n\n## Rules\n\nText.\n", + ); const all: Record = { ...files }; if (premise) @@ -416,6 +423,27 @@ describe("domain directories", () => { ).toMatch(/anchor '#no-such-heading' not found/); }); + it("passes an architecture ADR anchoring into the rule ledger", () => { + expect( + violations({ + [DECISION]: validAdr(), + "architecture/0064-a-code-decision.md": validAdr({ + normative: "docs/architecture-rules.md#rules", + }), + }), + ).toBe(""); + }); + + it("fails an architecture ADR naming a rule ledger section that is not there", () => { + expect( + violations({ + "architecture/0064-a-code-decision.md": validAdr({ + normative: "docs/architecture-rules.md#families", + }), + }), + ).toMatch(/anchor '#families' not found in docs\/architecture-rules\.md/); + }); + it("fails a model ADR pointing outside spec/v1", () => { expect( violations({ diff --git a/test/boundary-contract.test.ts b/test/boundary-contract.test.ts index c23ea5a..e4ddbbc 100644 --- a/test/boundary-contract.test.ts +++ b/test/boundary-contract.test.ts @@ -255,6 +255,24 @@ describe("the boundary lint", () => { expect(output).toMatch(/no-dev-dependency-in-src/); }); + it("fails a package an architecture decision already rejected", () => { + const { code, output } = cruise({ + "src/infrastructure/render.ts": mod(["handlebars"]), + "src/cli/index.ts": mod(["../infrastructure/render.js"]), + }); + expect(code).not.toBe(0); + expect(output).toMatch(/no-denied-dependency/); + }); + + it("fails an import that resolves to nothing", () => { + const { code, output } = cruise({ + "src/infrastructure/writer.ts": mod(["./gone.js"]), + "src/cli/index.ts": mod(["../infrastructure/writer.js"]), + }); + expect(code).not.toBe(0); + expect(output).toMatch(/no-unresolvable-import/); + }); + it("fails a deprecated node builtin", () => { const { code, output } = cruise({ "src/infrastructure/idna.ts": mod(["punycode"]), diff --git a/test/eslint-rules.test.ts b/test/eslint-rules.test.ts new file mode 100644 index 0000000..7089671 --- /dev/null +++ b/test/eslint-rules.test.ts @@ -0,0 +1,93 @@ +// The lint rules that apply to every file, executed. +// +// test/harness.test.ts proves the rules this repository adds for test files. +// These are the other half: the four rules eslint.config.js names for every +// TypeScript file, and the two presets it turns on, which between them carry +// far more rules than anyone will ever name by hand. A rule that has only +// ever run over clean code is untested: nothing proves it would fail. +// +// Each of these is a row in docs/architecture-rules.md, and +// scripts/lint-rules.ts checks that this file still mentions the rule id each +// row cites, so deleting a case below fails the rule ledger rather than +// quietly leaving a row unproven. +import { join } from "node:path"; +import { ESLint } from "eslint"; +import { describe, expect, it } from "vitest"; + +const REPOSITORY = join(import.meta.dirname, ".."); + +describe("the lint rules for every file", { timeout: 120_000 }, () => { + // Linted from memory at a path under scripts/, so the project-wide rules + // apply and the test-file overrides do not. It is not on disk, so the + // project service would refuse it; admitting that one path to the default + // project is the only thing this changes about the repository's own config. + const PROBE = "scripts/probe.ts"; + const eslint = new ESLint({ + cwd: REPOSITORY, + overrideConfig: { + languageOptions: { + parserOptions: { + projectService: { allowDefaultProject: ["*.js", "*.cjs", PROBE] }, + tsconfigRootDir: REPOSITORY, + }, + }, + }, + }); + + async function rulesFiredOn(source: string): Promise<(string | null)[]> { + const [result] = await eslint.lintText(source, { + filePath: join(REPOSITORY, PROBE), + }); + return (result?.messages ?? []).map((message) => message.ruleId); + } + + it("reject a suppression that silences a whole file", async () => { + const source = "// @ts-nocheck\nexport const v = 1;\n"; + expect(await rulesFiredOn(source)).toContain( + "@typescript-eslint/ban-ts-comment", + ); + }); + + it("reject an explicit any", async () => { + const source = "export const v = (x: any): number => Number(x);\n"; + expect(await rulesFiredOn(source)).toContain( + "@typescript-eslint/no-explicit-any", + ); + }); + + it("reject an unused binding that does not say so with an underscore", async () => { + const source = "const unused = 1;\nexport const v = 2;\n"; + expect(await rulesFiredOn(source)).toContain( + "@typescript-eslint/no-unused-vars", + ); + }); + + it("reject an object interpolated into a template literal", async () => { + const source = + "const o = { a: 1 };\nexport const v = `value: ${JSON.parse('{}') as object}` + String(o);\n"; + expect(await rulesFiredOn(source)).toContain( + "@typescript-eslint/restrict-template-expressions", + ); + }); + + it("reject a floating promise, which no rule here names and the preset carries", async () => { + const source = + "const work = async (): Promise => 1;\n" + + "export const v = (): void => {\n work();\n};\n"; + expect(await rulesFiredOn(source)).toContain( + "@typescript-eslint/no-floating-promises", + ); + }); + + it("reject an empty catch, which the recommended preset carries", async () => { + const source = + "export const v = (): void => {\n" + + " try {\n JSON.parse('1');\n } catch {}\n};\n"; + expect(await rulesFiredOn(source)).toContain("no-empty"); + }); + + it("accept a file that breaks none of them, so they are not simply forbidding everything", async () => { + const source = "export const v = (n: number): string => `value: ${n}`;\n"; + expect(await rulesFiredOn(source)).toStrictEqual([]); + }); +}); diff --git a/test/rules-contract.test.ts b/test/rules-contract.test.ts new file mode 100644 index 0000000..ae3fa45 --- /dev/null +++ b/test/rules-contract.test.ts @@ -0,0 +1,181 @@ +// The rule ledger, executed. docs/architecture-rules.md states which rules +// this repository and the compiler are held to, scripts/lint-rules.ts enforces +// that every row still resolves, and this is what makes that enforcement part +// of the test run rather than a thing someone remembers to run. It also starts +// the lint the way CI would, as a command, which is what proves the guard at +// the bottom of the script still runs it. +// +// REQ-014 (docs/requirements.md): every rule this repository enforces has a +// ledger row with a greppable id and a fixture that proves it fires. +import { execFileSync, spawnSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it, test } from "vitest"; +import { + dependencyCruiserRules, + eslintRules, + lintRules, + main, + parseFamilies, + parseRules, +} from "../scripts/lint-rules.ts"; +import { collect } from "./support/collect.ts"; +import { temporary } from "./setup.ts"; + +const REPOSITORY = join(import.meta.dirname, ".."); +const LEDGER = join(REPOSITORY, "docs", "architecture-rules.md"); +const ledgerText = (): string => readFileSync(LEDGER, "utf8"); + +test("the rules lint, run as a command, passes over the committed ledger", () => { + const out = execFileSync( + process.execPath, + [join(REPOSITORY, "scripts", "lint-rules.ts")], + { encoding: "utf8" }, + ); + expect(out).toMatch(/^rules lint: \d+ rules clean, \d+ pending$/m); +}); + +test("the ledger is non-empty, and its stated counts match what it holds", () => { + const { rows } = parseRules(ledgerText()); + expect(rows.length).toBeGreaterThanOrEqual(20); + const pending = rows.filter( + (row) => row.enforcement.kind === "pending", + ).length; + expect(lintRules(REPOSITORY)).toStrictEqual({ + rows: rows.length, + pending, + errors: [], + }); +}); + +test("every row's id is unique and shaped RULE-NNN", () => { + const ids = parseRules(ledgerText()).rows.map((row) => row.id); + for (const id of ids) expect(id).toMatch(/^RULE-\d{3}$/); + expect(new Set(ids).size).toBe(ids.length); +}); + +test("every family the taxonomy declares carries at least one rule", () => { + const { rows } = parseRules(ledgerText()); + const families = parseFamilies(ledgerText()); + expect(families.length).toBeGreaterThan(5); + const used = new Set(rows.map((row) => row.family)); + for (const family of families) expect(used, family).toContain(family); +}); + +test("every rule the graph ruleset configures is claimed by exactly one enforced row", () => { + const configured = dependencyCruiserRules( + readFileSync(join(REPOSITORY, ".dependency-cruiser.cjs"), "utf8"), + ); + expect(configured.length).toBeGreaterThan(10); + const { rows } = parseRules(ledgerText()); + for (const name of configured) { + const claimed = rows.filter( + (row) => + row.enforcement.kind === "depcruise" && row.enforcement.value === name, + ); + expect(claimed, name).toHaveLength(1); + } +}); + +test("every ESLint rule this repository names is claimed by exactly one enforced row", () => { + const configured = eslintRules( + readFileSync(join(REPOSITORY, "eslint.config.js"), "utf8"), + ); + expect(configured.length).toBeGreaterThan(5); + const { rows } = parseRules(ledgerText()); + for (const id of configured) { + const claimed = rows.filter( + (row) => + row.enforcement.kind === "eslint" && row.enforcement.value === id, + ); + expect(claimed, id).toHaveLength(1); + } +}); + +test("every enforced row's fixture is a real test that asserts on its witness", () => { + const { rows } = parseRules(ledgerText()); + const enforced = rows.filter((row) => row.fixture !== null); + expect(enforced.length).toBeGreaterThanOrEqual(20); + for (const row of enforced) { + const content = readFileSync(join(REPOSITORY, row.fixture ?? ""), "utf8"); + expect(row.fixture, row.id).toMatch(/\.test\.ts$/); + expect(content, row.id).toMatch(/\b(?:test|it)\s*\(/); + expect(content, `${row.id} witness`).toContain(row.witness ?? ""); + } +}); + +test("every pending row names a ticket and a reason, and claims no fixture", () => { + const { rows } = parseRules(ledgerText()); + const pending = rows.filter((row) => row.enforcement.kind === "pending"); + expect(pending.length).toBeGreaterThan(0); + for (const row of pending) { + const { enforcement } = row; + if (enforcement.kind !== "pending") throw new Error(`${row.id}: enforced`); + expect(enforcement.ticket, row.id).toMatch(/^(#\d+|n\/a)$/); + expect(enforcement.reason.length, row.id).toBeGreaterThanOrEqual(20); + expect(row.fixture, row.id).toBeNull(); + } +}); + +describe("the command", () => { + const LEDGER_BODY = + "# Rule ledger\n\n" + + "## Families\n\n" + + "| family | covers |\n|---|---|\n| gates | the gates |\n\n" + + "## Rules\n\n" + + "This ledger holds **1** rules, **0** of them pending.\n\n" + + "| id | family | the rule | enforced by | proved by |\n|---|---|---|---|---|\n" + + "| RULE-001 | gates | a rule holds | `file:test/a.test.ts` | " + + "[test/a.test.ts](../test/a.test.ts) `a witness` |\n"; + + /** A tree the lint can read: a ledger, a fixture, and empty configurations. */ + function fixture(body: string): string { + const root = mkdtempSync(join(temporary(), "rules-command-")); + mkdirSync(join(root, "docs"), { recursive: true }); + mkdirSync(join(root, "test"), { recursive: true }); + writeFileSync(join(root, "docs", "architecture-rules.md"), body); + writeFileSync( + join(root, "test", "a.test.ts"), + 'import { expect, it } from "vitest";\n' + + 'it("a witness", () => {\n expect(1).toBe(1);\n});\n', + ); + writeFileSync( + join(root, ".dependency-cruiser.cjs"), + "module.exports = {};", + ); + writeFileSync(join(root, "eslint.config.js"), "export default [];"); + writeFileSync(join(root, "package.json"), '{"scripts":{}}'); + execFileSync("git", ["init", "-q"], { cwd: root }); + execFileSync("git", ["add", "-A"], { cwd: root }); + return root; + } + + it("says how many rules are clean and how many are pending, and exits 0", () => { + const output = collect(); + expect(main([fixture(LEDGER_BODY)], output)).toBe(0); + expect(output.text()).toBe("rules lint: 1 rules clean, 0 pending\n"); + }); + + it("lists every violation under a count, and exits 1", () => { + const output = collect(); + const root = fixture("# Rule ledger\n\n## Families\n\n| a | b |\n"); + expect(main([root], output)).toBe(1); + expect(output.text()).toMatch(/^rules lint: \d+ error\(s\)\n/); + expect(output.text()).toContain("does not state how many rules it holds"); + }); + + it("checks this repository when no tree is named", () => { + expect(main([], collect())).toBe(0); + }); + + it("runs when Node starts the script, which is how CI would run it", () => { + const root = fixture("# Rule ledger\n\nno rules here\n"); + const run = spawnSync( + process.execPath, + [join(REPOSITORY, "scripts", "lint-rules.ts"), root], + { encoding: "utf8" }, + ); + expect(run.status).toBe(1); + expect(run.stderr).toContain("does not state how many rules it holds"); + }); +}); diff --git a/test/rules-lint-negative.test.ts b/test/rules-lint-negative.test.ts new file mode 100644 index 0000000..9e0b348 --- /dev/null +++ b/test/rules-lint-negative.test.ts @@ -0,0 +1,471 @@ +// Negative fixtures for the rule ledger. +// +// A lint that has only ever run against a clean ledger is untested: nothing +// proves it would fail. Each case builds a throwaway tree that violates +// exactly one rule and asserts the lint reports it, naming the row. +import { execFileSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { + dependencyCruiserRules, + eslintRules, + lintRules, + parseFamilies, + parseRules, +} from "../scripts/lint-rules.ts"; +import { temporary } from "./setup.ts"; + +type Files = Readonly>; + +function write(root: string, files: Files): void { + for (const [rel, content] of Object.entries(files)) { + const target = join(root, rel); + mkdirSync(dirname(target), { recursive: true }); + writeFileSync(target, content); + } +} + +/** A fixture that satisfies "exists, is a test file, holds a test, says it". */ +const VALID_FIXTURE = + 'import { expect, it } from "vitest";\n' + + 'it("x", () => {\n expect("a witness").toBe("a witness");\n});\n'; + +/** The configurations the lint reads, with one rule each to claim. */ +const DEFAULTS: Files = { + "test/a.test.ts": VALID_FIXTURE, + ".dependency-cruiser.cjs": "module.exports = { forbidden: [] };\n", + "eslint.config.js": "export default [];\n", + "package.json": '{"scripts": {"lint:x": "true"}}\n', +}; + +/** A git repository holding `files` over the defaults above. */ +function fixture(files: Files): string { + const root = mkdtempSync(join(temporary(), "rules-lint-")); + write(root, { ...DEFAULTS, ...files }); + execFileSync("git", ["init", "-q"], { cwd: root }); + execFileSync("git", ["add", "-A"], { cwd: root }); + return root; +} + +const FAMILIES = "| family | covers |\n|---|---|\n| gates | the gates |"; +const HEADER = + "| id | family | the rule | enforced by | proved by |\n|---|---|---|---|---|"; + +/** A whole ledger document around `rows`, with `counts` as its stated totals. */ +function ledger( + rows: string, + counts = "**1** rules, **0** of them pending", +): string { + return ( + `# Rule ledger\n\n## Families\n\n${FAMILIES}\n\n` + + `## Rules\n\nThis ledger holds ${counts}.\n\n${HEADER}\n${rows}\n` + ); +} + +const PROOF = "[test/a.test.ts](../test/a.test.ts) `a witness`"; +const CLEAN = `| RULE-001 | gates | a rule holds | \`file:test/a.test.ts\` | ${PROOF} |`; + +// Built from two halves, so this file's own source does not carry a citation +// that no row of the real docs/architecture-rules.md resolves. +const BOGUS_ID = ["RULE", "999"].join("-"); + +describe("lintRules", () => { + it("passes a clean, minimal ledger, so it is not simply forbidding everything", () => { + expect( + lintRules(fixture({ "docs/architecture-rules.md": ledger(CLEAN) })), + ).toStrictEqual({ + rows: 1, + pending: 0, + errors: [], + }); + }); + + it("fails, naming the file, when the ledger itself is missing", () => { + expect(lintRules(fixture({})).errors).toStrictEqual([ + "docs/architecture-rules.md: ledger missing", + ]); + }); + + it("fails a row that does not parse as five cells", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + "| RULE-001 | gates | a rule holds |", + ), + }); + expect(lintRules(root).errors).toContain( + "malformed row: | RULE-001 | gates | a rule holds |", + ); + }); + + it("fails an enforcer that is neither a kind:value token nor a pending entry", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + `| RULE-001 | gates | a rule holds | somebody reviews it | ${PROOF} |`, + ), + }); + expect(lintRules(root).errors[0]).toMatch( + /RULE-001: enforcer 'somebody reviews it' is neither/, + ); + }); + + it("fails an enforcer kind the lint cannot resolve against anything", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + `| RULE-001 | gates | a rule holds | \`vibes:strong\` | ${PROOF} |`, + ), + }); + expect(lintRules(root).errors[0]).toMatch(/RULE-001: enforcer/); + }); + + it("fails a family the Families table does not declare", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + `| RULE-001 | vibes | a rule holds | \`file:test/a.test.ts\` | ${PROOF} |`, + ), + }); + expect(lintRules(root).errors).toContain( + "RULE-001: family 'vibes' is not declared in the Families table", + ); + }); + + it("fails a declared family no rule uses, so the taxonomy cannot grow dead entries", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger(CLEAN).replace( + "| gates | the gates |", + "| gates | the gates |\n| vibes | nothing at all |", + ), + }); + expect(lintRules(root).errors).toStrictEqual([ + "docs/architecture-rules.md: family 'vibes' is declared and no rule uses it", + ]); + }); + + it("fails, naming the row, when the fixture it names does not exist", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + "| RULE-001 | gates | a rule holds | `file:test/a.test.ts` | " + + "[test/gone.test.ts](../test/gone.test.ts) `a witness` |", + ), + }); + expect(lintRules(root).errors).toStrictEqual([ + "RULE-001: names fixture 'test/gone.test.ts', which does not exist", + ]); + }); + + it("fails a fixture that never mentions the witness, so nothing there proves the rule fires", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + "| RULE-001 | gates | a rule holds | `file:test/a.test.ts` | " + + "[test/a.test.ts](../test/a.test.ts) `another witness` |", + ), + }); + expect(lintRules(root).errors).toStrictEqual([ + "RULE-001: fixture 'test/a.test.ts' never mentions 'another witness', " + + "so nothing in it proves this rule fires", + ]); + }); + + it("fails a fixture that is not a test file, and one that holds no test", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + "| RULE-001 | gates | a rule holds | `file:test/a.test.ts` | " + + "[test/notes.txt](../test/notes.txt) `a witness` |", + ), + "test/notes.txt": "a witness, in a text file that it( never runs\n", + }); + expect(lintRules(root).errors).toStrictEqual([ + "RULE-001: fixture 'test/notes.txt' is not a test file", + ]); + + const empty = fixture({ + "docs/architecture-rules.md": ledger( + "| RULE-001 | gates | a rule holds | `file:test/a.test.ts` | " + + "[test/empty.test.ts](../test/empty.test.ts) `a witness` |", + ), + "test/empty.test.ts": "// a witness, and no test\n", + }); + expect(lintRules(empty).errors).toStrictEqual([ + "RULE-001: fixture 'test/empty.test.ts' holds no test", + ]); + }); + + it("fails a row whose link text does not match its own target", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + "| RULE-001 | gates | a rule holds | `file:test/a.test.ts` | " + + "[test/wrong.test.ts](../test/a.test.ts) `a witness` |", + ), + }); + expect(lintRules(root).errors).toContain( + "RULE-001: link text 'test/wrong.test.ts' does not match its target '../test/a.test.ts'", + ); + }); + + it("fails an enforced row whose proof cell is not a linked fixture", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + "| RULE-001 | gates | a rule holds | `file:test/a.test.ts` | somewhere |", + ), + }); + expect(lintRules(root).errors[0]).toMatch( + /RULE-001: is enforced, so its proof cell must be a linked fixture/, + ); + }); + + it("fails a file, script, graph rule or ESLint rule the tree does not carry", () => { + const cases: readonly [string, RegExp][] = [ + [ + "`file:test/gone.ts`", + /names file 'test\/gone\.ts', which does not exist/, + ], + [ + "`npm:lint:gone`", + /names npm script 'lint:gone', which package\.json does not define/, + ], + [ + "`depcruise:no-such-rule`", + /names dependency-cruiser rule 'no-such-rule', which \.dependency-cruiser\.cjs does not configure/, + ], + [ + "`eslint:no-such-rule`", + /names ESLint rule 'no-such-rule', which eslint\.config\.js does not configure/, + ], + ]; + for (const [enforcer, expected] of cases) { + const root = fixture({ + "docs/architecture-rules.md": ledger( + `| RULE-001 | gates | a rule holds | ${enforcer} | ${PROOF} |`, + ), + }); + expect(lintRules(root).errors.join("\n"), enforcer).toMatch(expected); + } + }); + + it("fails a pending row that carries no reason", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + "| RULE-001 | gates | a rule holds | pending (#29): soon | pending |", + "**1** rules, **1** of them pending", + ), + }); + expect(lintRules(root).errors).toStrictEqual([ + "RULE-001: is pending with no reason; a pending row says why it is not enforced yet", + ]); + }); + + it("fails a pending row that names no ticket at all", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + "| RULE-001 | gates | a rule holds | pending: the compiler does not exist yet | pending |", + "**1** rules, **1** of them pending", + ), + }); + expect(lintRules(root).errors[0]).toMatch(/RULE-001: enforcer 'pending:/); + }); + + it("fails a pending row that also claims a fixture, so pending never reads as proven", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + "| RULE-001 | gates | a rule holds | pending (#29): the compiler does not exist yet | " + + `${PROOF} |`, + "**1** rules, **1** of them pending", + ), + }); + expect(lintRules(root).errors[0]).toMatch( + /RULE-001: is pending, so its proof cell must read 'pending'/, + ); + }); + + it("fails a configured graph rule that no row claims", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger(CLEAN), + ".dependency-cruiser.cjs": + 'module.exports = { forbidden: [{ name: "no-circular" }] };\n', + }); + expect(lintRules(root).errors).toStrictEqual([ + ".dependency-cruiser.cjs: rule 'no-circular' is enforced, and no ledger " + + "row claims it; a row that calls it pending does not count", + ]); + }); + + it("fails a configured rule a row has quietly moved to pending", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + "| RULE-001 | gates | no import cycle | pending (#29): waiting for the compiler to land | pending |", + "**1** rules, **1** of them pending", + ), + ".dependency-cruiser.cjs": + 'module.exports = { forbidden: [{ name: "no-circular" }] };\n', + }); + expect(lintRules(root).errors).toStrictEqual([ + ".dependency-cruiser.cjs: rule 'no-circular' is enforced, and no ledger " + + "row claims it; a row that calls it pending does not count", + ]); + }); + + it("fails a configured ESLint rule that no row claims, and ignores one turned off", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger(CLEAN), + "eslint.config.js": + 'export default [{ rules: {\n "no-shadow": "error",\n "no-undef": "off",\n} }];\n', + }); + expect(lintRules(root).errors).toStrictEqual([ + "eslint.config.js: rule 'no-shadow' is enforced, and no ledger row " + + "claims it; a row that calls it pending does not count", + ]); + }); + + it("fails one enforcer claimed by two rows", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + `| RULE-001 | gates | a rule holds | \`eslint:no-shadow\` | ${PROOF} |\n` + + `| RULE-002 | gates | another rule holds | \`eslint:no-shadow\` | ${PROOF} |`, + "**2** rules, **0** of them pending", + ), + "eslint.config.js": + 'export default [{ rules: { "no-shadow": "error" } }];\n', + }); + expect(lintRules(root).errors).toStrictEqual([ + "eslint.config.js: rule 'no-shadow' is claimed by 2 rows " + + "(RULE-001, RULE-002); one enforcer, one row", + ]); + }); + + it("fails two rows sharing one id, naming both sentences", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger( + `${CLEAN}\n| RULE-001 | gates | another rule holds | \`file:test/a.test.ts\` | ${PROOF} |`, + "**2** rules, **0** of them pending", + ), + }); + expect(lintRules(root).errors).toStrictEqual([ + "RULE-001: id used twice, also for 'a rule holds'", + ]); + }); + + it("fails when a stated count disagrees with the rows it holds", () => { + const rows = fixture({ + "docs/architecture-rules.md": ledger( + CLEAN, + "**2** rules, **0** of them pending", + ), + }); + expect(lintRules(rows).errors).toStrictEqual([ + "docs/architecture-rules.md: states 2 rules, holds 1", + ]); + + const pending = fixture({ + "docs/architecture-rules.md": ledger( + CLEAN, + "**1** rules, **1** of them pending", + ), + }); + expect(lintRules(pending).errors).toStrictEqual([ + "docs/architecture-rules.md: states 1 pending, holds 0", + ]); + }); + + it("fails a ledger that states neither count, and one with no Families table", () => { + const root = fixture({ + "docs/architecture-rules.md": `# Rule ledger\n\n## Families\n\n${FAMILIES}\n\n## Rules\n\n${HEADER}\n${CLEAN}\n`, + }); + expect(lintRules(root).errors).toStrictEqual([ + "docs/architecture-rules.md: does not state how many rules it holds", + "docs/architecture-rules.md: does not state how many rules are pending", + ]); + + const noFamilies = fixture({ + "docs/architecture-rules.md": + "# Rule ledger\n\n## Rules\n\nThis ledger holds **1** rules, " + + `**0** of them pending.\n\n${HEADER}\n${CLEAN}\n`, + }); + expect(lintRules(noFamilies).errors).toStrictEqual([ + "docs/architecture-rules.md: no Families table", + "RULE-001: family 'gates' is not declared in the Families table", + ]); + }); + + it("fails a tree whose rule configurations are missing altogether", () => { + const root = mkdtempSync(join(temporary(), "rules-bare-")); + write(root, { + "docs/architecture-rules.md": ledger( + CLEAN, + "**0** rules, **0** of them pending", + ), + }); + execFileSync("git", ["init", "-q"], { cwd: root }); + execFileSync("git", ["add", "-A"], { cwd: root }); + const { errors } = lintRules(root); + expect(errors).toContain( + "docs/architecture-rules.md: .dependency-cruiser.cjs is missing", + ); + expect(errors).toContain( + "docs/architecture-rules.md: eslint.config.js is missing", + ); + }); + + it("fails when an id no row carries is cited elsewhere in the tree", () => { + const root = fixture({ + "docs/architecture-rules.md": ledger(CLEAN), + "scripts/note.ts": `// see ${BOGUS_ID} for context\n`, + }); + expect(lintRules(root).errors).toStrictEqual([ + `scripts/note.ts cites ${BOGUS_ID}, which no row carries`, + ]); + }); + + it("ignores an untracked file's citation, the same way the em-dash ban ignores scratch files", () => { + const root = fixture({ "docs/architecture-rules.md": ledger(CLEAN) }); + writeFileSync(join(root, "scratch.md"), `${BOGUS_ID}, never staged\n`); + expect(lintRules(root).errors).toStrictEqual([]); + }); +}); + +describe("parseRules", () => { + it("returns one row per well-formed line, in document order", () => { + const { rows, errors } = parseRules( + `| RULE-002 | gates | second | \`file:a\` | ${PROOF} |\n` + + `| RULE-001 | gates | first | \`file:a\` | ${PROOF} |`, + ); + expect(errors).toStrictEqual([]); + expect(rows.map((row) => row.id)).toStrictEqual(["RULE-002", "RULE-001"]); + }); + + it("ignores prose that merely contains a pipe", () => { + const { rows, errors } = parseRules( + "Some table | with a pipe | but no id\n", + ); + expect(rows).toStrictEqual([]); + expect(errors).toStrictEqual([]); + }); +}); + +describe("reading the configurations", () => { + it("reads a graph ruleset's rule names, and nothing else that is quoted", () => { + expect( + dependencyCruiserRules( + 'module.exports = { forbidden: [\n { name: "no-circular", comment: "x" },\n' + + ' { name: "no-orphans" },\n] };\n', + ), + ).toStrictEqual(["no-circular", "no-orphans"]); + }); + + it("reads an ESLint config's named rules, skipping options and rules turned off", () => { + expect( + eslintRules( + 'export default [{ rules: {\n "a/one": "error",\n "a/two": [\n "error",\n' + + ' { "ts-ignore": true, "ts-expect-error": "allow-with-description" },\n' + + ' ],\n "a/three": "warn",\n "a/four": "off",\n} }];\n', + ), + ).toStrictEqual(["a/one", "a/two", "a/three"]); + }); + + it("reads the families a document declares, skipping its header row", () => { + expect( + parseFamilies( + `# L\n\n## Families\n\n${FAMILIES}\n| tests | the tests |\n\n## Rules\n\n| gone | not a family |\n`, + ), + ).toStrictEqual(["gates", "tests"]); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index f453d74..efdd967 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -22,25 +22,22 @@ export default defineConfig({ // A ratchet, per docs/adr/architecture/0101-coverage-is-a-ratchet.md: // set from what the suite reaches, and only ever raised. // - // Measured 2026-09-14, after the local secret scan - // (scripts/lint-secrets.ts) landed. Its file is small enough that the - // bottom-of-file entrypoint guard every other gate leaves uncovered - // would, left the same way, pull the suite under the previous - // threshold; test/secret-scan-contract.test.ts's "the entrypoint guard" - // covers it in-process instead, the way - // test/package-contents-contract.test.ts already does for - // check-package-contents.ts, and its two `??` fallbacks (the default - // binary name, a signal-killed scan's null exit status) each get a - // fixture of their own. Two runs of one tree, identical both times: - // statements 523/534, branches 274/302, functions 84/84, lines - // 482/493. What is left uncovered elsewhere is mostly the one-line - // command guard at the bottom of each other gate and the branches for a - // tool that cannot be started at all. + // Measured 2026-09-14, after the rule ledger gate + // (scripts/lint-rules.ts) landed on top of the local secret scan + // (scripts/lint-secrets.ts), with the tracked-tree helpers both the + // rule ledger and the requirements gate use moved into + // scripts/lib/tracked.ts. The new gate's negative fixtures reach every + // branch that decides, so all four metrics rose again over the secret + // scan's 97.94 / 90.72 / 100 / 97.76. Two runs of one tree, identical + // both times: statements 701/713, branches 379/410, functions 107/107, + // lines 652/664. What is left uncovered is the one-line command guard + // at the bottom of each other gate and the branches for a tool that + // cannot be started at all. thresholds: { - statements: 97.94, - branches: 90.72, + statements: 98.31, + branches: 92.43, functions: 100, - lines: 97.76, + lines: 98.19, }, }, },