From 05fad0fcd6957848df5f1dbc5e136d0ba39c61f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 17 Sep 2026 20:59:28 +0000 Subject: [PATCH] :memo: Record the autocomplete and matcher-design decisions The matcher is now two three-overload factories; library.md documents why the union merge, inferred universe, conditional RequireKeys and cases-first paths were rejected, and lists the two open issues (the fallback sees all of T; a redundant _ is still accepted). testing.md records the language server as the autocomplete oracle, and CONTRIBUTING points the exception at the matcher's own test file. The backlog marks the design-doc and autocomplete groundwork done alongside the adoption. --- CONTRIBUTING.md | 5 +++ backlog.tasks | 16 ++++++++ development/library.md | 88 +++++++++++++++++++++++++++++++++++++++++- development/testing.md | 63 +++++++++++++++++++++++++++--- 4 files changed, 166 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2063fad..c9a882c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -67,6 +67,11 @@ before the implementation. The loop is **type → red → green → refactor**: `npm run verify` as the definition-of-done gate. Every test pairs an `expectTypeOf(...)` with an `assert.*`; keep them together. +The autocomplete tests (`src/util/__tests__/lsp-completion.test.ts` for the +helper, `src/primitive.test.ts` for the matcher's popup) are the +exception — +the language server, not the type system, is the oracle (see +[development/testing.md § Autocomplete](./development/testing.md#autocomplete)). Each test body follows **AAA (Arrange–Act–Assert)** with labeled blocks separated by a blank line: `// Arrange` sets up the inputs (e.g. the matcher factory), `// Act` exercises the subject once from them (not a second diff --git a/backlog.tasks b/backlog.tasks index 448a052..f90afa0 100644 --- a/backlog.tasks +++ b/backlog.tasks @@ -27,6 +27,21 @@ v1.0: ☐ Achieve 100% branch coverage on `src/primitive.ts` ☐ Achieve 100% branch coverage on `src/index.ts` +Testing: +✔ Cover autocomplete with real completion test cases in the suite @medium @done + → `src/util/__tests__/lsp-completion.ts` (`#test-utils/…`) is the helper; the suite asserts its labels (see development/testing.md § Autocomplete) + → wire it into `node --test` so a test asserts the offered labels + → note: `Parameters[0]` resolves only the *last* overload; use `@ts-expect-error` call sites for factory negatives, not `not.toExtend>` + +Matcher: +✔ Clean up: adopt the 3-overload matcher (`src/prototype-ac2.ts`) and delete the prototypes @high @done + → `strict` / `widened`, each with overloads `ExhaustiveLoose` → `Fallback` → `Handlers` (order is load-bearing) + → fold into `src/primitive.ts` / the public API; drop `src/prototype*.ts` +☐ `_` should receive only the unhandled `T` keys, not all of `T` @medium + → today `_: (shape: T) => R`; desired `_: (shape: Exclude) => R` +☐ An exhaustive pattern that also carries `_` must be a compile error @medium + → `{ a, b, _ }` for `T = "a" | "b"` is accepted today; the redundant `_` should be rejected + Bugs: Enhancements: @@ -41,6 +56,7 @@ Documentation: ☐ Add comparison section vs. other TS pattern-matching libs in Readme.md ☐ Write migration guide for users coming from discriminated unions ☐ Create backlog tasks for implementation +✔ Document the matcher design paths in `development/` — union vs overload merge, inferred universe (`NoInfer`), conditional `RequireKeys`, cases-first — and why each was abandoned @done ☐ Validate code fences in Markdown (start with README.md) — compile the TypeScript examples against `src/` so the docs cannot drift from the API Workflow: diff --git a/development/library.md b/development/library.md index 280e467..3363103 100644 --- a/development/library.md +++ b/development/library.md @@ -3,4 +3,90 @@ The type-level design of the public API and the limitations it carries. The user-facing reference is [README § API](../README.md#api). -Currently the library is placeholder code. +The matcher below is implemented in `src/primitive.ts` and re-exported from +`src/index.ts`; the rest of the library is placeholder code. + +## Matcher shape + +#### Decision (2026-09) + +A matcher is built by a factory and applied to a pattern: + +```ts +const matcher = strict<"a" | "b">()({ a: (s) => …, b: (s) => … }); +``` + +Whether the pattern is exhaustive or has a fallback is decided **at the call +site**, by whether it carries `_` — F#'s `| _ ->`. Only the return-strictness +axis remains, so there are two factories: + +- `strict` — one common `R`, the best common return type of every handler; +- `widened` — the union of every handler's return type. + +Both are three overloads whose order is load-bearing: + +1. `ExhaustiveLoose` = `{ [K in T]: UnaryFn } & { _?: UnaryFn }` +2. `Fallback` = `Partial<…> & { _: UnaryFn }` +3. `Handlers` — the pure exhaustive shape + +#### Why + +- **Autocomplete reads the first overload, the error reads the last.** + TypeScript takes the first overload signature as the contextual type for the + object-literal popup, and the last for `No overload matches this call`. So + `ExhaustiveLoose` first yields the popup `_?, a, b` (T-keys required, `_` + optional) while `Handlers` last yields `Property 'b' is missing`. The two can be + tuned independently. +- Two factories, not four: the fallback is a pattern _shape_, not a separate API. +- The widened return union is derived from the pattern's handler types, so it + needs no fourth signature. + +#### Rejected + +- **Four factories** (`src/primitive.ts` today: exhaustive × fallback × + strict/widened). The exhaustive/fallback axis is expressible as one pattern + type; four signatures duplicate it. +- **Union merge** — one type `Exhaustive | (Partial<…> & { _: … })`, + explicit `()`. Type-safe and completable, but TypeScript reports the + near-miss union member, so a missing key reads `Property '_' is missing` + instead of naming the key. Arm order does not change the report; the overload + split does. +- **Overload merge with only the exhaustive arm last.** Fixes the missing-key + message, but a wrong `_` parameter is then reported against the exhaustive + arm, and `Parameters` sees only one arm. +- **Inferred universe** — `match(pattern)` with `T` taken from the keys + (exhaustive) or from `_`'s annotated parameter (fallback), via `NoInfer` and + `_?: never`, split by overloads (a plain union merges inference; measured + `T = "_" | "a"`). No explicit ``, and pipe-friendly. Rejected because: with + no declared universe the exhaustive popup offers only `_`; an unannotated `_` + widens `T` to `string | number`; and `NoInfer` leaks into the emitted `.d.ts`, + raising the consumer floor to TypeScript 5.4 (README promises `>= 5.0`). +- **Conditional `RequireKeys`** — parameter + `P & ("_" extends keyof P ? unknown : Handlers)`. Gives the good + missing-key message, but `keyof P` counts _optional_ keys: a widened value + whose declared type has `_?:` bypasses the completeness check. Demanding a + required `_` instead rejects that case but breaks `P` inference — `P` falls back + to its constraint and partial literals then demand every key. Typos also need a + `NoExtra` guard, whose message degrades to `not assignable to never`. +- **Cases-first curried** — `match(["a", "b"])({ a: …, b: … })`. Completion works + for exhaustive patterns, and the array is a single source of truth for the + runtime list and the union. Rejected as not pipe-friendly; it needs a runtime + array; and the single-call form `match(cases, pattern)` cannot infer `R` (the + mapped key type `K[number]` stays deferred, so `R` widens to `unknown`). + +#### Known issue + +- The `_` handler receives **all** of `T`, not the unhandled subset + (`Exclude`). +- An exhaustive pattern that also carries `_` is accepted; the redundant `_` + should be a compile error. +- The widened overloads carry a completeness guard + `keyof P extends T | "_" ? unknown : never`, because TypeScript does not apply + the excess-property check to a generic constraint: a generic parameter accepts + extra keys, a parameter typed as a concrete object type does not. + `PatternReturns` must be + `ReturnType, (...args: never[]) => unknown>>` so it survives + the closed, partly-optional `P` constraints. +- `Parameters[0]` resolves only the **last** overload, so it is + not a sound "rejected" oracle for a factory. Factory-negative tests use + `@ts-expect-error` call sites (the test file only — the general ban stands). diff --git a/development/testing.md b/development/testing.md index 7b6629e..ca85c48 100644 --- a/development/testing.md +++ b/development/testing.md @@ -30,7 +30,8 @@ before the implementation. - Testing the type only: it would not catch handler dispatch or the `_` fallback (see `src/primitive.test.ts`). -The runner is `node --test --strip-types "src/**/*.test.ts"` and the tiers are in +The runner is `node --test --strip-types` over `src/**/*.test.ts` and +`scripts/**/*.test.ts`; the tiers are in [CONTRIBUTING.md § Development commands](../CONTRIBUTING.md#development-commands). `c8` uses V8 coverage, so the `--strip-types` source is instrumented without a build step, and the runner relies on the `.ts` import-extension convention (see @@ -96,6 +97,10 @@ import { LspSession } from "#test-utils/lsp-completion.ts"; - The helper uses `node:` builtins (it drives a language server), so `import/no-nodejs-modules` is off for `src/util/__tests__/**` in `.oxlintrc.json` — the same exception the old `scripts/**` scope carried. +- Helpers carry no library coupling: their tests probe in-memory documents + whose contextual types are written inline, so only the helper's own contract + (marker handling, position, label extraction) is under test. A test that + asserts a _matcher's_ popup belongs with the matcher. #### Rejected @@ -111,11 +116,59 @@ import { LspSession } from "#test-utils/lsp-completion.ts"; still guards non-test helpers importing each other, and the exemption is only needed for the one specifier the mapping already solves cleanly. +## Autocomplete + +#### Decision (2026-09) + +Completion is verified by driving the repo's own language server +(`tsc --lsp --stdio`, the same server pi's LSP extension talks to) through +the test helper `#test-utils/lsp-completion.ts` +(`src/util/__tests__/lsp-completion.ts`), not through the type system: + +```sh +node --strip-types src/util/__tests__/lsp-completion.ts [] +``` + +The script prints the labels the server offers at a `/*COMPLETE*/` marker inside +`` (the marker is stripped before the document is sent). Its `LspSession` +is imported by `src/util/__tests__/lsp-completion.test.ts` — which tests the +helper itself against inline documents, never the library's code — and by +`src/primitive.test.ts`, where the same probe asserts the matcher's popup; +the CLI is for manual inspection. + +#### Why + +- Completion is a contextual-type property: it depends on which overload + signature TypeScript picks for the object literal, and no type-level assertion + observes that. +- `Parameters[0]` resolves only the _last_ overload, so it is + not the popup's contextual type either — see + [library.md § Matcher shape](./library.md#matcher-shape). +- The server is the only ground truth; the script reproduces what the editor + shows. + +#### Rejected + +- **`expect-type` would not work**: there is no operator for “the popup offers + these labels”. `toExtend` / `toEqualTypeOf` test assignability and cannot say + which overload supplied the contextual type. +- **Checking by hand in the editor**: not reproducible in review or by an agent. +- **`@ts-expect-error` at a completion position**: it asserts the absence of a + compile error, not the presence of specific labels. + +#### Known issue + +- Each test spawns its own `tsc` server so the tests share no state and pass in + any order; the file is an integration test (~1.6 s) that needs `node_modules`. + `didOpen` is handled in order before the completion request, so no settle + delay is needed. +- The server answers some requests with a string id (`client/registerCapability`); + the client must tolerate `string | number` ids or the server stalls. + ## Known issues - The type-aware linter misidentifies `expectTypeOf()` as a floating promise, so - test files that use it (`src/primitive.test.ts`) carry a - file-level `oxlint-disable -typescript/no-floating-promises` with an explanatory comment. It is a known - false positive, not a rule worth disabling project-wide (see + test files that use it (`src/primitive.test.ts`) carry a file-level + `oxlint-disable typescript/no-floating-promises` with an explanatory comment. + It is a known false positive, not a rule worth disabling project-wide (see [tooling.md § oxlint-disable directives live next to the code](./tooling.md#oxlint-disable-directives-live-next-to-the-code)).