diff --git a/.changeset/docs-resolve-rule-inconsistencies.md b/.changeset/docs-resolve-rule-inconsistencies.md new file mode 100644 index 0000000..2ed8985 --- /dev/null +++ b/.changeset/docs-resolve-rule-inconsistencies.md @@ -0,0 +1,5 @@ +--- +"@deessejs/errors": patch +--- + +Resolve twelve internal inconsistencies across the architecture rules in `docs/engineering/architecture/rules/` (rules 0001-0016, INDEX.md, README.md). Each fix is a clarification that aligns the text with the doctrine already expressed elsewhere in the ruleset; no new doctrine is introduced and no existing constraint is weakened. Includes: INDEX title and 0015 summary refresh, README length policy softening, threshold disambiguation across 0001/0002/0003/0005, 0016 self-contradiction on `run`/`execute`, 0013/0012/0014 reconciliation, and the 0010/0015 carve-out for environment values. The published runtime is unchanged. diff --git a/docs/engineering/architecture/decisions/README.md b/docs/engineering/architecture/decisions/README.md new file mode 100644 index 0000000..68cb3b1 --- /dev/null +++ b/docs/engineering/architecture/decisions/README.md @@ -0,0 +1,52 @@ +# Architecture Decisions + +This folder collects the Architecture Decision Records (ADRs) for the +DeesseJS Errors repository. Each ADR captures one significant +architectural choice, the context that led to it, and the consequences +that followed. + +## Format + +Each decision is stored as a Markdown file with the naming convention +`NNNN-short-slug.md`, where `NNNN` is a monotonically increasing +4-digit sequence. For example: + +- `0001-staging-first-branching-model.md` +- `0002-npm-trusted-publishing.md` + +The sequence numbers are **never reused**, even when an ADR is +superseded — superseded ADRs are linked from the new one but kept +in place for the historical record. + +## Status lifecycle + +Every ADR carries one of the following statuses, set in its frontmatter +and reflected in the title: + +- **Proposed** — under discussion, no commitment yet. +- **Accepted** — adopted by the team; future work must respect it. +- **Superseded** — replaced by a later ADR (cross-link required). +- **Deprecated** — kept on disk for context but no longer applies. + +## When to write an ADR + +Write one whenever a choice: + +- Affects the public API surface (`packages/errors/src/`). +- Changes the release pipeline (`.github/workflows/`, `release.yml`). +- Sets a long-lived convention (branching, commits, dependencies). +- Would surprise a future contributor if it were not written down. + +Do **not** write an ADR for one-off implementation details that live +inside a single PR; the PR description is enough. + +## Authoring + +Use the [`docs/internal/engineering/process/`](../../internal/engineering/process/) +templates if you want a starter, but a minimal ADR only needs: + +1. **Context** — what problem we were solving. +2. **Decision** — what we chose to do. +3. **Consequences** — what becomes easier, what becomes harder. + +Keep it short. The point is to be readable in 5 minutes a year from now. diff --git a/docs/engineering/architecture/rules/0001-project-mindset.md b/docs/engineering/architecture/rules/0001-project-mindset.md new file mode 100644 index 0000000..11c5ffa --- /dev/null +++ b/docs/engineering/architecture/rules/0001-project-mindset.md @@ -0,0 +1,233 @@ +# 0001 — Project Mindset: Excellence by Default + +**Status**: Active (enforced through code review and contributor onboarding). +**Date**: 2026-08-11. + +## Rule + +Every contribution to this repository must be made **as if it were the +last commit before the project reached its largest possible audience**. +The standard is "would I be comfortable explaining this line to a +contributor joining in two years, in front of a million users, with no +opportunity to revise it first?" + +There is no "good enough for now". There is no "we'll fix it later". +The work done today is the work that ships at scale. + +## Why + +A foundation library reaches a long tail of users. Each shortcut +compounds: a single `as any` raté costs a fraction of a second of +author time today, then costs hours of debugging at scale tomorrow, then +becomes the reason a downstream team migrates to a competitor. The +cost asymmetry is brutal in one direction and trivial in the other. + +The same logic applies to **understanding**. A line of code written +without fully grasping its consequences will eventually be the line +that breaks. There is no shortcut around comprehension. Anyone who +finds themselves reaching for one is, by definition, the wrong person +to write that line at that moment. + +The codebase is read far more often than it is written. Every +contribution must optimise for the **reader**, not the author. + +## The ten invariants + +Every contribution must satisfy all of these. They are not +guidelines; they are the floor. + +1. **No shortcuts.** A cast that bypasses the type system is a lie to + the audience. Either the return type is what you say it is, or it + is not, and the code should reflect that truthfully. + +2. **No conscious debt.** "We'll fix it later" is a promise to a + future that may not exist. The only moment we are paid to do + something well is the moment we are doing it. There is no later + that justifies a shortcut now. + +3. **Understand before writing.** If the API being called is not + understood in full, the code is not ready to be written. Reading + the source, asking the maintainer, or waiting for an answer are + all acceptable next steps. Guessing is not. + +4. **No speculative abstractions.** An abstraction added "in case" + is a wall the next contributor will have to climb. Abstract only + when three concrete cases exist (Rule of Three). Until then, the + duplication is cheaper than the abstraction. _The threshold for + extracting an abstraction (three) differs from the threshold for + moving a single file (two distinct concerns); see rule 0003 for + the file-level decision._ + +5. **No `any`.** `unknown` is the safe escape hatch. If a type cannot + be expressed, model it explicitly — through a schema, a discriminated + union, or a generic — rather than closing the eyes. + +6. **No silent failures.** A `try`/`catch` that swallows an error is + a betrayal of the user. Either re-raise, transform with explicit + context, or log through a structured channel. Never silently. + +7. **No compiler bypass.** `@ts-expect-error`, `as` casts, `// @ts-ignore`, + dynamic `require`, and friends are signals that the code has a + problem. Address the problem; do not silence the alarm. + +8. **No dependency without justification.** A new dependency is a + long-term commitment. Before adding it, be able to answer: what is + its license, its release cadence, its bus factor, and why this + one and not its alternatives. If the answer is "it has stars", it + is not ready. + +9. **Optimise for the reader.** The next maintainer is the + audience. If a PR is harder to read than to write, it is the + wrong PR. Comments explain _why_, not _what_. Names carry + meaning; comments carry context that names cannot. + +10. **Excellence is silent.** No commit message that celebrates. No + PR description that congratulates itself. The work is the + artefact. If it needs explanation to be recognised as good, the + work is not good enough. + +## The trust-the-type principle + +The single sentence that operationalises the ten invariants: + +> "If the type says it's not null, trust the type. If the type is +> wrong, fix the type. Don't add runtime null checks for values +> that can't be null." +> +> — Miguel Pizza, _No Defensive Null Checks_, Maintainable +> TypeScript doctrine. + +Every invariant in this rule is a consequence of that principle. +The compiler is the first reviewer (invariant 9); the compiler says +"not null", the runtime says "I trust you" — or, if the compiler +is wrong, the fix is in the type, not in the runtime (rule 0004 +operationalises this). No conscious debt (invariant 2) means no +guard that papers over a type we are afraid to fix. No speculative +abstractions (invariant 4) means no abstract `defensive(...)` +helper that catches everything on the assumption that anything +might happen. + +The principle is the slogan of the project. A reader who +remembers only one sentence from this rule set should remember +this one. + +## Enforcement + +- **Code review** is the primary gate. A reviewer who sees any of the + ten invariants violated is expected to block the PR, regardless of + urgency or seniority of the author. +- **Onboarding** documents must include this rule verbatim. New + contributors who arrive through a fast path (open-source + contribution, AI-assisted PR) are pointed here on their first + interaction with the repo. +- **Self-removal**: contributors who consistently violate the + invariants despite feedback are removed from the maintainer list. + This is not a punishment; it is a recognition that the project and + the contributor have different standards, and the project's + standard is the one that ships. + +## Examples + +Three invariants illustrated as bad/good pairs. The patterns are +generic; they apply to any code that takes the same shape. + +**Invariant 1 (no shortcut) — the lie of a cast:** + +```ts +// Bad: bypasses the type system because the author did not want to +// model the actual shape. +function loadConfig(path: string): Config { + const raw = readFile(path) as any; + return raw as Config; +} + +// Good: the author learned what the file actually contains and +// modelled it. If the file is malformed, the function says so. +function loadConfig(path: string): Config { + const raw = readJson(path); + if (!isConfig(raw)) { + throw new InvalidConfigError(path, raw); + } + return raw; +} +``` + +**Invariant 5 (no `any`) — escape hatches are modelling failures:** + +```ts +// Bad: the author could not express the union, so they shut their eyes. +function handle(event: any) { + if (event.type === 'click') { + /* ... */ + } +} + +// Good: the discriminated union models the truth. The compiler proves +// every branch is handled. +type Event = { type: 'click'; position: Position } | { type: 'key'; key: string }; + +function handle(event: Event) { + switch (event.type) { + case 'click': + return; /* ... */ + case 'key': + return; /* ... */ + } +} +``` + +**Invariant 6 (no silent failures) — the `catch` that lies:** + +```ts +// Bad: the author wrapped the call to be safe and caught "in case". +// Failures vanish. The user never learns. +try { + await sync(); +} catch { + /* nothing */ +} + +// Good: either re-raise with context, transform into a domain error, +// or log through a structured channel. Never silently. +try { + await sync(); +} catch (cause) { + throw new SyncError('sync failed', { cause }); +} +``` + +The remaining invariants are expanded in their dedicated rules: +see rule 0004 for invariants 4 and 7 (no speculative defences, no +compiler bypass) and rule 0008 for the type-side discipline +underlying invariant 7. + +## See also + +- **Rule 0002** — File Separation: the structure this mindset expects. +- **Rule 0003** — File Placement: the discipline that turns the + mindset into a code-shape decision. The Rule of Three named in + invariant 4 above is the _abstraction_ threshold; rule 0003 + applies a _second-concern_ threshold for file relocation — the + two are different decisions with different evidence. +- **Rule 0004** — No Speculative Defences: invariant 4 (no + speculative abstractions) and invariant 7 (no compiler bypass) + in operational form. +- **Rule 0007** — Top-Down Composition: the discipline that puts + the reader first. +- **Rule 0008** — No Chained Type Assertions: the type-side + application of invariant 7. + +## Exceptions + +None. The invariants are absolute. A request for an exception is a +signal that the request should be re-scoped until it no longer +requires one. + +## Sources + +- **Pizza, Miguel.** _No Defensive Null Checks._ Maintainable + TypeScript doctrine. Cited in rule 0001's trust-the-type epigraph + and again in rule 0004 (where the principle is operationalised + for runtime guards). The "trust the type" quote is the project's + slogan; the rule applies the principle at the level of + contributor mindset. diff --git a/docs/engineering/architecture/rules/0002-file-separation.md b/docs/engineering/architecture/rules/0002-file-separation.md new file mode 100644 index 0000000..bfc8bb2 --- /dev/null +++ b/docs/engineering/architecture/rules/0002-file-separation.md @@ -0,0 +1,145 @@ +# 0002 — File Separation: by Concern, not by Syntax Kind + +**Status**: Active (enforced through code review). +**Date**: 2026-08-11. + +## Rule + +Within a single concern (a feature, a module, a domain), code is +separated by **what it does**, not by **what kind of symbol it is**. + +- **Types** for one concern live in `types.ts` of that concern. +- **Constants** for one concern live in `constants.ts` of that + concern. +- **Functions** for one concern live in `index.ts`, `factory.ts`, + `parser.ts`, `formatter.ts`, or whatever verb-named file describes + the operation — not in a generic `utils.ts` or `helpers.ts` that + mixes every helper from every concern. + +Across concerns, types and helpers **must not leak** into a shared +global. There is no `src/types.ts`, no `src/constants.ts` that holds +"the types of the project", no `src/utils/index.ts` that re-exports +every helper in the repo. A file that wants a type or constant from +another concern imports it from that concern's `types.ts` or +`constants.ts` directly. + +## Why + +A type, a constant, and a function are not interchangeable artefacts. +They live different lifecycles: a constant changes rarely and reads +like a table of contents; a type is a contract that constrains every +caller; a function is an operation with inputs, outputs, and side +effects. Mixing them in a single file buries the contract in the +implementation, and forces a reader to skim past implementations to +find the shape of a value. + +The opposite failure is just as bad. A single `types.ts` at the +package root that holds every type in the codebase invites circular +imports, forces a deep dependency graph, and makes it impossible to +extract a sub-concern without surgery. The grain of separation must +match the grain of the domain, not the grain of the language. + +The right cut is per **concern**: a `ValidationError` carries its +own types, its own constants (validation codes, severity levels), +and its own functions. A `User` carries its own types and constants. +They share nothing at the package root, but within each concern the +kinds are separated. + +## What this looks like in practice + +A concern folder that follows the rule looks like: + +``` +validation/ +├── types.ts # interfaces, discriminated unions, type aliases +├── constants.ts # codes, defaults, lookup tables +├── validator.ts # the operation(s) +└── index.ts # public re-exports, if needed +``` + +A concern folder that **violates** the rule looks like one of these: + +- **Single mega file**: `validation/index.ts` contains the type, the + constants, and every function. Hard to skim, hard to refactor. + + ```ts + // validation/index.ts — every concern in one file + export interface ValidationRule { + /* ... */ + } + export const DEFAULT_RULES: ValidationRule[] = [/* ... */]; + export function validate(input: unknown): Result { + /* 200 lines */ + } + export function formatErrors(errors: Error[]): string { + /* ... */ + } + ``` + +- **Syntax-based split**: `types/types.ts` holding every type from + every concern, `constants/constants.ts` holding every constant. + Encourages cross-cutting imports, defeats tree-shaking, signals + "we don't know our own domain boundaries". + + ```ts + // types/types.ts — every type in the project + export interface ValidationRule { + /* ... */ + } + export interface UserProfile { + /* unrelated concern */ + } + export interface InvoiceLine { + /* unrelated concern */ + } + ``` + +## What about generic helpers? + +A helper that is genuinely cross-cutting (date formatting, string +trimming, an `assertNever` guard) belongs in a small, focused module +whose name describes **what the helper does**, not "utils". Two +helpers in the same file is fine if they serve the same purpose. A +catch-all `utils.ts` that grows over time is the symptom this rule +exists to prevent. + +## Enforcement + +- **Code review**. A reviewer who sees a `types.ts` at the package + root that contains types from multiple concerns blocks the PR. +- **Import-graph check**: a CI step (or a manual audit during + release prep) verifies that no module imports across concerns + through a shared barrel that re-exports types from more than one + concern. +- **Refactor signal**: when a `types.ts` or `constants.ts` starts + mixing concerns, splitting it is treated as a same-week cleanup, + not a backlog item. + +## Exceptions + +None at the package root level. Within a single concern, a tiny +helper that lives next to its single caller may stay in the same +file (e.g. an internal helper inside `validator.ts`); this is not a +violation because the file is named for its operation, not for +"helpers". + +## See also + +- **Rule 0003** — File Placement: the decision rule that picks the + home for a file once the concern is identified. This rule says + "no cross-concern `types.ts`"; 0003 says "where does this new file + go before I write it" and consolidates the codebase's three + extraction thresholds (one-caller inline, two-concern move, + three-case abstraction). +- **Rule 0011** — Filenames Are kebab-case: the casing discipline + that makes a folder of separated files read as one project. + +## Sources + +This rule is a synthesis of the project's own working +experience. No external reference anchors it. The shape (one +file per syntactic kind, per concern) is a JavaScript +convention; the project's experience is that the convention +breaks down when cross-concern types accumulate in a shared +`types.ts`. The rule captures the failure mode before it +becomes a smell. diff --git a/docs/engineering/architecture/rules/0003-file-placement.md b/docs/engineering/architecture/rules/0003-file-placement.md new file mode 100644 index 0000000..12c7c08 --- /dev/null +++ b/docs/engineering/architecture/rules/0003-file-placement.md @@ -0,0 +1,206 @@ +# 0003 — File Placement: Decide Before You Create + +**Status**: Active (enforced through code review). +**Date**: 2026-08-11. + +## Rule + +Every new file or directory is created **after** a deliberate decision +about where it belongs. The decision is made before the file is +written, not justified after. + +Concretely: + +1. Before creating the file, the author states (in the PR description, + in the commit body, or in code review) **why** this file lives + here and not somewhere else. "It felt right" is not a justification. +2. If the file holds a single function, the default placement is **next + to its sole caller**, in the caller's concern folder. It is not a + "common utility" until there are at least two callers in different + concerns. +3. If the file is a candidate for "common utils", the author must + demonstrate the **second use site** before extracting. Until then, + the duplication is the cheaper choice. + +## Why + +The instinct to drop a helper into a shared `utils.ts` (or to create a +`utils.ts` to host it) is an act of **premature centralisation**. The +function feels reusable, so we put it where "everyone can find it". +Two months later, the function has one caller, the file is the +graveyard of half-finished ideas, and the next contributor adds their +own helper next to it without reading the first one. Six months later, +the file has seventeen unrelated helpers and no shared concept. + +The cost of misplacing a file grows faster than the cost of a single +duplicate. A duplicate is at worst two lines that say the same thing +in two places; a misplaced file is a wrong contract that the rest of +the codebase imports. + +The discipline of "decide before you create" forces the author to +think about the **lifetime** of the file. A helper next to its caller +has the lifetime of the caller; a helper in `utils.ts` has the +lifetime of "everything". The shorter lifetime is the honest +contract. + +## How to decide + +Ask four questions, in order. Any "no" answers the question of whether +this file belongs at all. + +1. **What concern does it serve?** If the answer is "only one + concern", the file goes in that concern. If "two or more + concerns", continue. +2. **Is the second use site already real?** Not "I imagine using + this elsewhere". A real second use site: a different concern that + already needs this function today. If the second site is + speculative, keep the function near its first caller. +3. **Is the shared concept narrower than "utility"?** "Validation + helpers" is a concept. "String utilities" is not. A concept-narrow + module (`validation/helpers.ts`, `formatting/dates.ts`) ages + better than a generic `utils/`. +4. **Does the name of the new file describe what it does?** A file + named `helpers.ts`, `misc.ts`, `stuff.ts` is almost always wrong. + A file named `date-formatter.ts`, `assert-never.ts`, + `http-status.ts` describes its purpose and ages well. + +### Thresholds at a glance + +The codebase applies three different thresholds to three different +decisions. The thresholds are not interchangeable; each answers a +specific question. + +| Decision | Threshold | Evidence required | +| ------------------------------------------------- | ------------------- | ------------------------------------------------ | +| Keep a helper inline with its single caller | 1 caller | The helper has no second consumer yet. | +| Move a file to a shared location across concerns | 2 distinct concerns | A second concern already needs the file. | +| Introduce a generic abstraction (named algorithm) | 3 concrete cases | The abstraction has paid for itself three times. | + +The first row is the file-level default (rule 0005). The second row +is this rule's question 2. The third row is the _Rule of Three_ +named in rule 0001 (invariant 4). A reader applying the +"second-caller" threshold of this rule to a _generic abstraction_ +is using the wrong number; a reader applying the "three-cases" +threshold to a _file move_ is being over-cautious and accumulating +duplication the codebase has already paid for. + +## When extraction is appropriate + +A function moves to a shared location when: + +- It is called from at least two distinct concerns. +- The concept the function represents is named (not "a thing that + trims strings" but "an RFC 3986 percent-encoder"). +- The signature is stable: no caller has needed to extend it with + optional flags yet. + +A constant moves to a shared location when: + +- It is referenced from at least two concerns. +- It is a value the rest of the codebase would otherwise have to + duplicate or hard-code. + +A type moves to a shared location when: + +- It is used as a contract by two or more concerns. +- The type is small and self-contained (no cross-cutting dependencies). + +## What this looks like in violation + +The smell that this rule exists to catch: + +``` +src/ +├── utils/ +│ ├── index.ts +│ ├── date.ts # only used by report formatter +│ ├── string.ts # only used by error message builder +│ └── number.ts # only used by metrics collector +``` + +Three files, three single callers, one shared parent. The parent +exists because the author thought "these might be useful elsewhere". +They were not useful elsewhere; they never will be. The author +anticipated reuse that did not come. The right shape would have been +to keep each helper inside its sole caller's concern folder. + +```ts +// report/formatter.ts — the helper that should never have moved +import { formatIsoDate } from '../utils/date.js'; + +export function formatReport(event: ReportEvent): string { + // ...uses formatIsoDate exactly once... +} +``` + +The `formatIsoDate` import tells the reader the formatter depends on +a shared utility. But there is no second caller. The "shared" +utility is a single-caller helper dressed up as cross-cutting. The +right shape: + +```ts +// report/formatter.ts — the helper lives with its caller +export function formatReport(event: ReportEvent): string { + const formattedDate = formatIsoDate(event.occurredAt); + // ... +} + +function formatIsoDate(input: Date): string { + return input.toISOString().slice(0, 10); +} +``` + +The reader sees the helper and its caller in the same file. When a +second concern genuinely needs the same formatter, the move to a +shared location is justified by the second use site. + +## Enforcement + +- **Code review**. A reviewer who sees a new file under a + `utils/` or `common/` directory without a justifying comment + and a second use site blocks the PR. +- **File naming**. Files named `utils.ts`, `helpers.ts`, `misc.ts`, + `common.ts`, `stuff.ts` are blocked at review. The author is + asked to name the file after what it does. +- **Quarterly audit**. A standing review of "what lives in the + common directories" is part of release prep. Files that lost + their second use site are moved back to their last surviving + caller's folder. + +## Exceptions + +A genuine cross-cutting helper — for example, an `assertNever` exhaustiveness +guard, or a date format shared by logs, reports, and tests — lives in +a top-level module. The module's name must describe what it does +(`assert-never.ts`, `iso-date.ts`), not what it is (`utils.ts`). The +author must demonstrate the multiple use sites in the PR. + +## See also + +- **Rule 0001** — Project Mindset: invariant 4 names the _Rule of + Three_ (abstraction = three cases) as a sibling threshold. The + "second use site" of this rule is the _file-level_ threshold; the + "three cases" of 0001 is the _abstraction-level_ threshold. The + two coexist by design. +- **Rule 0002** — File Separation: the per-concern split this rule + assumes. This rule says "where does the file go"; 0002 says "what + kinds of files exist in a concern". +- **Rule 0005** — Named Algorithms and Independent Data Structures: + the rule that captures the _one-caller_ default for helpers + (single-caller algorithm stays inline; see the "When this rule + does not apply" section of 0005). +- **Rule 0007** — Top-Down Composition: the discipline that makes + the file the rule places read well from top to bottom. +- **Rule 0011** — Filenames Are kebab-case: the casing discipline + that complements this rule's placement discipline. + +## Sources + +This rule is a synthesis of the project's own working +experience. The discipline of "decide before you create" is +explicitly drawn from Rule of Three — a heuristic named +informally in software folklore that an abstraction is +worth its cost when three concrete cases exist. The rule names +the heuristic without citing a single reference because the +heuristic is older than the JavaScript ecosystem and predates +the project's chosen stack. diff --git a/docs/engineering/architecture/rules/0004-no-speculative-defences.md b/docs/engineering/architecture/rules/0004-no-speculative-defences.md new file mode 100644 index 0000000..8a30a3c --- /dev/null +++ b/docs/engineering/architecture/rules/0004-no-speculative-defences.md @@ -0,0 +1,365 @@ +# 0004 — No Speculative Defences + +**Status**: Active (enforced through code review). +**Date**: 2026-08-11. + +> "If the type says it's not null, trust the type. If the type is +> wrong, fix the type. Don't add runtime null checks for values that +> can't be null." +> +> — Miguel Pizza, _No Defensive Null Checks_, Maintainable +> TypeScript doctrine. + +## Rule + +A runtime guard exists to handle one of two cases: + +- A **demonstrated** runtime scenario where the value can fall outside + the type system's narrowing (cross-realm objects, host-provided + values, third-party APIs that lie about their types). +- A **demonstrated** production failure where the code was wrong + about its preconditions. + +If neither has happened, the guard does not belong in the code. A +`typeof x === 'object' && x !== null` after the compiler has already +narrowed `x` to non-null is not a defence. It is a wall the next +contributor has to climb while they figure out which scenario the +author was worried about. + +## Why + +Speculative defences grow like moss: + +- A guard against a scenario that never occurred encourages the next + contributor to add a guard against their own scenario. +- Each guard is a tax on every reader: they have to determine whether + the case is real before they can trust the code path that follows. +- Guards of equal status cover both real and imaginary cases, so the + reader cannot tell which is which. + +A senior codebase carries **only the defences that have paid for +themselves**. A defence has paid for itself when the scenario it +covers has either: + +- Been observed in production and traced back to the absence of the + guard. +- Been identified by a static analyser or fuzz test as reachable. + +If neither, the guard is a tax on everyone and a benefit to no one. + +## The pattern this rule catches + +The rule is not against being defensive. It is against defending +against cases that have not been demonstrated. The distinguishing +shapes: + +- **Real**: `if (typeof x === 'function')` after the type system + declared `x: () => void`. The compiler already proved it; the + guard is redundant. +- **Real**: `if (typeof err === 'object' && err !== null)` before + reading `err.message`, when `err: unknown` and the caller might + pass null. The compiler excluded the case but the input contract + permits it. +- **Not real**: the same `if (typeof err === 'object' && err !== null)` + re-applied two statements after a narrowing branch that already + proved non-null. The compiler still has the narrowing, but the + author lost confidence in their own code and wrote the check + twice. + +The first two are real defences against real contracts. The third is +the smell this rule exists to catch. + +## What to do instead + +When you find yourself about to write a runtime check, ask five +questions in order. The first is the reframe; the next four are the +checklist. Any "no" answers the question of whether the guard +belongs. + +0. **Why is this value nullable at all?** A guard against null is + a question: "why was this value allowed to be null in the first + place?" If the answer is "it shouldn't be", the type is wrong; + fix the type. If the answer is "it can be, by design", the + guard is legitimate. If the answer is "I don't know", the guard + is a superstition. The reframe is the gate: a guard whose + origin cannot be named is a guard that does not belong. + +1. **What is the input contract?** Be able to state the precondition + on which the function relies. "The caller passes either an + `ErrorFactory` or a native error class" is a contract. "Anything" + is not. +2. **Is the contract expressible in the type system?** If yes, + express it. `err: object` excludes null. `err: unknown` does not. + A narrower input type is a defence the compiler provides for + free. +3. **Is the runtime check covering a case the type system cannot + rule out?** If yes, keep the guard and add a comment that names + the scenario (cross-realm `instanceof`, JSON-parsed foreign + values, host-provided callbacks, etc.). A guard without a named + scenario is the smell. +4. **Has this scenario actually occurred?** If no, do not encode + the guard. Wait for the bug report, the fuzz output, or the + static analyser warning. Until then, the code path is the + cleanest expression of the contract you actually have. + +**Bad** — guard without a named scenario, swallowing the failure: + +```ts +function discriminate( + err: unknown, + type: ErrorFactory | (new (...args: unknown[]) => Error) +): boolean { + // Pre-existing narrowing was already in place above this point. + if (typeof err === 'object' && err !== null) { + // Redefence of a narrowing the compiler already proved. + } + + // The code below this comment never sees a cross-realm object. + // The catch is a tax paid by every reader. + try { + return err instanceof type; + } catch { + // "instanceof can fail for cross-realm errors" — but they never arrive here. + } + // ... +} +``` + +**Good** — trust the narrowing; if the cross-realm scenario ever +appears, add the guard with a comment that references the bug: + +```ts +function discriminate( + err: unknown, + type: ErrorFactory | (new (...args: unknown[]) => Error) +): boolean { + if (typeof type === 'function' && 'prototype' in type) { + // No try/catch. instanceof against a class never throws on its own + // in the contexts this function is called from. If a cross-realm + // scenario is reported, add the guard with the bug number. + return err instanceof type; + } + // ... +} +``` + +## What this looks like in violation + +A function declared with `err: unknown` that, three statements later, +re-checks `typeof err === 'object' && err !== null` even though a +prior branch already returned on `err == null`. The author was not +sure the narrowing survived the intervening statements. The right +move is to trust the narrowing (the compiler tracks it across the +whole function), or to restructure the function so the narrowing +happens once at the top. + +A second smell: a guard inside a `try { ... } catch { /* swallow */ }` +that hides an exception which the surrounding code could not +conceivably throw. The catch is there "in case". It is not a defence; +it is a lie about what can fail. + +## Enforcement + +- **Code review**. A reviewer who sees a runtime check whose comment + reads "just in case", "to be safe", or is empty, blocks the PR + and asks for a named scenario. +- **Self-audit during refactor**. When touching a function for any + reason, list every runtime guard inside it. For each, ask: what + scenario does this cover, and has it occurred? If the answer to + the second is no, remove the guard and the corresponding + impossible branch. +- **Bug-driven addition only**. When a production incident reveals + a missing guard, the guard is added **with the bug report + number** in a comment that explains the scenario in one sentence. + The guard and the report form a closed loop. + +## The `== null` idiom — when the rule does not apply + +The rule forbids speculative defences on non-nullable types. It +does **not** forbid the `x == null` idiom, which is the canonical +way to test "the value is absent" at a boundary where the contract +permits absence. + +The JavaScript specification defines loose equality such that +**only `null` and `undefined` are equal to `null`** in `==` +comparison. No other falsy value matches: + +```ts +// Only null and undefined match in this idiom. +console.log(null == null); // true +console.log(undefined == null); // true +console.log(0 == null); // false +console.log('' == null); // false +console.log(false == null); // false +``` + +This makes `== null` a safe idiom for "value is absent". It is +the **opposite** of a speculative defence: it is the precise check +for the case the type system says is possible (`string | null | +undefined`). Banning it would force the same check in two lines: + +```ts +// Bad — the rule does not require this. The above is one line and clearer. +if (x === null || x === undefined) { + /* ... */ +} +``` + +The idiom is documented and recommended: + +> "Recommend `== null` to check for both `undefined` or `null`. You +> generally don't want to make a distinction between the two." +> +> — Basarat Ali Syed, _TypeScript book_, 2024 edition. + +> "We intentionally allow this [comparison against null on +> non-nullable types] for the sake of defensive programming (i.e. +> defending against missing inputs from non-TS code). If there's +> enough demand we could add a flag or something." +> +> — Ryan Cavanaugh, TypeScript core team, GitHub +> microsoft/TypeScript#11920, October 2016 (issue still open in +> 2025). + +The Microsoft team has explicitly **declined** to warn on null +comparisons even on non-nullable types, because the check is +legitimate at the JavaScript boundary. Banning `== null` in this +project would diverge from both the JavaScript standard and the +TypeScript team's official position. + +### When to use `===` instead + +`=== null` or `=== undefined` are appropriate when the contract +**explicitly distinguishes** null from undefined. That happens in +two cases: + +- **Initialised vs uninitialised.** A variable that starts as + `undefined` and is later assigned `null` to mean "explicitly + cleared" benefits from `=== null` (or `=== undefined`) to + distinguish the two states. +- **Public API contract.** A library that exposes a parameter + accepting `null | undefined` as two distinct values may need to + distinguish them at the boundary. + +In every other case, `== null` is the right idiom. + +## Exceptions + +A documented, scenario-named guard against an input that crosses a +trust boundary is legitimate: deserialised JSON, foreign realms, +host-supplied callbacks, third-party APIs that lie about their types. +These guards must carry a comment that names the scenario and the +reason the type system cannot rule it out. A guard without that +comment is the smell, not the exception. + +## What senior practitioners say + +The rule is not a stylistic preference. Four sources capture the +consensus that operationalises it: + +> "Experienced developers don't eliminate null checks entirely — +> they reduce the need for them by designing stronger contracts +> and clearer domain boundaries. Instead of constantly defending +> your methods with 'Could this be null?', the architectural +> question should be: 'Why was this object allowed to enter the +> system as null in the first place?'" +> +> — Aziz Kale, _Why Senior Developers Rarely Need `if (x == null)`_, +> Dev Genius, July 2026. + +Kale's reframe is the first question this rule asks. The author +of a guard is not the author of a safety net; they are the author +of a question. If the question has no answer, the guard has no +purpose. + +> "If the type says it's not null, trust the type. If the type +> is wrong, fix the type. Don't add runtime null checks for values +> that can't be null." +> +> — Miguel Pizza, _No Defensive Null Checks_, Maintainable +> TypeScript doctrine. + +Pizza's formulation is the operational form of the rule. A guard +on a non-nullable type is not a defence; it is a signal that the +type is wrong. The fix is in the type, not in the runtime. + +> "If you find yourself constantly writing repeating code to +> perform some validations, it's a strong sign you fall into the +> trap of primitive obsession." +> +> — Vladimir Khorikov, _Defensive programming: the good, the bad +> and the ugly_, Enterprise Craftsmanship. + +Khorikov's point is the smell of repetition. A guard that appears +in five methods is not five guards; it is one domain invariant +expressed five times in five places. The right shape is a type +that owns the invariant once. + +> "I took out as much of this 'protection' as I could safely +> remove, and cleaned up the error handling so that I could +> actually maintain the system without losing what was left of my +> mind. I setup trust boundaries for the code [...] deciding what +> data couldn't be trusted and what could." +> +> — Jim Bird, _Defensive Programming: Being Just-Enough Paranoid_, +> Building Real Software, March 2012. + +Bird's anecdote is the cautionary tale. A system saturated with +guards becomes unmaintainable. The fix is not more guards; the +fix is trust boundaries. Decide what is outside (untrusted) and +what is inside (trusted); the defences live at the boundary, not +throughout the body. + +The four sources converge on the same operational rule: guards +belong at the boundary between trusted and untrusted; inside +the trust boundary, the type system is the defence. This rule is +the operational form of that position. + +## Sources + +- **Pizza, Miguel.** _No Defensive Null Checks._ Maintainable + TypeScript doctrine. Operationalises the trust-the-type principle + for runtime guards: the rule's title and core position are + Pizza's. +- **Basarat Ali Syed.** _TypeScript book_, chapter on null and + undefined. The `== null` idiom is documented and recommended; + Basarat's position is that the loose-equality check is the + precise tool for "value is absent" and is not the kind of + speculative defence the rule forbids. +- **Kale, Aziz.** _Why Senior Developers Rarely Need `if (x == +null)`._ Dev Genius, July 2026. The reframe — "why was this value + allowed to be null in the first place?" — is captured in the + rule's question 0. +- **Pizza again.** Cited for the operational form: "if the type + says it's not null, trust the type. If the type is wrong, fix + the type." +- **Wycliffe, Maina.** _Avoid using Type Assertions in + TypeScript._ All Things TypeScript, October 2023. The + responsibility-transfer framing ("we are now responsible for this + type") informs the rule's "a guard whose origin cannot be named + is a guard that does not belong". +- **Khorikov, Vladimir.** _Defensive programming: the good, the bad + and the ugly._ Enterprise Craftsmanship. The repetition smell — + five guards across five methods are one domain invariant expressed + five times — informs the rule's structural critique. +- **Bird, Jim.** _Defensive Programming: Being Just-Enough + Paranoid._ Building Real Software, March 2012. The cautionary + tale — a system saturated with guards becomes unmaintainable — + anchors the rule's trust-boundary principle. +- **Microsoft TypeScript team, Ryan Cavanaugh.** + microsoft/TypeScript#11920, October 2016 (open as of 2025). + The compiler intentionally allows null comparisons on non-nullable + types; the rule operationalises this by carving out the + trust-boundary exception. + +## See also + +- **Rule 0007** — Top-Down Composition: a function that accumulates + guards is a function that has grown past its name. The + reframe in question 0 often reveals that the function's + responsibility should be split, not defended. +- **Rule 0008** — No Chained Type Assertions: the type-side + complement. A guard at a boundary without a cast is this rule's + smell; a chain of casts is 0008's smell. The two rules compound: + one is the runtime discipline, the other the type discipline, + and both ask the same question — "what is the contract?". diff --git a/docs/engineering/architecture/rules/0005-named-algorithms-and-independent-data-structures.md b/docs/engineering/architecture/rules/0005-named-algorithms-and-independent-data-structures.md new file mode 100644 index 0000000..f419417 --- /dev/null +++ b/docs/engineering/architecture/rules/0005-named-algorithms-and-independent-data-structures.md @@ -0,0 +1,394 @@ +# 0005 — Named Algorithms and Independent Data Structures + +**Status**: Active (enforced through code review). +**Date**: 2026-08-11. + +## Rule + +Three invariants, all anchored in the same principle: a reader should +be able to understand **what** the code does without first having to +decipher **how** it is described. + +1. **Algorithms are named.** A depth-first search described in three + lines of inline code with a `// DFS walk` comment is not a named + algorithm. It is a comment that happens to be near code. A named + algorithm is a function, a type, or a class whose name is the + concept, and whose body is the implementation. The reader should + be able to read the name and trust it. + +2. **No project-internal diminutives.** Names carry the meaning that + comments cannot. A **project-internal diminutive** — one that + is not a word in the language, the ecosystem, or the + mathematical convention — trains the reader to translate every + line. `DFS`, `mgr`, `ctx`, `arr`, `fn`, `cb`, `usr`, `cfg`, + `evt` are diminutives; spell them out as + `depthFirstSearch`, `manager`, `context`, `items`, `function`, + `callback`, `user`, `configuration`, `event`. The cost of the + extra characters is paid once; the cost of the abbreviation is + paid every time the code is read. + +3. **Data structures are explicit and independent.** A stack, queue, + heap, ring buffer, or sorted map used by an algorithm is a + **thing**. It deserves a type, a file, and a name. It must not + be inlined as a primitive array with `push`/`pop` because that + was the first thing that worked. More importantly, the data + structure must be **independent of the algorithm that first used + it**: an `ErrorInheritanceStack` for the inheritance walk must + not encode "depth-first" in its type. If a second algorithm needs + a stack for breadth-first traversal, it must be able to use the + same type. + +### Diminutives: the three categories + +Not every short name is a diminutive. The rule distinguishes three +categories by their source, not by their length. + +- **Language-standard words**: `id`, `url`, `json`, `html`, `css`, + `api`, `http`, `cli`, `sdk`, `uri`. These are words in the + vocabulary of the language and the ecosystem. The reader does not + translate them; they are part of the working vocabulary. The + rule does **not** ban them. +- **Mathematical and conventional names**: `i`, `j`, `k`, `n`, `T`, + `U`, `V` for loop indices and generic type parameters. These + names have a tradition older than any project and a density of + meaning that is hard to replicate with longer names in the same + context. The rule does **not** ban them; the rule requires + them to stay within the contexts where the convention applies + (loop bodies, generic signatures, mathematical operations). +- **Project-internal diminutives**: `mgr`, `ctx`, `arr`, `fn`, + `cb`, `dfs`, `bfs`, `usr`, `cfg`, `evt`, `req`, `res`. These are + neither language-standard nor mathematical. They are local + shortcuts that the author chose for the line they were writing. + The reader has no way to know them without reading the project's + glossary. The rule **bans** them. + +The length of a name is irrelevant to the rule. `id` is one +character and a word; `usr` is three characters and a diminutive. +`T` is one character and a convention; `dfs` is three characters +and a diminutive. The source of the name is what matters, not +the length. + +### Scope and reuse + +The rule applies uniformly to public and private names. A private +helper that uses `dfs` as a parameter name still trains the next +contributor who reads the helper to translate `dfs` to "depth-first +search". The training tax is paid by every reader, including the +author on the day they forget the context. + +Loop indices (`i`, `j`, `k`) are the **single exception** because +they are a mathematical convention, not a project choice. The +exception is scoped to tight, single-screen loops where the +convention is universal. An `i` in a fifty-line function is not +the same as an `i` in a five-line loop; the second is convention, +the first is a diminished name that should be spelled out. + +## Why + +A comment that names an algorithm is a **deferred definition**. The +comment promises a structure that does not exist; the code that +follows is responsible for delivering on the promise. If the code +delivers, the comment becomes redundant; if it does not, the comment +becomes a lie. A function whose name is the algorithm is honest by +construction: the name is the contract, the body is the proof. + +Diminutives are a tax that compounds. A codebase that uses `ctx` +everywhere trains its readers to translate every line. A codebase that +spells `context` trains them to read. The first codebase looks +"professional"; the second is professional. + +Coupling a data structure to the algorithm that first used it is a +form of premature commitment. The next algorithm that needs the +same structure either duplicates it or forks it; either way the +codebase loses. Independence is what makes a `Stack` reusable +across BFS, DFS, and undo-log implementations. + +## What this looks like in violation + +Three shapes that this rule exists to catch: + +- **Inline algorithm with comment**: + + ```ts + // DFS walk of inheritance tree using stack (prevents GC pressure) + const stack: ErrorFactory[] = [factory as ErrorFactory]; + const seen = new Set(); + while (stack.length > 0) { + const current = stack.pop()!; + if (seen.has(current)) continue; + seen.add(current); + if (current === ErrorType) return true; + const inherits = (current as ErrorFactory).inherits; + if (inherits !== undefined) { + if (Array.isArray(inherits)) { + for (let i = 0; i < inherits.length; i++) { + stack.push(inherits[i]); + } + } else { + stack.push(inherits); + } + } + } + ``` + + What is wrong: the comment names the algorithm, but the algorithm + is not a function. The reader has to read twenty lines to confirm + that the comment is accurate. The stack is a primitive `Array` + whose only contract is `push` and `pop`; a second algorithm cannot + reuse it without duplicating the type. The names `stack`, `seen`, + `current`, `inherits` are local; a reader scanning the function + does not know which is which. + + What the right shape looks like: + + ```ts + const result = walkInheritance( + factory, + inheritanceDepthFirst(), + (current) => current === ErrorType + ); + return result.found; + ``` + + Where `walkInheritance` is a generic traversal function in a + shared module, `inheritanceDepthFirst` is a stack-based strategy + in its own file, and the predicate is named for what it tests. + +- **Diminutive-heavy naming**: + + ```ts + function mgrErr(err: ErrT, ctx: Ctx): void { + const m = err.msg; + const arr = ctx.items.map((it) => it.id); + cb(arr); + } + ``` + + What is wrong: a reader has to translate `mgr`, `ErrT`, `Ctx`, + `m`, `arr`, `cb`, `it` before they can think about the function. + The function body is shorter to type than to read. + + What the right shape looks like: + + ```ts + function reportError(error: DomainError, requestContext: RequestContext): void { + const message = error.message; + const identifiers = requestContext.items.map((item) => item.identifier); + notifyListeners(identifiers); + } + ``` + +- **Algorithm-specific data structure**: + + ```ts + // In the inheritance walker + interface InheritanceWalkerState { + stack: ErrorFactory[]; // Implicit: depth-first + seen: Set; + found: boolean; + } + ``` + + What is wrong: the type encodes the algorithm in its shape. A + breadth-first walker cannot reuse `InheritanceWalkerState` without + duplicating the interface. The right shape is to separate the + walker from the strategy: + + ```ts + // Stack.ts (independent) + interface Stack { + push(item: T): void; + pop(): T | undefined; + isEmpty(): boolean; + } + + // depth-first.ts (strategy) + function depthFirst( + start: T, + expand: (node: T) => Iterable, + visit: (node: T) => boolean | void + ): boolean { + /* ... */ + } + + // Inheritance traversal (consumer) + function isInheritedFrom(factory: ErrorFactory, target: ErrorFactory): boolean { + return depthFirst( + factory, + (f) => f.inherits ?? [], + (f) => f === target + ); + } + ``` + +## When this rule does not apply + +A single-caller helper whose concept is local to its file lives +inline. See rule 0003. The point of this rule is to capture **shared +concepts** (an algorithm, a data structure, a name) and to surface +them at the right grain. A five-line local computation that does not +deserve a name does not need one. + +## Enforcement + +- **Code review**. A reviewer who sees an inline algorithm with a + comment-naming-it blocks the PR and asks for the algorithm to be + extracted to a function or type. +- **Naming audit**. A standing review of function and variable names + during PR review catches abbreviations before they land. "If I + had to look up what this abbreviation means, it is wrong." +- **Structure audit**. A quarterly review of "where does this data + structure live, and which algorithms use it?" surfaces the + structure-vs-algorithm coupling when it is still small. + +## Exceptions + +- **Language-standard words**: `id`, `url`, `json`, `html`, `css`, + `api`, `http`, `cli`, `sdk`, `uri`, `xml`. These are words in + the working vocabulary, not diminutives. The rule does not ban + them. +- **Mathematical and conventional names**: `i`, `j`, `k`, `n`, `T`, + `U`, `V`. These are conventions older than any project. The + rule applies them only within the contexts where the convention + holds (loop bodies, generic signatures, mathematical operations). + An `i` in a five-line loop is fine; an `i` in a fifty-line + function is not. +- **Generated code, vendor code, and bindings to external systems** + where the shape is fixed by the other side and the rule cannot + apply. + +## What senior practitioners say + +### On diminutives + +Three sources span the spectrum, and the rule operationalises the +intersection. + +> "Wrong. You might think you are faster at typing, but you don't +> write code in one go and never ever get back to it again. [...] +> Spending the extra minute it takes to write words in full will +> benefit you and your readers. [...] Can you tell what any of +> these names refer to, univocally?" +> +> — Julio Merino, _Readability: No abbreviations_, June 2013. + +Merino's position is the strictest. The rule bans project-internal +diminutives for the same reason he gives: the reader cannot +decipher them without a glossary, and the glossary does not exist. + +> "Names must be descriptive and clear to a new reader. Do not use +> abbreviations that are ambiguous or unfamiliar to readers +> outside your project, and do not abbreviate by deleting letters +> within a word." +> +> — Google TypeScript Style Guide, § Naming. + +Google's position is the calibrated one. The rule follows Google's +framing: abbreviations that are ambiguous or unfamiliar are banned; +abbreviations that are standard are kept. The three-category +distinction in this rule is Google's distinction made explicit. + +> "Standard Abbreviations are Fine [...] `iostream`, `int`, +> `std`, `cout`, `cin`, `endl` are all abbreviations. You +> wouldn't expect these to 'count' as abbreviations per se, +> because they are part of the language." +> +> "A name's length should not exceed its information content. +> For a local variable, the name `i` conveys as much information +> as `index` or `idx` and is quicker to read." +> +> — Keegan Donley, _When Can I Use Abbreviated Variable Names?_, +> August 2023; Russ Cox, _research!rsc: Names_, February 2010. + +Donley and Cox are the conventional exceptions. `i` in a loop, +`T` in a generic, `url` in a request handler — these names have +a meaning density that long names cannot replicate in the same +context. The rule's category "mathematical and conventional names" +is the union of Donley's language-standard and Cox's +information-content positions. + +### On independent data structures + +The "data structures are explicit and independent" invariant has +a thirty-year lineage in software engineering, anchored in +generic programming. + +> "By expressing the algorithms in terms of these basic access +> operations and making the operations parameters, we permit a +> single expression of the algorithms to be used with any concrete +> representation of the container." +> +> — Alexander Stepanov and David Musser, _Algorithm-oriented +> Generic Libraries_, Software — Practice and Experience, +> vol. 24(7), July 1994. + +Stepanov and Musser formalised what the rule calls +"independence": an algorithm parameterised by access operations +(`push`, `pop`, `less`, `swap`) works against any container that +exposes those operations. The container does not encode the +algorithm; the algorithm does not encode the container. The +`Stack` of the rule is the `Sequence` of Stepanov; the +depth-first walker is the `for_each` of Stepanov. Same +principle, three decades apart. + +> "A `for` loop is just a `find_if` over a range with a body +> side effect. A `find` is a `count_if` over a range with early +> termination." +> +> — Alexander Stepanov, _Notes on Programming_ (talk transcript, +> A9.com, 2007). + +Stepanov's deeper point: the algorithms are also independent +of each other. A walker can be expressed in terms of a fold; a +fold can be expressed in terms of a traversal. The rule's +"algorithms are named" invariant is the project-level restatement +of this. Each algorithm is a thing the consumer can name and +combine, not an inline shape the consumer has to read. + +## See also + +- **Rule 0007** — Top-Down Composition: the discipline that puts + the named algorithms this rule produces at the top of their + callers. This rule extracts; 0007 composes. +- **Rule 0009** — Open Extension, Closed Modification: the discipline + that turns the named data structures (Stack, Queue, etc.) into + reusable registries. +- **Rule 0003** — File Placement: a single-caller algorithm does + not need to be extracted yet; this rule says "name it when it has + a second caller", not "extract every algorithm immediately". + +## Sources + +The "named algorithms" invariant is anchored in: + +- **Merino, Julio.** _Readability: No abbreviations._ jmmv.dev, + June 2013. The strictest position on diminutives: spelling + words out is a tax the author pays once and the reader pays + forever. +- **Google TypeScript Style Guide.** _Naming._ The calibrated + position: abbreviations that are ambiguous or unfamiliar are + banned; abbreviations that are standard are kept. The + three-category distinction in this rule is Google's distinction + made explicit. +- **Donley, Keegan.** _When Can I Use Abbreviated Variable + Names?_ August 2023. The conventional exceptions: `i`, `T`, and + standard words have meaning density that long names cannot + replicate in their context. +- **Cox, Russ.** _research!rsc: Names._ February 2010. The + information-content framing: a name's length should not exceed + its information content. + +The "independent data structures" invariant is anchored in: + +- **Stepanov, Alexander, and David Musser.** _Algorithm-oriented + Generic Libraries._ Software — Practice and Experience, + vol. 24(7), July 1994. The canonical source for parameterising + algorithms by container access operations. The `Stack` + reusable across BFS, DFS, and undo-log is the TypeScript-level + restatement of Stepanov's `Sequence` parameterised by + iterators. +- **Stepanov, Alexander.** _Notes on Programming._ A9.com, 2007 + talk transcript. The deeper point: algorithms are also + independent of each other. A walker is a fold; a fold is a + traversal. The rule's "algorithms are named" invariant is the + project-level restatement. diff --git a/docs/engineering/architecture/rules/0006-technology-choices.md b/docs/engineering/architecture/rules/0006-technology-choices.md new file mode 100644 index 0000000..6f4bc09 --- /dev/null +++ b/docs/engineering/architecture/rules/0006-technology-choices.md @@ -0,0 +1,205 @@ +# 0006 — Technology Choices: Assumptions Made Explicit + +**Status**: Active (enforced through code review and release process). +**Date**: 2026-08-11. + +## Rule + +Every technology choice that shapes the codebase — language mode, +module system, validation strategy, dependency philosophy, runtime +target — must be a **deliberate assumption**, not an inheritance from +defaults. + +A deliberate assumption is: + +1. Stated in writing, in a place a contributor will find it (an ADR + in `decisions/`, a rule in `rules/`, or a comment at the boundary + where the assumption bites). +2. Justified in terms of what it rules out, not just what it enables. + "We use TypeScript strict mode" is not enough; "We use TypeScript + strict mode so that the compiler is the first reviewer of every + PR" is. +3. Revisited when the assumption starts costing more than it saves. + The cost shows up as test-suite patches that exist only to satisfy + the type system, or as dependency conflicts at every release. + +The defaults of the language, the framework, or the package manager +are not assumptions. They are accidents of choice. Treating them as +assumptions is how a codebase drifts from "we chose this" to "it just +happened to be like that". + +## Why + +A foundation library reaches users who do not share its assumptions. +The choice of ESM-only, the choice of a specific validator, the +choice of a Node.js version, the choice of CommonJS-or-not, all of +these become contracts that downstream code must respect. A choice +that was not deliberate becomes a constraint that no one can +explain. + +The same logic applies inside the codebase: a function that silently +relies on a Promise being resolved synchronously, a module that +assumes Node.js 22 features, a config that depends on a specific +build tool's behaviour. Each of these is an assumption made by an +author who did not have to think about the alternative. When the +alternative becomes relevant — when a Node version is dropped, when +a build tool is replaced, when a user reports a bug — the assumption +becomes a wall. + +## The shape of a deliberate technology assumption + +Every assumption in this codebase should answer four questions in a +single paragraph: + +- **What is the choice?** "ESM-only TypeScript, published as `.js` + with `.d.ts` declarations. No CommonJS shim." +- **What does it enable?** "Consumers import from `@scope/pkg` and + get tree-shaking, top-level await, and exact types from the + package source." +- **What does it rule out?** "Consumers on CommonJS resolvers + cannot use this package without dynamic `import()` or a build + step. We accept that exclusion because the alternative (a CJS + shim) would double the surface area to maintain." +- **When would we revisit?** "When Node.js ends ESM-only support + (it has not announced this), or when a downstream pattern + suggests the exclusion is becoming a tax rather than a choice." + +A choice without a "what does it rule out" is the smell. Every +choice rules something out; an author who cannot name what is +ruled out has not understood the choice. + +## Specific choices this codebase commits to + +These are the assumptions made explicit. New assumptions join this +list, they do not replace it. + +- **TypeScript strict mode.** The compiler is the first reviewer. + No `any` leak, no implicit `any`, no unchecked index access. +- **ESM-only.** No CommonJS shim, no `module: "commonjs"`, no + dynamic require from the published surface. A consumer on CJS + uses dynamic `import()`. +- **Standard Schema for runtime validation.** The validation + contract is the schema, not the validator. Zod, Valibot, and + ArkType all implement Standard Schema; the consumer chooses. + Pinning to a specific validator would couple every consumer to + a release cadence that is not ours. +- **Dependency minimalism.** The runtime surface is the library + plus its declared peer dependencies. A new runtime dependency + must justify itself (rule 0001, invariant 8); a new dev + dependency must justify itself by what it enables in CI or in the + inner loop. +- **Function-based API surface.** No classes on the consumer side. + Factories, predicates, and combinators are the public shape. The + reason: classes introduce an inheritance coupling that the + consumer did not ask for. Functions compose without inheritance. +- **Honest runtime.** No transpilation tricks that hide the runtime + target. The code is written for the runtime it ships to. + +## How to add a new assumption + +When a PR introduces a new technology choice — a new dependency, a +new build step, a new runtime feature, a new compiler flag — the PR +description must: + +1. Name the choice. +2. State what it enables. +3. State what it rules out. +4. State under what circumstances the choice would be revisited. + +If any of the four is missing, the PR is incomplete and the +reviewer should ask for it. + +When the choice is durable — it will shape the codebase for a year +or more — the same four answers live in an ADR under +`architecture/decisions/`. When the choice is local to a module, the +four answers live in a comment at the boundary where the choice +bites. + +## What this looks like in violation + +A PR that adds `pnpm add lodash` with the commit message "needed for +deep clone". What is missing: + +- What does lodash enable that the standard library does not? +- What does lodash rule out (license risk, release cadence, + surface area, etc.)? +- Under what circumstances would we revisit? + +The PR adds an assumption without naming it. The next contributor who +looks for "why lodash?" finds nothing and either keeps it (because +removing it feels risky) or duplicates it (because they do not know +whether to add or remove). + +A second smell: a `tsconfig.json` with `"strict": false` because the +project started before strict mode was the default. The assumption +"TypeScript with relaxed checks" was inherited from a default, not +chosen. It becomes a wall every time a contributor wants to enable a +strict-mode feature. + +**Bad** — a choice without a justification: + +```ts +// In a PR description +'Added zod to validate the user signup payload.'; +``` + +The reviewer reads this and has no way to evaluate the choice. Is +the standard library not enough? Is Standard Schema acceptable? Why +zod and not Valibot or ArkType? The reviewer is forced to either +trust the author or block the PR to ask. + +**Good** — the four questions answered in the PR description: + +```md +## Why zod + +- **What is the choice?** zod as the runtime validator for user + signup payloads, used via the Standard Schema adapter (not the + zod-native API). +- **What does it enable?** Inference of `UserSignup` types directly + from the schema, ergonomic error messages, and Standard Schema + compliance that lets consumers swap validators later. +- **What does it rule out?** A direct dependency on zod's API. We + use Standard Schema as the contract; if a future user prefers + Valibot, the change is local to the validator factory. +- **When would we revisit?** If the package's release cadence slows + below our SLA, or if a security advisory lands and is not + resolved within 30 days. +``` + +The reviewer can now evaluate the choice against alternatives. The +next contributor who looks for "why zod?" finds the answer in the +git log. + +## Enforcement + +- **PR review**. A reviewer who sees a new dependency, a new build + step, a new compiler flag, or a new runtime feature without a + four-answer justification in the PR description blocks the PR. +- **Release audit**. A standing review at release time lists every + dependency and every technology assumption. Anything that has lost + its justification (the dependency is no longer used, the choice + no longer applies) is removed in the same release. +- **Quarterly review**. The list of "specific choices this codebase + commits to" above is reviewed. Stale choices are either reaffirmed + with a current justification or marked for removal. + +## Sources + +This rule is a synthesis of the project's own architectural +commitments. No single external reference anchors it. The +four-question template (what, enables, rules out, revisits) is +modelled on the _Architecture Decision Record_ convention +popularised by Michael Nygard's _Documenting Architecture +Decisions_; the project tracks individual ADRs in +`docs/engineering/architecture/decisions/` and the rule governs +the **shape** those ADRs and inline commitments must take. + +## Exceptions + +A transitive dependency installed by a direct dependency is not a +choice and is not subject to this rule. The choice was the direct +dependency; the transitive follows. If a transitive dependency's +behaviour becomes load-bearing, that is a smell to investigate +under rule 0001 invariant 8 ("no dependency without +justification"), not a rule on its own. diff --git a/docs/engineering/architecture/rules/0007-top-down-composition.md b/docs/engineering/architecture/rules/0007-top-down-composition.md new file mode 100644 index 0000000..41daba4 --- /dev/null +++ b/docs/engineering/architecture/rules/0007-top-down-composition.md @@ -0,0 +1,208 @@ +# 0007 — Top-Down Composition: the Consumer's Eye Wins + +**Status**: Active (enforced through code review). +**Date**: 2026-08-11. + +## Rule + +A function reads **top-down**: the first lines tell the reader what +the function does, and every subsequent step is either a name they +already understand or a name this file introduces in service of the +story. + +The story is the consumer's story. When the consumer reads the file, +they see the **outcome first** and the mechanism second. Mechanism +that does not advance the story is a candidate for extraction, not +for inlining. + +The internal cleverness of an implementation — its micro-optimisations, +its compactness, its "I can fit this in three lines" feel — is +secondary to the consumer's experience of reading the function. +Cleverness that does not serve the reader is a wall. + +## Why + +A function written bottom-up reads like an archaeological dig: the +reader has to reconstruct the author's thinking to recover the +intent. A function written top-down reads like a sentence: the +subject is named in the first line, the verb in the second, the +modifiers after. The reader's mental model updates as they read, +not after. + +The rule is not "top-down is good". The rule is "top-down serves the +reader". Bottom-up is sometimes appropriate when the function is +inherently low-level (a primitive that the top layers compose); in +that case the bottom-up style is honest because the function is a +building block, not a story. The rule picks the right style for the +right layer. + +DX is the constraint that keeps top-down honest. A function that +reads beautifully at the top but hides its costs in the helpers it +calls has not earned its beauty; the reader has to climb into the +helpers to know what they actually do. The right shape is the one +where the **entire call chain** is honest at each layer. + +## What this looks like in practice + +A function written top-down looks like this in shape: + +```ts +function walkInheritanceDepthFirst( + start: ErrorFactory, + predicate: (factory: ErrorFactory) => boolean +): boolean { + return walkGraph(start, factoryChildren, depthFirstTraversal(), predicate); +} +``` + +The reader sees: walk the inheritance graph, depth-first, return +whether the predicate matched. They do not see: `Array.push`, +`Set.has`, `while (stack.length > 0)`, the cycle-detection set, the +pop order. Each of those belongs in a function whose name carries +the concept; the consumer's eye sees the concept, not the +mechanism. + +A function written bottom-up looks like this in shape: + +```ts +function walkInheritanceDepthFirst( + start: ErrorFactory, + predicate: (factory: ErrorFactory) => boolean +): boolean { + // Inline depth-first with cycle detection + const stack: ErrorFactory[] = [start]; + const seen = new Set(); + while (stack.length > 0) { + const current = stack.pop()!; + if (seen.has(current)) continue; + seen.add(current); + if (predicate(current)) return true; + const inherits = current.inherits; + if (inherits !== undefined) { + if (Array.isArray(inherits)) { + for (let i = 0; i < inherits.length; i++) { + stack.push(inherits[i]); + } + } else { + stack.push(inherits); + } + } + } + return false; +} +``` + +The reader has to parse the algorithm to recover the intent. The +algorithm is correct; the readability is not. + +## How to write top-down + +When you start a function, write the **first line** as if it were +the only line the consumer will see. Then ask: what would the +consumer need to know next? That is the second line. Then the +third. The function's body is a sequence of names the consumer +follows, not a sequence of operations the consumer has to evaluate. + +Three operations help: + +1. **Name the operation before implementing it.** If you cannot + name it, you have not understood it yet (rule 0001 invariant + 3). Go back to the input contract. +2. **Extract before composing.** When the second step is more than + a line, it is a candidate for extraction (rule 0003). The + extraction makes the top layer honest. +3. **Read the function aloud.** If the names, in order, do not + form a sentence the consumer would recognise, the order is + wrong. Reorder, or extract, until they do. + +## The DX constraint + +"DX wins" is not a slogan. It is a decision rule that overrides +other considerations in a fixed order: + +1. If a refactor makes the consumer's experience better, do it + even if it costs an internal layer. +2. If a micro-optimisation makes the consumer's experience worse + (more lines to read, more concepts to hold) and does not produce + a measurable improvement at scale, do not do it. +3. If a clever abstraction makes the consumer's experience worse + because it forces them to learn a new vocabulary, prefer the + obvious spelling even if it is a few lines longer. + +The rule is not "no cleverness". It is "cleverness must be earned by +serving the reader". A clever abstraction that the consumer +benefits from is welcome. A clever abstraction that only the author +benefits from is a wall. + +## What this looks like in violation + +Three shapes that this rule exists to catch: + +- **Twenty-line algorithm in a one-line function's body.** The + consumer sees `walkInheritanceDepthFirst(...)` and expects a + one-line answer. Instead they get an inline traversal that they + have to parse to know what the function does. +- **Helpers named after their implementation, not their + intention.** `cycleDetectedSet()` is named after what it does in + this file; `visitHistory()` is named after what it represents + in the consumer's vocabulary. The first is bottom-up, the second + is top-down. +- **Functions that do too much at the top.** A function whose + first line says `processItem(item)` and whose body is fifty + lines is honest about what it does, but dishonest about how + readable it is. Either the function is doing too much (extract), + or its name is too vague (rename to capture the actual + outcome). + +## When this rule does not apply + +A primitive whose job is to be a primitive is not subject to the +rule. A `Stack.pop()` method that returns the top element or +`undefined` is bottom-up by design: the primitive is the mechanism. +The top-down shape lives in the function that uses the primitive, +not in the primitive itself. + +A function whose only reader is the author (a one-off test fixture, +a debug helper, a temporary script) is not subject to the rule. +Top-down is a discipline for code that ships. + +## Enforcement + +- **Code review**. A reviewer who reads a function top-down and + cannot summarise what it does in one sentence by the third line + blocks the PR. The fix is extraction or rename, not "add a + comment". +- **Self-review**. Before opening a PR, the author reads each new + function aloud. If the names, in order, do not form a sentence + the consumer would recognise, the function is not ready. +- **Quarterly review**. A standing review of "which functions in + this codebase have grown past their name?" surfaces candidates + for extraction. A function whose name no longer summarises its + body is a refactor candidate, not a backlog item. + +## Exceptions + +Generated code, vendor bindings, and the lowest-level helpers of a +shared primitive module are not subject to top-down reading. They +are read by the consumer's eye at the layer above; their own +internal style may be bottom-up because their job is to be +mechanism. + +## See also + +- **Rule 0005** — Named Algorithms and Independent Data Structures: + this rule composes the named algorithms that 0005 extracts. +- **Rule 0009** — Open Extension, Closed Modification: the discipline + that keeps the composed layers stable across changes to the + enumeration. + +## Sources + +This rule is a synthesis of the project's own working +experience. The "DX wins" framing draws on common usage in the +JavaScript ecosystem (the term appears in many libraries' +contributing guides), but the operational form — the first line +must say what the function does, the consumer must not have to +climb into helpers — is not anchored to a single external +reference. The rule captures a discipline the project has paid +for in past reviews. diff --git a/docs/engineering/architecture/rules/0008-no-chained-type-assertions.md b/docs/engineering/architecture/rules/0008-no-chained-type-assertions.md new file mode 100644 index 0000000..fcb8818 --- /dev/null +++ b/docs/engineering/architecture/rules/0008-no-chained-type-assertions.md @@ -0,0 +1,264 @@ +# 0008 — No Chained Type Assertions + +**Status**: Active (enforced through code review). +**Date**: 2026-08-11. + +## Rule + +A type assertion may appear at most once in a single expression. +The shapes `as X as Y`, `as unknown as Y`, and any sequence of two or +more assertions are forbidden. + +A single assertion is allowed when it crosses exactly one type +boundary: augmenting a host type the consumer controls, narrowing a +`unknown` from a documented boundary (an IPC, a deserialised value, a +foreign-realm object), or asserting the runtime shape of a value the +type system cannot describe. + +A chained assertion is not an assertion. It is a confession that the +author has lost the thread of the type and is reaching for the +escape hatch twice in a row to make the compiler stop complaining. +The compiler is right to complain. The fix is not a longer cast; the +fix is a better type. + +## Why + +A single assertion documents a contract: "I know this value is of +type X, even though the type system does not." The reader can audit +the contract once. + +A chained assertion documents nothing. The intermediate `unknown` +or `as X` erases the reasoning between the source type and the +target type. The reader cannot audit what was assumed; they can +only see that two casts were stacked, and assume the author had a +reason. Often the author did not. + +The compiler is not the enemy. When the compiler rejects an +assertion, it is pointing at a real ambiguity in the code. A chained +cast papers over the ambiguity instead of resolving it. The code +compiles, but the type contract is now fictional. + +## What the rule forbids + +- `value as A as B` — two assertions in one expression. +- `value as unknown as B` — the explicit "I give up" double cast. +- `value as unknown as unknown as B` and longer chains. +- The functional equivalent in generics: `(value as Foo).bar as Baz`. +- `as` casts that target a type that requires another `as` to + construct. If the right-hand side is not reachable in one cast, + the right-hand side is the wrong target. + +## What the rule allows + +- A single `value as T` where `T` is reachable from the source type + by one explicit widening or narrowing the author can name. +- A single `value as unknown` followed by **structural work** that + produces a new value, not a second cast. Example: + `value as unknown; if (!isShape(value)) throw ...; return value as +Shape;` is acceptable because the work between the two + occurrences is a runtime guard, not another assertion. +- Augmentation of host types with `declare module` to teach the type + system about a property the runtime provides. This is a + declaration, not an assertion. + +## How to fix a chained cast + +When the compiler forces you to write `value as X as Y`, the right +fix is one of three, in order of preference: + +1. **Use a runtime guard that produces the type the compiler + expects.** A function that takes `unknown` and returns `T | null` + removes the cast at the call site: `const typed = toShape(value); +if (typed === null) throw ...;`. The compiler narrows after the + guard; the assertion disappears. + +2. **Change the source type.** If `value` is typed too narrowly to + cast to `Y`, the source type is the bug. Widen the source by + making the function that produces it return a more precise type, + or by accepting `unknown` at the boundary. + +3. **Add a typed accessor.** If the chain exists because the + consumer has to reach into a host object, write a function + `getFactorySymbol(error: unknown): ErrorFactory | undefined` that + hides the cast inside a named operation. Callers stop casting; + the cast lives in one named place that can be reviewed. + +The rule is not "no casts ever". The rule is "if a cast crosses two +boundaries, you have not understood what you are doing. Stop and +ask what the cast is for." + +## What this looks like in violation + +Two patterns that this rule exists to catch. Each is shown bad +then good. + +**The first, common — chained cast on a value the author controls:** + +```ts +// Bad — the author could have typed `instance` correctly from the start. +const instance = new Error(message) as unknown as Record unknown>; +instance[FACTORY_SYMBOL] = ErrorFactoryInstance; +``` + +The cast is two-level. The first `as unknown` erases the `Error` +type. The second `as Record<...>` reinvents a type that does not +exist. The author could have declared `instance` as +`ErrorInstance` from the start and assigned the symbol via the +declared property — no cast needed. + +```ts +// Good — the constructor's return type carries the right shape; the +// property assignment goes through the declared type. +const instance = new Error(message) as ErrorInstance; +instance[FACTORY_SYMBOL] = ErrorFactoryInstance; +``` + +One cast. The cast crosses one boundary (the `Error` constructor +returns a base `Error`, not the augmented `ErrorInstance`). The +property assignment is on the declared type, not a re-invented +shape. + +**The second, defensive — single cast at the wrong layer:** + +```ts +// Bad — the cast is one level, but it is in business logic, not at a boundary. +const marker = error as Record; +const factory = marker[FACTORY_SYMBOL]; +``` + +The cast is in business logic. The `error` value crossed no IPC, +no deserialisation boundary, no foreign realm. The cast is a leak +of internal knowledge into the call site. + +```ts +// Good — the cast lives inside a named accessor that is the only +// place it appears. +const factory = getFactory(error); +``` + +Where `getFactory` returns `ErrorFactory | undefined` and the cast +lives inside it. The cast is now at a named boundary; the business +logic is honest about what it knows. + +## The positive example — when a single cast is correct + +A single cast crossing **one** named boundary is allowed. Example: + +```ts +// Good — the cast crosses one boundary: the JSON parse returned a +// value of unknown shape; the guard below narrows without a second +// cast. +const raw = JSON.parse(payload) as unknown; +if (!isShape(raw)) { + throw new InvalidPayloadError(payload); +} +return raw; +``` + +One cast, one boundary. The guard below the cast does the structural +work. If the guard is missing, this is a violation of rule 0004 (no +speculative defences), not a violation of rule 0008. The two rules +compound cleanly: a cast at a boundary without a guard is 0004's +smell; a chain of casts is 0008's smell. + +## Enforcement + +- **Code review**. A reviewer who sees two casts in one expression + blocks the PR. The fix is one of the three patterns above, not a + comment justifying the chain. +- **Lint rule**. A future ESLint rule (`@typescript-eslint/no-duplicate-type-assertions`) + or a custom one can flag the pattern mechanically. The rule's + existence is the enforcement signal even before it is automated. +- **Self-review**. Before opening a PR, the author searches the diff + for `as` and counts the assertions per expression. Any + expression with more than one is rewritten before submission. + +## Exceptions + +A pattern that crosses a documented IPC, deserialisation, or +foreign-realm boundary may legitimately require a single `unknown` +cast on the receiving side. The cast must be at the boundary, not +deeper in the call chain. If the cast moves into business logic, it +is no longer a boundary cast and is forbidden. + +## What senior practitioners say + +The rule is not a stylistic preference; it is the operational +form of a position shared by senior TypeScript practitioners. Three +sources capture the consensus: + +> "Casting like this takes away TypeScript's power because you are +> now telling it what to believe rather than the tooling basing +> that belief on logic, inference, etc." +> +> — Darryl Edwards, _TypeScript – don't misuse casting_, Code +> Krispies, June 2024. + +Edwards's point is that `as unknown as Y` is not a workaround; it +is the abdication of the type system's job. The author is +substituting their own reasoning for the compiler's reasoning, +and the compiler can no longer help. The reasoning that was lost +is the reasoning the reader would have benefited from. + +> "When we use type assertion we are basically telling the +> TypeScript compiler that we know what the type is and it should +> trust us, i.e. we know what we are doing. The problem with this +> is that we prevent TypeScript from helping us where it should +> and take on that responsibility ourselves." +> +> — Maina Wycliffe, _Avoid using Type Assertions in TypeScript_, +> All Things TypeScript, October 2023. + +Wycliffe makes the responsibility transfer explicit. The cast +is a contract: "I, the author, am now responsible for this +type." The reader inherits that responsibility when they touch +the code. A cast at a boundary is a documented contract; a cast in +business logic is an undocumented one. + +> "Any external data has an `unknown` type by default until it is +> inferred." +> +> — Anton Beluzhenko, _Why `as unknown as Type` should be banned_, +> JavaScript in Plain English, April 2024. + +Beluzhenko frames the legitimate case. A value at a boundary is +`unknown` by definition — the contract is that the next step is +inference (via a guard, a parser, or a schema). The cast +`as unknown` at the boundary is honest; the cast `as unknown as Y` +at the same boundary is dishonest because it skips the inference +step that the boundary demands. + +The three sources converge on a single operational rule: a single +cast is acceptable at a boundary, paired with a guard; a chain of +casts is never acceptable. This rule is the operational form of +that position. + +## See also + +- **Rule 0004** — No Speculative Defences: the rule that covers + casts at a boundary without a guard. The two rules compound: + a cast at a boundary without a guard is 0004's smell; a chain + of casts is 0008's smell. +- **Rule 0012** — Prefer `type` Over `interface`: the type + discipline this rule relies on. A `type` declaration that + requires a cast to use is a violation of 0008; the declaration is + the wrong shape. + +## Sources + +- **Beluzhenko, Anton.** _Why `as unknown as Type` should be + banned._ JavaScript in Plain English, April 2024. The title is + the position; the body explains why the pattern abdicates the + type system's power. +- **Edwards, Darryl.** _TypeScript – don't misuse casting._ Code + Krispies, June 2024. The categorical "never use this pattern" + framing. +- **Wycliffe, Maina.** _Avoid using Type Assertions in + TypeScript._ All Things TypeScript, October 2023. The + responsibility-transfer framing: a cast is a contract the + reader inherits. +- **Pizza, Miguel.** _No Defensive Null Checks._ Maintainable + TypeScript doctrine. The "trust the type" formulation, which + underwrites rule 0004 and is also the slogan of rule 0001, + applies equally to casts: a chain of casts is not an + assertion, it is a confession. diff --git a/docs/engineering/architecture/rules/0009-open-extension-closed-modification.md b/docs/engineering/architecture/rules/0009-open-extension-closed-modification.md new file mode 100644 index 0000000..9556464 --- /dev/null +++ b/docs/engineering/architecture/rules/0009-open-extension-closed-modification.md @@ -0,0 +1,165 @@ +# 0009 — Open Extension, Closed Modification + +**Status**: Active (enforced through code review). +**Date**: 2026-08-11. + +## Rule + +A function that branches on a **known set of values** dispatches +through a registry (a `Map`, a `Record`, or a typed table), not +through a chain of `if`/`switch` statements. New values are added +by extending the registry; they are not added by editing the +branching function. + +A function that branches on a value **not drawn from a known set** +(value the function does not own — user input, foreign values, +free-form strings) keeps its branching as the right shape, because +there is no registry to extend. + +The distinction matters. A registry is the right shape when the +function itself enumerates the cases. Branching is the right shape +when the function validates an input it did not enumerate. + +## Why + +A chain of `if (kind === A) ... else if (kind === B) ... else if +(kind === C) ...` puts every case in the same place as the +dispatcher. Adding a case means editing the dispatcher. Removing a +case means searching the dispatcher for the string. Renaming a case +means changing it in the dispatcher and every call site. The +function is the **centre of gravity** for everything related to +the enumeration; everything else orbits it. + +A registry inverts the shape. The function reads from a table; +adding a case means adding a row to the table, in the place that +already knows about cases (the table itself, or the module that +exports the table). The dispatcher does not change. The function +becomes **stable across changes to the enumeration** — which is +the property the rule is named after. + +The registry also makes the set visible. A `Map` named `messageFormatters` reads as a table of contents; +a chain of `if (modifier === 'upper')` reads as implementation +detail. The first tells the reader what exists; the second forces +them to read every branch to know. + +## What this looks like in practice + +A chain of branches that should be a registry: + +```ts +function formatTemplate(template: string, data: Record): string { + return template.replace(/\{(\w+)(?::(\w+))?\}/g, (_, fieldName, modifier) => { + const value = data[fieldName]; + if (value === undefined) return _; + if (modifier === 'upper') return String(value).toUpperCase(); + if (modifier === 'lower') return String(value).toLowerCase(); + if (modifier === 'json') return JSON.stringify(value); + return String(value); + }); +} +``` + +Adding `:base64` means editing the function. The set of modifiers is +not visible at a glance. + +The same logic as a registry: + +```ts +// format/modifiers.ts +type MessageFormatter = (value: unknown) => string; +const messageFormatters = new Map([ + ['upper', (value) => String(value).toUpperCase()], + ['lower', (value) => String(value).toLowerCase()], + ['json', (value) => JSON.stringify(value)], +]); + +// format/template.ts +function formatTemplate(template: string, data: Record): string { + return template.replace(/\{(\w+)(?::(\w+))?\}/g, (_, fieldName, modifier) => { + const value = data[fieldName]; + if (value === undefined) return _; + const formatter = messageFormatters.get(modifier ?? ''); + return formatter ? formatter(value) : String(value); + }); +} +``` + +Adding `:base64` is one line in `modifiers.ts`. The dispatcher +does not change. The set of modifiers is visible in one file. + +## When the rule does not apply + +The rule is about **internal enumerations**: sets of values the +codebase itself defines and recognises. The rule does not apply to: + +- **Validation of external input.** A function that validates + user-supplied strings does not have a registry of valid inputs; + it has a condition. Branching is correct. +- **Type narrowing of polymorphic values.** A function that + dispatches on a discriminated union does not need a registry; the + union is the registry, and an exhaustive `switch` is the right + shape because the compiler can prove completeness. +- **Two or three cases that never change.** A binary toggle does + not need a registry. The rule applies when the set is open or + grows. + +## How to refactor a chain into a registry + +When you see a chain of branches that you suspect should be a +registry, ask three questions: + +1. **Who owns the enumeration?** If the function itself defines + the cases (a known set of message modifiers, a known set of + MIME types, a known set of error codes), the cases belong in a + table. If the function is just validating external input, the + chain is fine. +2. **Where would a new case be added?** If the answer is "in this + function", the function is the bottleneck. Move the table to a + module that is the natural home for the enumeration. +3. **Is the set visible at a glance?** If a reader has to read + every branch to know what exists, the table is hidden inside the + dispatcher. The fix is to surface it. + +## Enforcement + +- **Code review**. A reviewer who sees a chain of `if (kind === X) +... else if ... else if ...` on an internally-defined enumeration + asks for the table form. +- **Quarterly review**. A standing review of "which dispatchers + have grown past three branches?" surfaces the candidates before + the chain becomes impossible to read. +- **Lint rule** (future). A custom ESLint rule can detect chains + longer than a threshold on a single parameter and suggest the + registry form. The rule's existence is the enforcement signal + even before it is automated. + +## Exceptions + +A binary or ternary branch over values that are not an +enumeration (`if (isDryRun) ... else ...`) does not benefit from a +registry. The registry would be larger than the chain. + +A `switch` over a discriminated union, where the compiler can +prove exhaustiveness, is a better shape than a registry because the +compiler can warn if a case is added to the union but not the +switch. Keep the `switch`; do not turn it into a registry. + +## Sources + +This rule operationalises the **Open/Closed Principle** as +articulated in Robert C. Martin's _Designing Object-Oriented +C++ Applications_ (Prentice Hall, 1995) and later popularised +through his writings on agile software development. The OCP +states that software entities should be open for extension but +closed for modification; in this codebase the rule extends OCP +to the function level: a function that branches on an +internally-defined enumeration is closed for modification +(the dispatcher does not change) and open for extension (a new +case is a new row in the registry). + +The rule is consistent with how framework code in mature +JavaScript projects handles dispatch tables (e.g. Redux reducers +shaped as `Record`, Vite plugin hooks +shaped as a registry). The registry pattern is the TypeScript- +native expression of OCP at the function level. diff --git a/docs/engineering/architecture/rules/0010-typed-environment-access.md b/docs/engineering/architecture/rules/0010-typed-environment-access.md new file mode 100644 index 0000000..e7fa85f --- /dev/null +++ b/docs/engineering/architecture/rules/0010-typed-environment-access.md @@ -0,0 +1,185 @@ +# 0010 — Typed Environment Access + +**Status**: Active (enforced through code review). +**Date**: 2026-08-11. + +## Rule + +No business-logic file reads `process.env`, `globalThis`, or any +other runtime global directly. Every environment value is read +through a **typed accessor module** that: + +1. Declares the expected keys as a literal-union type or a schema. +2. Reads the values exactly once at module load (or behind an + explicit, memoised function). +3. Returns typed values, not strings, to the rest of the codebase. + +The accessor module is the only place in the codebase that touches +runtime globals. Every other file imports the typed accessor. + +## Why + +`process.env.X` is a `string | undefined` with no documentation, no +type, and no validation. Every read repeats the same type assertion +that hides the value's real shape. Two modules reading the same key +may disagree about what it means. A typo in the key name compiles. + +The typed accessor solves all four problems in one move. The keys +are declared once; the values are parsed once; the rest of the +codebase gets a function call that returns the right type. The +accessor becomes the place where the runtime meets the type system, +and there is exactly one such place. + +This is also the rule that lets the codebase avoid the runtime +dependency on `@types/node` (rule 0006). The accessor declares its +own ambient `process` shape; every other file does not have to. + +## What this looks like in practice + +A `process.env` read scattered across the codebase: + +```ts +// In one module +const legacyGate = process?.env?.DEESSEJS_ERRORS_LEGACY_TEMPLATES; +if (legacyGate === '1') return; + +// In another module +const logLevel = process?.env?.LOG_LEVEL ?? 'info'; + +// In a third module +const apiKey = process?.env?.API_KEY; +``` + +Each read redeclares `process` because `@types/node` is not in scope. +Each read uses `?.` and `??` to defend against the absence of +`process`. None of the keys are documented. None of the values are +validated. + +The same logic centralised: + +```ts +// env.ts — the only file that touches process.env +type Environment = { + DEESSEJS_ERRORS_LEGACY_TEMPLATES?: '1' | undefined; + LOG_LEVEL?: 'debug' | 'info' | 'warn' | 'error'; + API_KEY?: string; +}; + +declare const process: { env: Environment } | undefined; + +function readEnvironment(): Environment { + return (process as { env: Environment } | undefined)?.env ?? {}; +} + +const environment: Environment = readEnvironment(); + +// env.ts is also the only file allowed to declare `process`. +``` + +Every other file imports the typed accessor: + +```ts +import { environment } from './env.js'; + +if (environment.DEESSEJS_ERRORS_LEGACY_TEMPLATES === '1') return; +const logLevel = environment.LOG_LEVEL ?? 'info'; +``` + +No cast in any consumer. No duplicated `?.` chains. The keys are +documented in one place. The values are validated in one place. + +## When the rule does not apply + +A test fixture, a debug script, or a build-time tool that runs once +is allowed to read `process.env` directly. The rule applies to +**code that ships in the runtime**: the library, the apps, the +shared modules. The boundary between "tooling" and "runtime" is +sharp; do not blur it. + +A Node-API integration that genuinely needs runtime global +(`globalThis.crypto.subtle`, `process.versions.node` for capability +detection) is allowed, but the read happens inside a small typed +module and the rest of the codebase imports the typed accessor. + +## Why the values stay primitives (rule 0015 carve-out) + +The `Environment` type in the example above declares +`API_KEY?: string` — a primitive. Rule 0015 says a value that +represents a domain concept should be typed as a domain-specific +type, not a primitive. The carve-out is intentional: environment +values are **runtime configuration**, not domain concepts. A +`Message` is a `Message` because the application reasons about +messages; an `API_KEY` is an opaque string the application passes +to a third party. Wrapping `API_KEY` in a branded type would +add ceremony without information — there is no second +`API_KEY`-shaped value in the codebase that the brand would +prevent the consumer from confusing it with. + +The carve-out has a sharp boundary: the moment a value crosses +the typed accessor and enters the domain, it must be converted +to the domain type (rule 0015). The accessor is the only place +where the primitive is allowed to live. + +## How to refactor a scattered read + +When you find `process.env` references in business code: + +1. List every key that appears. +2. Create `env.ts` (or `config.ts`) at the appropriate boundary + (the library, the app, the workspace). +3. Declare the environment as a literal-union type, with each key + optional and each value typed. +4. Move the reads into `env.ts`. Each read happens once. +5. Export a typed `environment` object or a typed `getEnv()` + function. +6. Replace every `process.env` reference in business code with the + import. + +The refactor is mechanical and reviewable in one PR. + +## Enforcement + +- **Code review**. A reviewer who sees `process.env` in a business + file (anything under `src/` that ships at runtime) blocks the PR. +- **Grep gate**. A standing check before release: `grep -r +"process\.env" src/` returns only the accessor module. If any + other file shows up, the release is blocked until the references + are migrated. +- **CI lint** (future). A custom rule or `no-restricted-syntax` + can flag `process.env` access outside the accessor module. The + rule's existence is the enforcement signal even before it is + automated. + +## Exceptions + +A file that **defines** the accessor (the file the rule says is the +only place `process.env` is read) is allowed to access it. That +file is the rule, not the exception. + +## See also + +- **Rule 0006** — Technology Choices: the rule that motivates + avoiding `@types/node` as a runtime dependency. This rule is the + operational form of "minimal dependencies, typed boundaries". +- **Rule 0008** — No Chained Type Assertions: the type discipline + this rule relies on. A typed accessor that required an `as +unknown as Environment` to construct is a violation of 0008; the + accessor module is the only file that legitimately narrows the + ambient `process` shape. +- **Rule 0015** — Domain-Specific Types Over Primitives: rule + 0015 requires domain concepts to be domain types. Environment + values are configuration, not domain concepts — they remain + primitives inside the accessor (see "Why the values stay + primitives" above). The moment a value crosses the accessor + and enters the domain, rule 0015 applies in full. + +## Sources + +This rule is a synthesis of the project's own architectural +commitments. The pattern of a single typed accessor module is +common in Node.js backends (NestJS ConfigService, Vite's +`loadEnv`, Next.js env validation via `@t3-oss/env-nextjs`); the +project does not adopt any of these libraries directly because +the rule's discipline is one line of code, not a dependency. +The rule is the lightweight version of a pattern those libraries +formalise; the formalisation is left to the rule itself. diff --git a/docs/engineering/architecture/rules/0011-filename-kebab-case.md b/docs/engineering/architecture/rules/0011-filename-kebab-case.md new file mode 100644 index 0000000..3470063 --- /dev/null +++ b/docs/engineering/architecture/rules/0011-filename-kebab-case.md @@ -0,0 +1,210 @@ +# 0011 — Filenames Are kebab-case + +**Status**: Active (enforced through code review and CI lint). +**Date**: 2026-08-11. + +## Rule + +Every file in this repository is named in **kebab-case**: lowercase +letters, digits, and hyphens. No spaces, no underscores, no +uppercase, no camelCase, no PascalCase. + +The rule applies to: + +- Source files (`.ts`, `.tsx`, `.js`, `.mjs`). +- Test files. +- Configuration files (when the tool allows the name). +- Documentation files (`.md`, `.mdx`). +- Directory names. +- Asset filenames. + +The rule applies to **filenames**. The contents of a file are free +to use whatever convention the language requires: a `.ts` file may +export `PascalCaseComponent`, a `.tsx` file may declare +`PascalCaseComponents`. The boundary is the name on disk. + +## Why + +A consistent filename convention removes a category of decisions +that the contributor does not need to make. Every commit, every PR, +every grep, every file-listing in a tool reads the same way. The +casing of a file tells the reader nothing about its content, but +the **inconsistency** of casing tells the reader that someone made +a choice that did not need to be made. + +Kebab-case is chosen because: + +- It is the convention the broader JavaScript ecosystem uses for + filesystem names (Next.js, Vite, many build tools emit kebab-case + routes by default). +- It survives cross-platform case-insensitive filesystems (macOS, + Windows) without ambiguity. A file named `ErrorFactory.ts` and + one named `errorFactory.ts` are the same file on a case-insensitive + filesystem. +- It composes with the file-separation rule (0002) and the + file-placement rule (0003): a concern folder reads as a list of + kebab-case nouns, each describing what the file does. + +## What the rule forbids + +- **camelCase**: `errorFactory.ts`, `errorHandler.ts`. +- **PascalCase**: `ErrorFactory.ts`, `ErrorHandler.ts`. Even if the + file exports a `PascalCase` symbol, the filename stays kebab-case. +- **snake_case**: `error_factory.ts`. Underscores are forbidden. +- **Mixed case in one file**: `Error-factory.ts` is not kebab-case. +- **Uppercase abbreviations**: `URLParser.ts` becomes `url-parser.ts`. + +## What the rule allows + +- **Numbers** in the filename, between hyphens: `http2-server.ts`, + `error-codes-1.ts`. Numbers are part of the name, not a separator. +- **Version-style numbers without hyphens**: `v2-handler.ts` is + fine; `v2handler.ts` is not. +- **Test files** that match the source file they test with a + `.test.ts` suffix: `error-factory.ts` is tested by + `error-factory.test.ts`. The base name stays kebab-case. +- **Index files**: `index.ts` is the single exception. It is the + convention every bundler, every import path, and every language + module system recognises. The filename `index.ts` is a + vocabulary word, not a casing choice. +- **Filesystem-mandated names**: `.gitignore`, `.npmrc`, + `eslint.config.js`, `tsconfig.json`, `package.json`. These names + are dictated by the tools that consume them, not by the project. + They are not subject to the rule. +- **Documentation files in this very rules folder**: the rules + themselves use `NNNN-kebab-case-slug.md` as filenames, including + the digit prefix and the kebab-case slug. The rule applies to its + own naming; the rule does not exempt itself. + +## How to fix a wrong-cased filename + +When you rename a file to fix its case, two things happen: + +1. The file appears as a deletion and an addition in `git status` + even though the content is unchanged. This is correct: git tracks + case-sensitive differences on case-insensitive filesystems. +2. Imports of the file (if any) must be updated in the same PR. + Stale imports will not compile. + +A rename PR is therefore one commit that: + +- Renames the file with `git mv` (or equivalent). +- Updates every import site in the same commit. +- Runs the test suite to confirm nothing is broken. + +If the file is renamed multiple times across the history, a `git +log --diff-filter=R` can surface the rename chain. The current +filename is what the rule cares about; history is not rewritten. + +## What this looks like in violation + +Three shapes that this rule exists to catch. + +The first, common: + +``` +src/ +├── errorFactory.ts # camelCase +├── ErrorHandler.ts # PascalCase +└── error_factory.ts # snake_case +``` + +Three files, three conventions, all in the same directory. A reader +who greps for `error` sees hits in three different cases; their +mental model has to track which is which. + +The second, mixed: + +``` +src/ +├── apiClient.ts # camelCase +├── http-client.ts # kebab-case +├── HttpServer.ts # PascalCase +└── http_server.ts # snake_case +``` + +The same project, four files, four conventions. Each was probably +written by a different author at a different time. The drift tells +the reader that no one is reading the existing names before adding +new ones. + +The third, case-insensitive-filesystem trap: + +A file `ErrorFactory.ts` is created on macOS or Windows. The next +contributor, on Linux, creates `errorFactory.ts`. Git tracks both. +On macOS, the second contributor sees only one file because the +filesystem sees them as the same. The merge conflict appears only +when someone tries to checkout on Linux. The fix is to enforce +kebab-case at the rule level so the trap is impossible to enter. + +**Bad** — three folders, three conventions, same project: + +``` +src/ +├── errorFactory/ # camelCase +│ └── index.ts +├── ErrorHandler/ # PascalCase +│ └── index.ts +└── http_client/ # snake_case + └── index.ts +``` + +A grep for `error` returns hits in three different cases. A file +listing reads as three unrelated projects. The reader's mental model +has to track which casing means which. + +**Good** — one folder per concern, kebab-case throughout: + +``` +src/ +├── error-factory/ # kebab-case +│ └── index.ts +├── error-handler/ # kebab-case +│ └── index.ts +└── http-client/ # kebab-case + └── index.ts +``` + +A grep for `error` returns hits in one case. A file listing reads +as one project. The reader's mental model is uniform; the casing +does not need to be learned. + +## Enforcement + +- **Code review**. A reviewer who sees a non-kebab-case filename + blocks the PR. The fix is `git mv`, not "I'll fix it later". +- **CI lint** (existing). The existing lint workflows catch + obvious casing inconsistencies in the diff. The rule is the + review-time signal that those catches are doing real work. +- **Quarterly audit**. A standing review of "which files in this + repo have non-kebab-case names?" surfaces the candidates that + slipped through. Each is a one-commit rename PR. + +## Exceptions + +The exceptions are **vocabulary words, not casing choices**: + +- `index.ts` in any directory. +- Tool-mandated names: `.gitignore`, `.npmrc`, `.editorconfig`, + `.prettierignore`, `.eslintrc.*`, `.husky`, `.lintstagedrc.*`, + `tsconfig*.json`, `vitest.config.ts`, `next.config.*`, + `package.json`, `pnpm-*.yaml`, `.changeset/*.md` (changeset + filenames are dictated by the changesets tool). +- Branded filenames that the tool requires (none today; revisit if + one shows up). + +The exceptions are listed because a reviewer should know what is +not subject to the rule. They are not loopholes; they are +constraints from tools the project depends on. + +## Sources + +This rule is a synthesis of the project's own working +experience. The casing choice (kebab-case) is the convention +the broader JavaScript ecosystem follows (Next.js, Vite, most +build tools emit kebab-case routes by default); the rule is +not anchored to a single external reference because the +convention is universal enough that citing one would imply the +others are wrong. The case-insensitive-filesystem trap is a +Git behaviour; the rule documents the failure mode without +attributing it. diff --git a/docs/engineering/architecture/rules/0012-prefer-type-over-interface.md b/docs/engineering/architecture/rules/0012-prefer-type-over-interface.md new file mode 100644 index 0000000..2bd0052 --- /dev/null +++ b/docs/engineering/architecture/rules/0012-prefer-type-over-interface.md @@ -0,0 +1,267 @@ +# 0012 — Prefer `type` Over `interface` + +**Status**: Active (enforced through code review). +**Date**: 2026-08-11. + +## Rule + +Every shape in this codebase is declared with `type`, not +`interface`, unless one of three conditions is met: + +1. **Declaration merging is required.** A consumer extends the + shape by adding fields to a second `interface X { ... }` + declaration in a different file. Only `interface` supports + this; `type` does not. +2. **A class implements the shape and the shape has no runtime + behaviour.** `class Foo implements Shape { ... }` works with + both, but a class that needs the shape to be **open** for + third-party additions (a library extension point) is the + natural fit for `interface`. +3. **The shape is part of a host type the project does not own.** + Augmenting a third-party `interface` (e.g. extending a host + framework's request type) uses `interface` because the host + already chose `interface`. + +In every other case — a shape that describes a value, a union, an +intersection, a function signature, a conditional, a mapped type — the +declaration is `type`. The shape of the code becomes uniform; the +choice between `type` and `interface` stops being made on every +declaration. + +## Why + +`type` and `interface` overlap in the cases most codebases use them. +For a plain object shape, both compile to the same structural type; +both have the same IDE support; both produce the same error +messages on misuse. The choice between them is not a type-system +choice; it is a **convention** choice. + +The convention this rule picks is `type`, for three reasons: + +- **`type` is more expressive.** `type` accepts unions, + intersections, conditionals, mapped types, and template literal + types. `interface` accepts unions only with `&` and only for + object shapes. A codebase that uses `type` uniformly never has + to reach for `interface` when the shape needs an expression + `type` cannot write. +- **`type` is one declaration site.** An `interface X` declared in + two files is a single shape that the compiler merges. A `type X` + declared twice is an error. The merge is occasionally useful + (declaration merging for host augmentation) and frequently a + source of "where did this field come from?" bugs. Defaulting to + `type` makes merge a deliberate choice, not an accident. +- **`type` is the union's natural home.** A codebase that mixes + unions and interfaces has to remember that `interface X extends +Y | Z` is invalid; the syntax switches between the two. A + codebase that uses `type` uniformly has one syntax for "shape" + and one syntax for "either this or that". + +The rule is not "no `interface` ever". The rule is "the choice +between the two is not yours to make on every declaration. The +default is `type`. The exception list is short, named, and audited." + +## What this looks like in practice + +A declaration that should be `type`: + +```ts +// Good — the shape is a value description, not an extension point +type ValidationRule = { + readonly field: string; + readonly severity: 'error' | 'warning'; + readonly code: string; +}; +``` + +A declaration that must be `interface` because declaration merging +is the point: + +```ts +// Required: third-party host augmentation +declare module 'express' { + interface Request { + requestId: string; + } +} +``` + +A declaration that must be `interface` because a library exposes +an extension point: + +```ts +// The library author chose interface deliberately — they expect +// consumers to add fields by declaring the interface again in +// their own module. +interface PluginContext { + config: Record; + logger: Logger; +} +``` + +The library author writes `interface` because the consumer might +add fields via declaration merging. The consumer writes `type` for +their own shapes, because they control their own shapes. + +## When the rule does not apply + +The rule applies to **shape declarations**. It does not apply to: + +- **Class declarations** themselves. `class Foo { ... }` is not + affected by the rule; the rule is about declaring the shapes + classes implement or the unions and intersections they + participate in. +- **Type assertions** of host types. The shape of the ambient + declaration is fixed by the host. +- **Build-time tooling** that requires one or the other for + configuration (rare, but some tool configs use `interface X` as a + literal label). +- **Augmentation files** (the `*.d.ts` files that extend host + types). Augmentation is `interface`; this is the canonical + exception. + +## How to convert an `interface` to a `type` + +When a contributor has written `interface` and the rule says `type`, +the conversion is mechanical: + +```ts +// Before +interface ValidationRule { + readonly field: string; + readonly severity: 'error' | 'warning'; +} + +// After +type ValidationRule = { + readonly field: string; + readonly severity: 'error' | 'warning'; +}; +``` + +The conversion is lossless for object shapes that do not use +declaration merging. The compiler emits the same structural type; +the IDE shows the same hints; the consumers see no change. + +A conversion that requires more than a keyword swap is a signal that +the original `interface` was doing something `type` cannot do. The +signal is not a failure to convert; it is the rule's way of telling +the contributor "this `interface` is on the exception list; document +why". + +## What this looks like in violation + +Three shapes that this rule exists to catch. + +The first, mixed convention: + +```ts +// File 1 +interface User { + id: string; + email: string; +} + +// File 2 +type Admin = { + id: string; + permissions: string[]; +}; + +// File 3 +interface Config { + env: 'production' | 'staging'; +} + +// File 4 +type FeatureFlag = { + name: string; + enabled: boolean; +}; +``` + +The same project, four files, four declarations, two conventions. +The reader has to remember which keyword was used in which file +when they want to add a field. + +The second, `interface` used where the shape is closed: + +```ts +// Bad — the shape is closed; no third party extends it +// The author chose interface because that was the example they +// followed, not because they wanted declaration merging. +interface UserSettings { + theme: 'light' | 'dark'; + language: string; +} +``` + +A `type` is the right shape. A future contributor who wants to +add a field finds `type UserSettings = { ... }` and edits the +shape in one place. + +The third, `type` used where `interface` is required: + +```ts +// Bad — third-party host augmentation with type +// (TypeScript will accept this in some configurations but +// declaration merging does not work as expected.) +declare module 'express' { + type Request = { + requestId: string; + }; +} +``` + +The augmentation does not merge into the host `Request`. The +consumer's `requestId` field is invisible to library code that +expects an augmented `Request`. The fix is `interface`. + +## Enforcement + +- **Code review**. A reviewer who sees `interface X` in a file that + does not perform declaration merging, class implementation of an + open shape, or third-party augmentation asks for the `type` + conversion. +- **Lint rule** (future). A custom ESLint rule can flag `interface` + declarations outside the canonical exception list. The rule's + existence is the enforcement signal even before it is automated. +- **Quarterly audit**. A standing review of "where do we still use + `interface`?" surfaces the candidates that slipped through. Each + is either converted or annotated as a documented exception. + +## Exceptions + +The rule is absolute except for the three conditions listed at the +top: declaration merging, class implementation of open shapes, and +host type augmentation. Each `interface` declaration in the codebase +should be traceable to one of those three conditions. A +declaration that cannot be traced is a violation. + +## See also + +- **Rule 0002** — File Separation: a `types.ts` file is the + natural home for the `type` declarations this rule produces. +- **Rule 0008** — No Chained Type Assertions: the type discipline + this rule relies on. A `type` declaration that requires a cast + to use is a violation of 0008; the declaration is the wrong + shape. +- **Rule 0014** — Functions Over Classes for Public API: the + exception 2 above ("a class implements the shape and the shape + has no runtime behaviour") describes a class that _implements_ + an `interface`; that class must remain non-exported per rule 0014. The public shape is the interface; the class is the + internal implementer. The two rules cooperate: 0012 picks + `interface` for the extension point, 0014 hides the class that + implements it. + +## Sources + +This rule is a synthesis of the project's own working +experience. The TypeScript documentation itself states that +`type` and `interface` are largely interchangeable in the cases +most codebases use them; the rule picks `type` as the default +because the expressiveness, single-declaration-site, and +union-friendly characteristics are not duplicated by +`interface`. The three documented exceptions (declaration +merging, class implementation of open shapes, host type +augmentation) are the cases where `interface` is genuinely +required. diff --git a/docs/engineering/architecture/rules/0013-entity-first-naming.md b/docs/engineering/architecture/rules/0013-entity-first-naming.md new file mode 100644 index 0000000..1074b5c --- /dev/null +++ b/docs/engineering/architecture/rules/0013-entity-first-naming.md @@ -0,0 +1,344 @@ +# 0013 — Entity-First Naming: Refuse Bare `-er` Suffixes + +**Status**: Active (enforced through code review). +**Date**: 2026-08-11. + +## Rule + +A class, type, or module name is **never** a bare job title. The +suffixes `Manager`, `Service`, `Handler`, `Controller`, `Helper`, +`Writer`, `Reader`, `Converter`, `Validator`, `Router`, +`Dispatcher`, `Observer`, `Listener`, `Sorter`, `Encoder`, +`Decoder`, and every other `-er` ending that names what the thing +**does for the caller** rather than what the thing **is** — +are refused as standalone names. + +The rule is against suffixes as **the only content of a name** AND +against suffixes that survive qualification. A `Handler` is refused +because the name says only that something is being handled; a +`CancelOrderHandler` is also refused, because the suffix still +describes the **role the thing plays in someone else's code**, +not the thing itself. The qualifier does not change the smell; it +only makes the smell larger and harder to spot. + +Two patterns, in increasing order of severity: + +- **Suffix as name content** (`Manager`, `Service`, `Handler`, + `CancelOrderHandler`, `UserCreationService`) — refused. The + suffix is part of the name, with or without a qualifier. The + name describes a role, not an entity. +- **Entity name** (`SortedApples`, `ValidatedPayload`, + `CancellationRequest`) — the only accepted shape. The name + describes what the thing **is**, not what it does for the + caller. + +The rule accepts the entity shape only. The suffix shape, with +or without a qualifier, is the smell this rule exists to catch. + +## Why + +A bare job title is a confession that the author could not name +the thing they were building. The thing exists; the author wrote +it. But the name they gave it is the name of the **role the thing +plays in someone else's code**, not the name of the thing itself. +The author outsourced the naming to the caller. + +This is the same anti-pattern as a function called `doStuff`, +applied at the type level. The reader who meets the type for the +first time cannot tell what it represents, only what it does for +the system that uses it. The reader has to read the callers to +recover the entity, when the name should have done that work. + +The deeper problem is **diffusion of responsibility**. A class +named `Manager` is a class to which any method can be added +without breaking its name. A class named `OrderCancellationService` +is a class to which only order-cancellation methods can be added +without breaking its name. The first grows; the second stays +focal. The smell is not aesthetic; it is a measurement of how +much scope a class is allowed to absorb. + +## What this looks like in violation + +The first shape, bare job title: + +```ts +// Bad — what does this manage? +class UserManager { + createUser(input: CreateUserInput): User { + /* ... */ + } + updateUser(id: string, input: UpdateUserInput): User { + /* ... */ + } + deleteUser(id: string): void { + /* ... */ + } + authenticateUser(credentials: Credentials): Session { + /* ... */ + } + sendPasswordResetEmail(email: string): void { + /* ... */ + } + generateUserReport(filters: ReportFilters): Report { + /* ... */ + } + // ... and so on, indefinitely. +} +``` + +Six unrelated responsibilities under one name. The next contributor +who adds a method asks "where does it go?" and the answer is +"UserManager". The class grows until it is unmanageable. + +The second shape, qualified job title, is the shape the rule +**refuses**. It is mentioned here only because it is what most +codebases reach for as the "smaller" compromise: + +```ts +// Refused — the suffix still describes a role, not an entity. +// The qualifier makes the smell larger, not smaller. +class OrderCancellationHandler { + handle(command: CancelOrderCommand): CancellationResult { + /* ... */ + } +} +``` + +The qualifier does not turn a role into an entity. The class is +still named for what it does for the caller (`Handler`), not for +what it is. A reader who meets `OrderCancellationHandler` learns +that there is a handler; they still have to read the body to +discover that the handler is the cancellation. + +The right shape: + +```ts +// The name describes what the thing is. +// The class stays internal — see rule 0014. +class OrderCancellation { + cancel(command: CancelOrderCommand): CancellationResult { + /* ... */ + } +} + +// Or, more idiomatic in functional code: +type OrderCancellation = (command: CancelOrderCommand) => CancellationResult; +``` + +The class is the **thing**; the method is the **operation on the +thing**. The reader does not need to know who is using it. _The +class above is internal; rule 0014 forbids exporting it. The +public shape is a factory function (`createOrderCancellation`) +or, more often, a type alias and a free function. The example +here shows the naming pattern; rule 0014 shows the public-API +pattern._ + +## When the rule does not apply + +The rule applies to **shape names** — the name of a class, a type, +a module, a service handle. It does not apply to: + +- **Variable names** that hold an instance briefly. `const +manager = new OrderCancellationHandler();` is acceptable; the + variable is scoped to one expression and the type name carries + the focal responsibility. +- **Test names**. `UserManagerTest` is acceptable as a test class + name when the type under test is `UserManager`; the test name + mirrors the type name. Refusing the test name would create a + useless indirection. +- **Build-time tooling**. Generated code, vendor bindings, and + frameworks where the shape is fixed by the other side. + +## How to refactor a suffix-bearing name + +When a contributor has written `Manager`, `Service`, `Handler`, +or any qualified version (`CancelOrderHandler`, +`UserCreationService`), the refactor has two steps, in order: + +1. **Identify the entity.** The class is doing something for the + caller. The thing it operates on, or the thing it **is**, is + the entity. A `CancelOrderHandler` is the cancellation; the + qualifier already names it. A `UserCreationService` is the user + creation; the qualifier already names it. The entity is in + the qualifier; the suffix is what is removed. +2. **Rename to the entity, drop the suffix.** + `CancelOrderHandler` → `OrderCancellation`. `UserCreationService` + → `UserCreation`. The method on the entity is the action + (`cancel`, `create`). + +There is no "smallest change" path that keeps the suffix. The +suffix is the smell; removing it is the change. If the class is +too small to deserve its own entity name, the qualifier is +replaced by the action: `cancelOrder(command: CancelOrderCommand)` +is a function on the `OrderCancellationService` — but the function +itself does not need a wrapper class; it is a function. + +The refactor in three steps when the class is large enough: + +1. Split into one entity per focal responsibility. +2. Each entity exposes one operation (`cancel`, `create`, + `validate`). +3. The suffix goes away with the wrapper class. + +## What this looks like in violation — the focused case + +A class that has the right name but the wrong shape: + +```ts +// Bad — the name says one thing, the methods do many. +class OrderCancellation { + cancel(command: CancelOrderCommand): CancellationResult { + /* ... */ + } + refund(orderId: string, amount: Money): RefundResult { + /* ... */ + } + notifyCustomer(orderId: string): void { + /* ... */ + } + generateCancellationReport(filters: ReportFilters): Report { + /* ... */ + } +} +``` + +The name promises one responsibility; the body delivers four. +The right move is to split. Each method becomes its own entity +or its own command. The class `OrderCancellation` then has only +the `cancel` method; the others live in their own types. + +## What senior practitioners say + +> "Manager. Controller. Helper. Handler. Writer. Reader. +> Converter. Validator. Router. Dispatcher. Observer. Listener. +> Sorter. Encoder. Decoder. This is the class names hall of shame. +> Have you seen them in your code? In open source libraries +> you're using? In pattern books? They are all wrong. What do +> they have in common? They all end in '-er.' And what's wrong +> with that? They are not classes, and the objects they +> instantiate are not objects. Instead, they are collections of +> procedures pretending to be classes." +> +> — Yegor Bugayenko, _Don't Create Objects That End With -ER_, +> March 2015. + +Bugayenko's diagnosis is the philosophical ground of the rule. +A bare job title is a class that has no entity to be; it is a +collection of procedures that the caller orchestrates. The shape +exists; the entity does not. + +> "If a class only has a single responsibility it will be pretty +> difficult to attach a Manager suffix to the class name." +> +> — Scott Muc, _Manager Suffixes Are a Code Smell_, June 2008. + +Muc's observation operationalises Bugayenko. The suffix is a +proxy measurement for responsibility count. When the suffix is +necessary, the count is high. + +> "Six methods each with around five unit tests: that's 30 unit +> tests, probably all in the same file name +> 'CartServiceTest'. That's beginning to be a lot harder to +> manage. [...] Every class should be a noun and every method +> should be a verb." +> +> — Charles-H lavoie, _"Service" should be a banned word_, Proper +> Code, June 2019. + +Lavoie quantifies the cost. A bare `Service` accumulates methods +faster than it accumulates tests. The total cost of the class is +the methods times the tests; the suffix makes the cost invisible. + +> "A command handler's job is to coordinate the execution of a +> command. It's named for the command, not the entity. The +> handler is named `CancelOrderHandler`, not `OrderHandler`." +> +> — Jimmy Bogard, _Domain Command Patterns - Handlers_, +> March 2018. + +Bogard's position is the most permissive of the four sources. The +rule does not follow Bogard; the rule follows Bugayenko. The +qualifier-plus-suffix shape Bogard recommends is, in this rule's +reading, still a role-naming convention: `CancelOrderHandler` +tells the reader what the thing does for the caller, not what +the thing is. The rule's stance is that a class which is +described by what it does for the caller should be a function +on the entity it operates on, not a wrapper class. The +qualifier is the entity; the suffix is the wrapper. + +The four voices still converge on the **diagnosis** — the suffix +is a smell — even when they disagree on the **threshold**. The +rule picks the strictest threshold: no suffix, with or without a +qualifier. The other positions describe intermediate shapes +project contributors may reach for during a refactor; the rule +captures the shape the project is moving toward. + +## Enforcement + +- **Code review**. A reviewer who sees a bare `-er` suffix in a + class or type name (`UserManager`, `PaymentService`, + `EventHandler`) blocks the PR and asks for a qualifier, a split, + or an entity rename. +- **Naming audit**. A standing review of "which classes in this + codebase have a bare `-er` suffix?" surfaces the candidates that + slipped through. Each is a refactor candidate, not a backlog + item. +- **Lint rule** (future). A custom ESLint rule can flag bare + `-er` suffixes in class and type declarations. The rule's + existence is the enforcement signal even before it is automated. + +## Exceptions + +A test class name (`UserManagerTest`) is permitted because it +mirrors the type under test. The test class is not the entity; it +is the verification of the entity. Renaming `UserManagerTest` to +`UserTest` while the type under test is still `UserManager` would +create a useless indirection; the rename is the responsibility of +the type rename, not the test rename. + +A variable that holds an instance briefly (`const manager = ...`) +is permitted. The type name carries the focal responsibility; the +variable name is local. The rule refuses type names, not +variable names. + +Build-time tooling, generated code, vendor bindings, and +frameworks where the shape is fixed by the other side are +permitted. The rule applies to code the project writes, not to +code the project consumes. + +## See also + +- **Rule 0002** — File Separation: a class with bare `-er` is + usually a class that mixes types and operations; the rule on + file separation makes that mixing visible. +- **Rule 0007** — Top-Down Composition: the same principle at + the function level. A function called `processData` is the + function-level equivalent of a `DataManager` class. The + principle is the same — name the thing, not the job. +- **Rule 0011** — Filenames Are kebab-case: a class named + `UserManager` usually lives in a file named `user-manager.ts`, + which is the right casing. The rule's wrong shape is the class + name, not the file name. + +## Sources + +- **Bugayenko, Yegor.** _Don't Create Objects That End With + -ER._ March 2015. The philosophical ground: a bare job title + is a class that has no entity to be; it is a collection of + procedures that the caller orchestrates. The hall-of-shame + list (Manager, Controller, Helper, Handler, Writer, Reader, + Converter, Validator, Router, Dispatcher, Observer, Listener, + Sorter, Encoder, Decoder) is the rule's reference list. +- **Muc, Scott.** _Manager Suffixes Are a Code Smell._ June 2008. + The operational reading: a suffix is a proxy measurement for + responsibility count. When the suffix is necessary, the count + is high. +- **lavoie, Charles-H.** _"Service" should be a banned word._ + Proper Code, June 2019. The quantified cost: six methods, + thirty unit tests in one file. The bare suffix makes the cost + invisible. +- **Bogard, Jimmy.** _Domain Command Patterns - Handlers._ + March 2018. The counter-example: the handler suffix is fine + when qualified by the command it handles. The rule does not + follow Bogard; the rule follows Bugayenko. The qualifier is + the entity; the suffix is the wrapper. diff --git a/docs/engineering/architecture/rules/0014-functions-over-classes-for-public-api.md b/docs/engineering/architecture/rules/0014-functions-over-classes-for-public-api.md new file mode 100644 index 0000000..8f264a3 --- /dev/null +++ b/docs/engineering/architecture/rules/0014-functions-over-classes-for-public-api.md @@ -0,0 +1,382 @@ +# 0014 — Functions Over Classes for Public API + +**Status**: Active (enforced through code review). +**Date**: 2026-08-11. + +## Rule + +The public API of this codebase exposes **functions**, not +classes. A class is a **detail of internal implementation** that +the consumer never instantiates, never inspects with `instanceof`, +never extends, and never imports by name. + +Concretely: + +- **An export is a function** (or a type, or an `Object.freeze`'d + value). A class is **not** an export. +- **A consumer creates an entity by calling a function**: + `const group = createGroup(...)`, not `const group = new Group(...)`. +- **The consumer sees the type of the constructed entity**, not + the class. The class is a private symbol; the type is public. + +This rule applies to **public API**. Inside a module, classes are +permitted when they are the right shape for state — a `Stack`, an +`Error`, an `Event`, a `Group` — because the class encapsulates +mutable state more cleanly than a closure. The rule is about what +**crosses the module boundary**. + +## Why + +A class is a **template** for an object. The consumer has the +template in hand. With the template, the consumer can: + +- Create other instances by `extends`, with logic the author did not + anticipate. +- Override methods by inheritance, replacing behaviour the author + tested. +- Reach into private state with `as any` casts, breaking the + invariants the author maintained. +- Couple their code to method names the author may want to rename + in a future version. + +Each of these is a freedom the consumer did not need and the +author did not want to grant. Every API surface is a contract; a +class is a contract that includes the freedom to break it. + +A function is a contract that does not include those freedoms. +The consumer calls it; they get back a value of a public type; +they cannot reach into the implementation. The function is the +boundary. The class, if any, is behind it. + +## What this looks like in practice + +The bad shape, class as public API: + +```ts +// Bad — the consumer can `new Group(...)`, extend it, override its +// methods, or cast it to access private state. +export class Group { + private members: Member[] = []; + + add(member: Member): void { + this.members.push(member); + } + + // ... +} + +export function group(): Group { + return new Group(); +} +``` + +The consumer receives a `Group` reference. They can do +`new Group()`, `class MyGroup extends Group`, `group() as any` to +access `members`. Every API change risks breaking them. The +factory function adds nothing here; the class is the API. + +The right shape, function as public API: + +```ts +// Good — the consumer sees `Group` as a type, not a class. +// They cannot instantiate it, extend it, or reach into it. +class Group { + #members: Member[] = []; + + add(member: Member): void { + this.#members.push(member); + } + + // ... +} + +export type Group = ReturnType; + +export function createGroup(): Group { + const instance = new Group(); + // ... + return instance; +} + +// Or: `function group(): Group` as the public constructor. +``` + +The consumer sees `Group` as a type. They call `createGroup()` or +`group()` and get back a value of that type. The class itself is +not exported; the consumer cannot `new Group()` because the symbol +is not in scope. The factory function is the **only** entry point. + +## When the rule does not apply + +The rule applies to **exports**. It does not apply to: + +- **Internal classes** within a module. A class that is created + inside a file and only ever returned through a factory function + is fine. +- **Classes that are genuinely host types** — when augmenting a + host type (Express request, Error subclass), the host already + chose `class`; the project follows. +- **Built-in classes** — `Error`, `Map`, `Set`, `Date`, `URL`, + `URLSearchParams`. These are language-standard classes; the rule + does not apply. +- **Test files**, where a class is the natural shape for a test + fixture. +- **Frameworks that require classes** — a decorator-based framework + where the @Injectable() pattern requires a class is not negotiable; + the framework dictates the shape. + +## Why a class at all, then? + +A class is the right shape for **mutable state with a clear +identity**. A `Stack` that supports `push`, `pop`, and `peek` +has state (the items) and identity (the order). A closure-based +factory that returns `{ push, pop, peek }` works but the state is +buried in a closure the reader has to mentally unwrap. A class +makes the state visible. + +A class is also the right shape for **inheritance the project +controls**. The project owns the class; no consumer extends it; +the class exists to give the project a clear shape for state and +behaviour. The factory function is the boundary; the class is +behind it. + +A class is **not** the right shape for: + +- A pure function (no state). +- A pure value (no behaviour). +- A namespace of related functions (use a module). +- An API the consumer is expected to instantiate, extend, or + customise. + +## The factory function pattern + +A factory function in this codebase has three properties: + +1. **It is the only export that constructs the entity.** A + consumer cannot reach the class through any other path. +2. **It returns a typed value.** The return type is the public + shape; the class is hidden. +3. **It does not leak the class symbol.** The class is declared + inside the file or inside a private submodule. It is not + re-exported. It is not referenced in the public types. + +Example: + +```ts +// group.ts + +// Internal: never exported. +class GroupImpl { + #members: Member[] = []; + + add(member: Member): void { + this.#members.push(member); + } +} + +// Public: the type the consumer sees. +export type Group = { + add(member: Member): void; +}; + +// Public: the only constructor. +export function group(): Group { + const impl = new GroupImpl(); + return { + add: impl.add.bind(impl), + }; +} +``` + +The consumer imports `group` (the function) and `Group` (the +type). They cannot import `GroupImpl` because it is not exported. +They cannot `new Group()` because `Group` is a type, not a class. +They cannot extend the implementation because they have no +reference to it. + +For richer entities, the implementation may export a class name +that is the public **type** while keeping the constructor +private. TypeScript supports this pattern: + +```ts +class GroupImpl { + // ... +} + +// Public type: same name, no runtime entity. +// Consumers see Group as the type, not the constructor. +export type Group = GroupImpl; + +// Public constructor. +export function group(): Group { + return new GroupImpl(); +} +``` + +Here `Group` is a type alias to `GroupImpl`. The consumer sees the +type but cannot construct it directly because `GroupImpl` is not +exported. The constructor is `group()`, not `new Group()`. + +## What this looks like in violation + +Three shapes that this rule exists to catch. + +The first, class as the only export: + +```ts +// Bad — the consumer imports the class and instantiates it. +export class ErrorHandler { + // ... +} +``` + +The consumer can `new ErrorHandler(...)`, extend it, override +methods. Every API change risks breaking them. + +The second, class plus factory, but the class is also exported: + +```ts +// Bad — the factory is just sugar; the class is still reachable. +export class ErrorHandler { + // ... +} +export function createErrorHandler(): ErrorHandler { + return new ErrorHandler(); +} +``` + +The consumer can still `import { ErrorHandler }` and `new +ErrorHandler(...)`. The factory adds an entry point; it does not +remove the old one. The fix is to drop `export` from the class. + +The third, class exported for "convenience": + +```ts +// Bad — the author thought the consumer would want both. +// They don't. Pick one (the function). +export class Group { + // ... +} +export const createGroup = (): Group => new Group(); +``` + +Two paths to the same thing. The consumer uses one or the other; +both ship; both are supported. The fix is to drop the class +export and the constructor becomes the function. + +## What senior practitioners say + +> "Resist making classes your public API. [...] You can always +> hide your classes behind the factory functions. If you expose +> them, people will inherit from them in all sorts of ways that +> make zero sense to you, but that you may break in the future." +> +> — Dan Abramov, _How to Use Classes and Sleep at Night_, +> October 2015. + +Abamov's position is the strictest among the senior sources and +the one this rule follows. The class is a detail of internal +implementation; the API is the function. + +> "When using factory function, only the methods we expose are +> public, everything else is encapsulated." +> +> — Cristian Salcescu, _Class vs Factory function: exploring the +> way forward_, freeCodeCamp, March 2018. + +Salcescu operationalises Abamov with the encapsulation argument. +A factory function closes by default; a class opens by default. +The rule picks the closed default because the consumer never needs +the open one. + +> "Don't expect people to use your classes. Even if you choose +> to provide your classes as a public API, prefer duck typing +> when accepting inputs." +> +> — Dan Abramov, _How to Use Classes and Sleep at Night_, +> October 2015. + +The duck-typing corollary: the function's input type does not +require `instanceof ClassName`. It accepts anything that has the +methods the function calls. The class is the implementation; the +type is the contract. + +The three positions converge on the operational rule: classes +are for state, functions are for API. The rule applies the +position to this codebase's exports. + +## Enforcement + +- **Code review**. A reviewer who sees `export class` in any file + blocks the PR. The class is fine if it is internal; it is not + fine if it is exported. +- **Lint rule** (future). A custom ESLint rule can flag any + `export class` declaration. The rule's existence is the + enforcement signal even before it is automated. +- **Public API audit**. A standing review of "what classes does + this codebase export?" returns an empty list. A non-empty list + is a release-blocking finding. + +## Exceptions + +A built-in class is exempt: `Error`, `Map`, `Set`, `Date`, `URL`, +`Promise`, etc. The project does not own these; the rule does +not apply. + +A framework-mandated class is exempt: a decorator-based DI +container requires a class; the framework dictates the shape; the +rule does not fight the framework. + +A test class is exempt: tests are internal to the project; they +are not consumed by the public. + +A genuinely public type whose construction is fixed by a host +(e.g. a framework's `Request`) is exempt: the project augments +the host; the augmentation is `interface`, not `class`. + +## See also + +- **Rule 0012** — Prefer `type` Over `interface`: rule 0012 + carves out `interface` for declaration merging, open-shape + class implementation, and host augmentation. When rule 0012 + permits an `interface` because a class implements an open + shape (the "library extension point" pattern), that class is + internal per this rule; the public contract is the `interface`, + the public constructor is the factory function, the class is + behind the boundary. +- **Rule 0013** — Entity-First Naming: the factory function is + the natural name for the **action** that produces the entity + (`group`, `createGroup`, `cancelOrder`). The class is the + entity behind the action; the action is what the consumer calls. +- **Rule 0001** — Project Mindset: invariant 9 ("optimise for + the reader") is the philosophical ground of the rule. The + reader of an exported function sees only the function's contract. + The reader of an exported class sees the contract plus the + freedom to break it. +- **Rule 0006** — Technology Choices: the function-based API + surface is one of the explicit choices the codebase commits to. + Adding a class is a change to that commitment, not a local + refactor. + +## Sources + +- **Abramov, Dan.** _How to Use Classes and Sleep at Night._ + October 2015. The position the rule follows: "resist making + classes your public API; you can always hide your classes + behind factory functions; if you expose them, people will + inherit from them in ways that make zero sense to you but + that you may break in the future." Abramov's point is the + strictest among senior practitioners; the rule adopts it. +- **Salcescu, Cristian.** _Class vs Factory function: exploring + the way forward._ freeCodeCamp, March 2018. The + encapsulation argument: a factory function closes by default, + a class opens by default. The rule picks the closed default. +- **Abramov, Dan.** _How to Use Classes and Sleep at Night._ + Cited again for the duck-typing corollary: don't expect people + to use your classes; prefer duck typing when accepting + inputs. +- **MobX Cookbook.** _Classes VS Functions for Stores._ The + practical confirmation that the factory function pattern + works in TypeScript: `ReturnType` infers the + type from the factory, preserving DRY between class and + type. diff --git a/docs/engineering/architecture/rules/0015-domain-specific-types.md b/docs/engineering/architecture/rules/0015-domain-specific-types.md new file mode 100644 index 0000000..3e83f80 --- /dev/null +++ b/docs/engineering/architecture/rules/0015-domain-specific-types.md @@ -0,0 +1,435 @@ +# 0015 — Domain-Specific Types Over Primitives + +**Status**: Active (enforced through code review). +**Date**: 2026-08-11. + +## Rule + +Every value that represents a **domain concept** is typed as a +**domain-specific type**, not as a primitive. A `Message` is a +`Message` with a `content` field, a `type` field, and whatever +fields the domain grows; it is **not** a bare `string`. A `UserId` +is a `UserId` with whatever fields a user-id carries; it is **not** +a bare `string`. + +The rule applies to **all values that cross a module boundary** +or that participate in **more than one function**. A local variable +inside a one-line lambda may be a primitive; anything the consumer +will see, type, or extend is a domain-specific type. + +A primitive is permitted at the **boundary** where a value first +enters the system (parsing JSON, reading an env var, accepting +foreign input). The conversion from primitive to domain type +happens at that boundary; the rest of the codebase sees the domain +type only. + +## Why + +A primitive is **typeless at the semantic level**. `string` is +every string at once; `number` is every number at once. The +compiler cannot tell a message from a username, a percentage from +a count, a user-id from a session-id. The reader who sees +`function send(message: string, user: string)` cannot know +whether the second argument is the username, the user-id, or the +session-id — the type is the same in all three cases. The reader +has to read the body to recover the meaning. + +A domain-specific type **carries the meaning**. `function send( +message: Message, recipient: UserId)` says what each value is. The +reader does not have to guess. The compiler refuses to mix the +two: `send(message, recipient)` will not accept a username in the +second position. + +The deeper problem is **extensibility**. A `Message` with `content` +and `type` is a **shape that can grow**. A new field (`priority`, +`correlationId`, `sentAt`) is a one-line type extension that +preserves every existing caller. A bare `string` cannot grow; +adding a field means breaking every function signature. The +domain-specific type is the **stable shape** that future +contributors extend without breaking consumers. + +## What this looks like in practice + +The bad shape, primitive as a domain concept: + +```ts +// Bad — what is the difference between the two strings? +function sendNotification(message: string, recipient: string): void { + // ... +} + +sendNotification('Hello, world!', 'usr_123'); +// Is the second argument a username, an id, a phone number? +// The compiler does not know. The reader has to read the body. +``` + +The right shape, domain-specific types: + +```ts +// Good — each value is what it is. Literal unions are extracted as +// named domain types so the shape is reusable across the codebase +// and the literal strings appear in one place. +type MessageType = 'text' | 'image' | 'audio'; +type MessagePriority = 'low' | 'normal' | 'high'; + +type Message = { + readonly content: string; + readonly type: MessageType; + readonly priority?: MessagePriority; + readonly correlationId?: string; +}; + +type UserId = { + readonly value: string; +}; + +function sendNotification(message: Message, recipient: UserId): void { + // ... +} + +sendNotification({ content: 'Hello, world!', type: 'text' }, { value: 'usr_123' }); +// The compiler checks the shape. The reader does not have to guess. +``` + +The literal unions are extracted as named types (`MessageType`, +`MessagePriority`) rather than inlined because: + +- The same set of literals appears in multiple places (the type, + the validation function, the rendering function). Extracting + them once keeps the literal strings in **one place**; adding a + new value is one line in the type and the compiler enforces + that every consumer handles it. +- The name carries the meaning. A function parameter typed as + `MessageType` is more readable than one typed as the literal + union `'text' | 'image' | 'audio'`. The reader sees the + concept; the literal is one step removed. +- The union is **reusable**. Other types that need the same set + of literals (`NotificationFilter`, `MessageDraft`, + `MessageSummary`) reuse `MessageType` instead of repeating the + union. + +The shape of `Message` is **open to extension** without breaking +existing callers. Adding `priority` is one line in the type +definition. Adding `sentAt` is one line. Each extension is +**additive** because the type is the contract. + +The bad shape, primitive as an identifier: + +```ts +// Bad — three strings, all the same type. +function transfer(fromAccountId: string, toAccountId: string, amount: number): void { + // ... +} +``` + +When the right shape applies — multiple semantic IDs in the same +context: + +```ts +// Good — each identifier is a distinct type. The compiler refuses +// to swap them. +type AccountId = string & { readonly __brand: 'AccountId' }; +type CustomerId = string & { readonly __brand: 'CustomerId' }; + +function transfer(from: AccountId, to: AccountId, amount: number): void { + // ... +} + +// transfer(customerA, accountB, 100) — type error at the call site, +// before the function runs. +``` + +When the branded type does **not** apply — single identifier with +no internal structure: + +```ts +// Bad — the brand adds no information. The id is just an +// incremental or UUID string. Branding forces every consumer to +// construct the branded type, which is friction without benefit. +type OrderId = string & { readonly __brand: 'OrderId' }; + +function getOrder(id: OrderId): Order { + // ... +} + +// The caller has to do this: +getOrder(order.id as OrderId); +// Or this: +getOrder({ value: order.id } as OrderId); + +// Both are friction. The compiler was never going to confuse +// `OrderId` with `CustomerId` if there is only one id type. +``` + +When the codebase has only one identifier per domain value +(`OrderId` and nothing else), the primitive is the right shape. +The brand would be ceremony without value. The right move is: + +```ts +// Just use the primitive. The type name is the contract. +function getOrder(id: string): Order { + // ... +} + +getOrder(order.id); +``` + +The brand is justified only when the codebase has **two or more +IDs of the same primitive type that must not be confused**. The +moment a second ID appears (`OrderId` and `CustomerId` next to +each other in a transfer function, say), the brand becomes the +cheapest way to tell them apart at compile time. + +The branded type has the same runtime shape as `string`, but the +type system distinguishes them. The consumer cannot swap +`AccountId` and `CustomerId`; the compiler refuses. + +## The four patterns + +The codebase uses four patterns for domain types, in increasing +order of expressiveness. + +- **Branded type** (identifier, scalar): `type AccountId = string & { +readonly __brand: 'AccountId' }`. Same runtime shape as the + primitive; the type system prevents mix-ups. Use when the value + is a single string or number with no internal structure. +- **Record type** (shape with fields): `type Message = { readonly +content: string; readonly type: 'text' | 'image' }`. Use when the + value has internal structure the domain cares about. +- **Discriminated union**: `type Event = { kind: 'click'; x: number; +y: number } | { kind: 'key'; key: string }`. Use when the value + has multiple shapes the consumer switches on. +- **Branded record** (identifier with metadata): `type UserId = { +readonly value: string; readonly tenantId: string }`. Use when + the identifier carries metadata the domain cares about. + +The pattern is chosen by **what the value carries**, not by +preference. A `UserId` that is just a string gets the branded +pattern; a `UserId` that carries tenant info gets the branded +record. A `Message` with content and type gets the record; an +`Event` with multiple shapes gets the discriminated union. + +## When the rule does not apply + +The rule applies to **values that participate in the domain**. +It does not apply to: + +- **Truly local values** that never escape a single function. + `function double(n: number)` is fine; the consumer never sees + `n`. +- **Built-in primitive usages** that the language requires — + `Array.prototype.length` is `number`; `string.length` is `number`; + iteration indices are `number`. The rule is about domain values, + not language-level primitives. +- **Boundary conversions**. When JSON is parsed, the parser returns + `string`; the conversion to `Message` happens immediately + after. The conversion is the boundary. +- **Algorithm-internal values** that the algorithm never exposes. + A `Stack` may use `T[]` internally; the public `Stack` + type hides the array. + +## The conversion at the boundary + +The conversion from primitive to domain type happens **once**, at +the boundary where the value enters the system. After conversion, +the rest of the codebase uses the domain type. The conversion +function is named for the domain concept, not the primitive: + +```ts +// Bad — the parser returns string; the rest of the codebase uses string. +function parseNotification(raw: string): string { + // ... +} + +// Good — the parser returns the domain type; the rest of the +// codebase sees the shape. +function parseNotification(raw: string): Message { + // ...validate, then construct the typed value. + return { content: '...', type: 'text' }; +} + +// And the conversion function lives next to the type: +function parseUserId(raw: string): UserId { + if (!isValidUserId(raw)) { + throw new InvalidUserIdError(raw); + } + return { value: raw }; +} +``` + +The conversion function **validates the contract** that the +primitive cannot. A `parseUserId` rejects strings that are not +valid user-ids; the rest of the codebase never has to check. + +## What this looks like in violation + +Three shapes that this rule exists to catch. + +The first, primitive as a function parameter: + +```ts +// Bad — what is the difference between the three strings? +function createUser(name: string, email: string, password: string): User { + // ... +} +``` + +Three strings, one type. The compiler cannot tell which argument +is which; the reader has to read the call sites to know. + +The second, primitive as a function return type: + +```ts +// Bad — what does this string mean? +function getUserId(user: User): string { + // ... +} +``` + +The return type is `string`. The consumer receives a string. The +consumer does not know whether this string is the user-id, the +username, or the email. The fix is `function getUserId(user: User): +UserId`. + +The third, primitive in a data structure: + +```ts +// Bad — three strings in one shape. +type Notification = { + readonly message: string; + readonly sender: string; + readonly recipient: string; +}; +``` + +A `Notification` with three strings is a shape where the consumer +cannot tell the fields apart. The fix is to make each field a +domain type: + +```ts +type Notification = { + readonly message: Message; + readonly sender: UserId; + readonly recipient: UserId; +}; +``` + +The shape is the same shape at runtime; the type system now +refuses to mix the three. + +## What senior practitioners say + +> "TypeScript's type system doesn't always provide a way to +> differentiate types that seem to be structurally the same. [...] +> We need a way to 'brand' (mark) some value as being not just any +> old value, but specifically the type we want." +> +> — Josh Goldberg, _Branded Types_, Learning TypeScript, +> August 2024. + +Goldberg captures the structural-typing limitation. Two strings +are the same to the compiler; the brand is what makes them +different. The rule's branded-type pattern is Goldberg's pattern. + +> "More general types like `number` or `string` can suffice in +> terms of general compile-time checks, but they fail to provide +> checks for more nuanced cases. [...] You may not immediately +> notice the error during runtime. Only at a later time when you've +> forgotten about writing this could the issue pop up again +> unexpectedly." +> +> — Ferreira, _Opaque / Branded Types in TypeScript_, 2022. + +Ferreira captures the **time dimension** of the rule. The bare +primitive compiles today; the bug appears six months later when +the consumer has forgotten the convention. The domain type +**shifts the error to compile time**, which is the only time the +author can catch it. + +> "The reason for having a symbolic name for a type isn't +> 'consistency'. It's to increase the expressivity of your code. +> [...] It makes it easier to work with for humans." +> +> — Kilian Foth, _Is it OK to have type aliases for primitive +> types in TypeScript?_, Software Engineering Stack Exchange, +> accepted answer (22 votes), 2022. + +Foth captures the **reader's economy**. A primitive that is aliased +to a domain type still reads as a primitive to the consumer; the +alias adds nothing. A domain-specific type adds the **shape** the +domain cares about, and the shape is what the reader sees. + +The three sources converge on the operational rule: primitives +are the **wrong level of abstraction** for any value the domain +treats as a concept. The right level is a type the domain owns. + +## Enforcement + +- **Code review**. A reviewer who sees a `string` or `number` in a + public function signature, a return type, or a data structure + blocks the PR and asks for the domain type. +- **Lint rule** (future). A custom ESLint rule can flag function + parameters typed as primitives when the parameter name is a + domain concept (`message`, `recipient`, `amount` without a + custom type). The rule's existence is the enforcement signal + even before it is automated. +- **Quarterly audit**. A standing review of "what primitives + cross module boundaries in this codebase?" surfaces the + candidates that slipped through. + +## Exceptions + +Built-in language primitives are exempt: `Array.prototype.length` +is `number`; iteration indices are `number`. The rule applies to +**domain** primitives. + +A truly local value that never escapes a function is exempt. The +point of the rule is the **boundary** and the **consumer**; local +values have neither. + +A boundary conversion is exempt: the conversion function +**accepts** the primitive. The conversion produces the domain +type. After the conversion, the primitive is gone from the +codebase's vocabulary. + +Algorithm-internal primitives are exempt. A `Stack` may use +`T[]` internally; the public type hides the array. The internals +of an algorithm are not the boundary. + +## See also + +- **Rule 0012** — Prefer `type` Over `interface`: domain types + are declared as `type`, not `interface`. The rule's pattern + is the structural form this rule's content takes. +- **Rule 0014** — Functions Over Classes for Public API: the + conversion function (`parseUserId`, `parseNotification`) is a + factory function in the sense of rule 0014 — it is the only + public construction of the domain value, and the consumer never + `new`s a `Message`. +- **Rule 0001** — Project Mindset: invariant 9 ("optimise for + the reader") is the philosophical ground. The reader sees a + domain type at the boundary; the reader does not see a + primitive that may or may not be the right thing. + +## Sources + +- **Goldberg, Josh.** _Branded Types._ Learning TypeScript, + August 2024. The structural-typing limitation: two strings + with the same shape are the same type to the compiler; a + brand is what makes them different. The rule's branded-type + pattern is Goldberg's pattern. +- **Ferreira.** _Opaque / Branded Types in TypeScript._ 2022. + The time dimension: a primitive bug appears six months later + when the consumer has forgotten the convention; a domain type + shifts the error to compile time. The rule's "compiletime vs + runtime" framing is Ferreira's. +- **Foth, Kilian.** _Is it OK to have type aliases for primitive + types in TypeScript?_ Software Engineering Stack Exchange, + accepted answer (22 votes), 2022. The reader's economy: a + primitive aliased to a domain type still reads as a primitive + to the consumer; the alias adds nothing. The rule picks the + shape over the alias. +- **AIWalker.** Cited from the same SE question: the alias + pattern is a soft antipattern when it is purely descriptive + (no validation, no discrimination). The rule captures the + distinction between alias and brand. diff --git a/docs/engineering/architecture/rules/0016-no-generic-verbs.md b/docs/engineering/architecture/rules/0016-no-generic-verbs.md new file mode 100644 index 0000000..029485c --- /dev/null +++ b/docs/engineering/architecture/rules/0016-no-generic-verbs.md @@ -0,0 +1,281 @@ +# 0016 — No Generic Verbs + +**Status**: Active (enforced through code review). +**Date**: 2026-08-11. + +## Rule + +A function name's verb must answer three questions: + +1. **What transformation does it perform?** (`decode` is the + inverse of `encode`; `parse` takes a string and returns a + parsed value; `validate` checks a condition.) +2. **What does it return?** (`decodeJwt` returns a `Jwt`; + `parseUserId` returns a `UserId`; `validateAge` returns a + boolean.) +3. **What is the contract on the input?** (`parse` accepts a + `string` of a specific format; `decode` accepts an `Encoded` of + a specific algorithm.) + +The verbs `parse`, `convert`, `validate`, `transform`, `handle`, +`process`, `do`, `make`, `perform`, `manage` +fail at least one of the three questions. They are **generic +verbs**: they say "I do something" without saying what. The rule +refuses them as the verb of a function name when a more specific +verb is available. + +The verbs `run` and `execute` sit in a different category. They +_can_ be specific when the function's contract is the orchestration +itself (see "When the rule does not apply" below: `runPipeline`, +`executeSteps`). They are excluded from the generic-verb blacklist +on that basis; the exception below is the canonical place to +evaluate them. + +When no specific verb is available, the rule says: **do not +write the function**. A function whose verb is `process` is a +function whose author did not yet understand what the function +does. Understanding the function is a prerequisite for naming it; +naming it `process` is the symptom of a missing understanding. + +## Why + +A generic verb is a **promise without content**. The function +exists; the author wrote it; the name says only "this function +runs". The reader who meets the function knows nothing new from +the name. The reader must read the body to recover the intent — +when the body could be skipped if the name carried the intent. + +The deeper problem is **epistemic**: the author who wrote +`processMessage` did not yet know what the function did. They +reached for the generic verb because they had no other name to +reach for. The name is a confession: the author named the function +before they understood the function. + +A specific verb is a **claim of understanding**. `decodeJwt` says +the author knew the function takes an encoded JWT and returns a +decoded one. `parseUserId` says the author knew the function takes +a raw string and returns a validated `UserId`. The verb carries +the contract. + +## What this looks like in violation + +The bad shape, generic verb that says nothing: + +```ts +// Bad — what does this function do? +function processMessage(message: Message): void { + // ... +} + +// Bad — what does this convert? +function convert(input: Input): Output { + // ... +} + +// Bad — what does this handle? +function handleRequest(req: Request, res: Response): void { + // ... +} + +// Bad — what does this validate? +function validate(value: string): void { + // ... +} +``` + +Each name is a placeholder. The author reached for a verb when +they did not have a specific verb to use. The body of each +function is where the work lives; the body is where the reader +must go to recover the intent that the name should have carried. + +The right shape, specific verb that says what: + +```ts +// Good — the verb says the transformation, the return type says +// the result. +function decodeJwt(token: EncodedJwt): Jwt { + // ... +} + +// Good — parse says "raw string in, parsed value out"; the return +// type says which parsed value. +function parseUserId(raw: string): UserId { + if (!isValidUserIdFormat(raw)) { + throw new InvalidUserIdError(raw); + } + return { value: raw }; +} + +// Good — send is the operation; the return type says the +// acknowledgement. +function sendNotification(message: Message, recipient: UserId): NotificationAck { + // ... +} + +// Good — handle is generic; what the handler does is the verb. +function onOrderCancelled(order: Order): void { + // Mark the order as cancelled, refund the customer, notify them. +} +``` + +Each verb is specific. Each return type names the result. The +reader knows what the function does from the name. + +## The test for a good verb + +Before committing a function name, ask four questions: + +1. **Can the reader tell what the function does from the name + alone?** If not, the verb is generic. +2. **Is the return type the answer to "what does this function + produce"?** If not, the function does too many things; split + it. +3. **Is the input type the answer to "what does this function + accept"?** If not, the function accepts too many things; + narrow the input. +4. **Could a reader write a unit test for this function without + reading the body?** If the test requires reading the body to + know what to assert, the name is not specific enough. + +A "no" to any question is a signal to rename. + +## When the rule does not apply + +The rule refuses generic verbs as **the verb of a function name**. +It does not refuse: + +- **Generic verbs inside a function body** — a comment, a log + message, an error message. `// process the message before +sending` is fine in a comment; the comment does not have to + carry the contract. +- **Generic verbs as nouns** — `handleRequest` as a class name is + a different smell (rule 0013). The rule here is about the + verb of a function. +- **Truly generic operations** — a function whose job is genuinely + "do several things in order" may be named `runPipeline` or + `executeSteps` if the steps are not the function's contract; + the function delegates to named helpers. _This is why `run` + and `execute` are not in the blacklist at the top of the + rule: the orchestrator's contract is the order of the steps, + and the verb names the order. A function called `run` (or + `execute`) on a single step is back in the violation case + above; a function called `runPipeline` is the legitimate + shape._ + +## What senior practitioners say + +> "There are 2 hard problems in computer science: cache +> invalidation, naming things, and off-by-1 errors." +> +> — Phil Karlton, paraphrased in Daniel Lübke, _The easiest rule +> to not give bad names for your APIs and operations: No Generic +> Terms_, 2021. + +The Karlton joke becomes operational in this rule: the hardest +problem in computer science is naming, and generic verbs are the +easiest way to fail at it. + +> "Forbidden words in identifiers: do, make, handle, perform, +> something. They are so generic. An operation will do something +> by definition. So you should not mention that. Make is double +> the characters without conveying any more meaning." +> +> — Daniel Lübke, _The easiest rule to not give bad names for +> your APIs and operations: No Generic Terms_, January 2021. + +Lübke's forbidden list is the operational form of the rule. +This rule extends Lübke's list with the function-name-specific +verbs that are common in this codebase: `parse`, `convert`, +`validate`, `transform`. + +> "Parse, don't validate. [...] Returning `Maybe` is undoubtedly +> convenient when we're implementing `head`. However, it becomes +> significantly less convenient when we want to actually use it! +> [...] The burden falls upon its callers to handle that +> possibility." +> +> — Alexis King, _Parse, don't validate_, November 2019. + +King's principle, applied to naming: a function called `validate` +returns `boolean`; the caller must handle the case where the +boolean is `false`. A function called `parse` returns the parsed +value; the caller does not handle the negative case (the parse +function throws). The verb encodes the contract. + +> "An `AutoMakersList : List` adds complexity without +> adding information. [...] The list is still of `string`, and +> the last time I checked, there were no validation methods on +> `string` that validate they are auto maker's names." +> +> — Andrew Theken, _The "Named Generic" Anti-pattern_, June 2010. + +Theken targets types; this rule targets verbs. Both are instances +of the same principle: a name that adds zero information is a +smell, whether it is a class name or a function name. + +The four sources converge on the operational rule: a function +name's verb must encode the transformation, the return type must +encode the result. Generic verbs fail both tests. + +## Enforcement + +- **Code review**. A reviewer who sees a generic verb (`process`, + `convert`, `validate`, `transform`, `handle`) in a function + name blocks the PR and asks for a specific verb. +- **Naming audit**. A standing review of "which function names in + this codebase use a generic verb?" surfaces the candidates that + slipped through. Each is a rename candidate. +- **Lint rule** (future). A custom ESLint rule can flag function + names that match a list of generic verbs. The rule's existence + is the enforcement signal even before it is automated. + +## Exceptions + +A function whose job is genuinely "do several things in order" +may use a verb that names the orchestration (`runPipeline`, +`executeSteps`) when the function's contract is the order, not the +work. The work is done by named helpers; the function delegates. +The rule is not against orchestration verbs; it is against +naming-without-understanding. + +A test or fixture name may use a generic verb (`setupTest`, +`createFixture`) because the test's contract is "set up state", +not "do domain work". The rule applies to production code; test +code is exempt. + +## See also + +- **Rule 0005** — Named Algorithms and Independent Data Structures: + a function whose verb is `process` is a function whose algorithm + is not named. Rule 0005 captures the algorithm naming; + rule 0016 captures the verb naming. +- **Rule 0013** — Entity-First Naming: a class named `Handler` + is the noun-level equivalent of a function named `handle*`. The + two rules compound; the suffix smell and the verb smell are + instances of the same "named generic" anti-pattern. +- **Rule 0015** — Domain-Specific Types Over Primitives: a generic + verb often pairs with a primitive return type. `processMessage` + returns `void`; the verb is generic because the return is + generic. The fix for both is the same: pick a specific verb and + a specific type. + +## Sources + +- **Lübke, Daniel.** _The easiest rule to not give bad names for + your APIs and operations: No Generic Terms!_ January 2021. The + forbidden-words list (`do`, `make`, `handle`, `perform`, + `something`) is the operational form of this rule; the rule + extends the list with the function-name-specific generic verbs + the codebase encounters (`parse`, `convert`, `validate`, + `transform`). +- **King, Alexis.** _Parse, don't validate._ November 2019. The + principle that a function's verb encodes the contract: `validate` + returns a boolean; `parse` returns the parsed value. The rule + applies the principle to naming — the verb says what the + function returns. +- **Theken, Andrew.** _The "Named Generic" Anti-pattern._ June 2010. Targets types; this rule targets verbs. Both are instances + of the same principle: a name that adds zero information is a + smell. +- **Karlton, Phil.** Paraphrased by Lübke, the canonical + formulation of the naming problem. The rule operationalises the + joke. diff --git a/docs/engineering/architecture/rules/INDEX.md b/docs/engineering/architecture/rules/INDEX.md new file mode 100644 index 0000000..204f7a3 --- /dev/null +++ b/docs/engineering/architecture/rules/INDEX.md @@ -0,0 +1,91 @@ +# Rules — Index + +This folder collects the project's standing **architecture rules**. +Every rule is a durable, always-on constraint that every contribution +must respect. Rules are enforced through code review; selected rules +are enforced through CI lint as the harness matures. + +> "If the type says it's not null, trust the type. If the type is +> wrong, fix the type. Don't add runtime null checks for values that +> can't be null." +> +> — Miguel Pizza, _No Defensive Null Checks_, Maintainable +> TypeScript doctrine. + +The slogan of the project. Rule 0001 elevates this as the +operating principle behind every invariant; rule 0004 +operationalises it for runtime guards. The remaining rules +inherit from it. + +## The sixteen rules at a glance + +| # | Rule | One-sentence summary | +| ---- | ------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 0001 | Project Mindset | Every contribution must be made as if it were the last commit before the project reached its largest possible audience; ten absolute invariants, no exceptions. | +| 0002 | File Separation | Within a concern, types/constants/functions split into their own files; across concerns, no shared barrel that re-exports types or helpers. | +| 0003 | File Placement | Decide where a new file lives before creating it; a single-caller helper stays next to its caller, extraction requires a second real use site. | +| 0004 | No Speculative Defences | A runtime guard exists to handle a demonstrated scenario; "just in case" guards are a tax on every reader. | +| 0005 | Named Algorithms and Independent Data Structures | Algorithms live as named functions, not inline comments; no diminutives; data structures are independent of the algorithm that first used them. | +| 0006 | Technology Choices | Every language mode, module system, validator, and dependency is a deliberate assumption; each must answer four questions (what, enables, rules out, revisits). | +| 0007 | Top-Down Composition | A function reads top-down; the first line tells the reader what it does, every subsequent step is a name the consumer follows; DX wins over internal cleverness. | +| 0008 | No Chained Type Assertions | `as X as Y` and `as unknown as Y` are forbidden; a single assertion crossing one boundary is allowed; the fix for a chain is a runtime guard or a better source type, not a longer cast. | +| 0009 | Open Extension, Closed Modification | A function that branches on an internally-defined enumeration dispatches through a Map or typed table; new values are added by extending the registry. | +| 0010 | Typed Environment Access | `process.env` is read in exactly one file per workspace; the rest of the codebase imports a typed accessor. | +| 0011 | Filenames Are kebab-case | Every file in this repository is named in lowercase letters, digits, and hyphens; no camelCase, PascalCase, snake_case. | +| 0012 | Prefer `type` Over `interface` | Shapes are declared with `type`; `interface` is reserved for declaration merging, class implementation of open shapes, and host type augmentation. | +| 0013 | Entity-First Naming | Any name that ends in `-er` (`Manager`, `Service`, `Handler`, `CancelOrderHandler`) is refused; only entity names (`OrderCancellation`) are accepted. | +| 0014 | Functions Over Classes for Public API | Classes are internal implementation details; the public API exports factory functions (`group()`, `createGroup()`), never `new ClassName()`. | +| 0015 | Domain-Specific Types Over Primitives | A `Message` is a `Message` (with `content`, `type`, …), not a bare `string`; a domain identifier is branded only when a second identifier of the same primitive would otherwise be confused with it. Primitives cross boundaries only at conversion functions. | +| 0016 | No Generic Verbs | A function's verb must encode the transformation (`decode`, `parse`, `validate`) and the return type must encode the result; `process`, `convert`, `handle`, `do` are refused. | + +## How to read this folder + +If you are new to the project, read in this order: + +1. **Rule 0001** — the mindset. Every other rule is a consequence of + the ten invariants. +2. **Rule 0002** then **Rule 0003** — how files are split and + placed. The structure every other rule assumes. +3. **Rule 0004** then **Rule 0005** — what code is honest about its + runtime behaviour and its algorithms. +4. **Rule 0006** then **Rule 0007** — what assumptions the project + commits to and how those assumptions read in code. +5. **Rules 0008, 0009, 0010, 0011** — the type and runtime + discipline. These are mechanically checkable and become CI gates + as the harness matures. + +## Cross-references + +Each rule has its own `## See also` section listing the rules it +depends on or complements. A rule that says "see rule 0009" means +"the discipline is fully stated in 0009; this rule is the upstream +constraint that 0009 then enforces". + +The cross-reference graph is intentionally dense; the rules are +meant to be read together. A reader who finishes one rule should +have a clear next rule to consult. + +## Adding a new rule + +The format and lifecycle are documented in this folder's +[`README.md`](./README.md). The short version: + +- One concept per rule. If a rule says "X and Y", it is two rules + waiting to be split. +- A `NNNN-short-slug.md` filename, monotonic. +- The rule must answer four questions: what is it, what does it + enable, what does it rule out, when would we revisit. +- The rule must include at least one bad/good code example unless + the rule is purely structural (a casing rule, a placement rule). +- The rule must include a `## See also` section that links to + neighbouring rules. +- The rule must declare its enforcement: review, CI lint, or both. + +## Status lifecycle + +| Status | Meaning | +| ---------------------- | ------------------------------------------------------------------------------------------ | +| **Active** | Currently enforced. Every PR must respect this rule. | +| **Enforced via CI** | The rule is checked mechanically on every PR. _(Target state — no rule has migrated yet.)_ | +| **Superseded by NNNN** | Replaced by a later rule; the old rule is kept for context and cross-references. | +| **Deprecated** | Kept on disk for context but no longer required. | diff --git a/docs/engineering/architecture/rules/README.md b/docs/engineering/architecture/rules/README.md new file mode 100644 index 0000000..7f24b5c --- /dev/null +++ b/docs/engineering/architecture/rules/README.md @@ -0,0 +1,68 @@ +# Architecture Rules + +This folder collects the standing **architecture rules** for the +DeesseJS Errors repository. Unlike ADRs (which capture one decision at +a time), rules are durable, always-on constraints that every PR must +respect. + +## Format + +Each rule is stored as a Markdown file with the naming convention +`NNNN-short-slug.md`, where `NNNN` is a monotonically increasing +4-digit sequence. For example: + +- `0001-project-mindset.md` +- `0002-file-separation.md` + +The sequence numbers are **never reused**. When a rule is rescinded, +the file is moved to `_superseded/` with a `Superseded by NNNN` +header at the top. + +## Status lifecycle + +Rules carry one of the following states: + +- **Active** — currently enforced by CI or code review. +- **Enforced via CI** — the rule is checked automatically on every PR. +- **Superseded** — replaced by a later rule (cross-link required). +- **Deprecated** — kept on disk for context but no longer required. + +## When to add a rule + +Add a rule when: + +- A constraint has come up three or more times in PR review. +- A constraint cannot be expressed in the type system alone. +- A constraint is not obvious from reading the code (e.g. it spans + multiple files or workflows). + +Do **not** add a rule for things that TypeScript or ESLint already +enforce — point to those tools instead. + +## Authoring + +Each rule should have: + +1. **Rule** — one sentence that says what is required. +2. **Why** — the architectural reason, in 2-3 sentences. +3. **Enforcement** — CI check, lint rule, or review-only. +4. **Exceptions** — if any, with the rationale for each. + +Rules are focused, but their length follows the doctrine they +encode, not the other way around. A rule that names a heuristic, +cites its sources, and walks through the violation it catches may +be long; the length is the cost of being unambiguous. A rule that +has grown past what the doctrine needs is a refactor candidate. +Process documents (release runbook, PR authoring guide) live under +`docs/internal/engineering/process/`; the boundary is the **purpose** +of the document, not its length. + +## Active rules + +- See the files in this directory for the current rule set. + +## See also + +- [`../decisions/`](./decisions/) — Architecture Decision Records. +- [`../../internal/engineering/process/`](../../internal/engineering/process/) — process documents + (release runbook, PR authoring guide, etc.).