diff --git a/eslint.config.mjs b/eslint.config.mjs index afb62d378f..2bb933145c 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -252,6 +252,25 @@ const ENGINE_QUERY_READ_METHODS = ['find', 'findOne', 'count', 'aggregate']; // erase the type to construct input `tsc` would otherwise refuse. A blocking // rule there would fight the tests that prove the contract is enforced, so the // test surface is held by a COUNT instead — see the ratchet. +// +// ⚠️ #8210, stated honestly because it was previously assumed rather than +// measured: shrinking that count does not put every counted site behind a +// working guard. `QUERY_OPTIONS_ANY_MESSAGE` below is accurate for the sites +// this rule actually BLOCKS (non-test code, where `tsc` is the real, live +// channel), but roughly 60% of today's test-surface sites live in packages +// whose OWN `tsconfig.json` excludes `**/*.test.ts` — for those, typing the +// options removes an `any` that would blind an editor's language service (and +// is a precondition for the day the exclusion lifts), but neither `tsc` nor +// this repo's ESLint config catches a wrong key there today: this repo runs +// one `eslint.config.mjs`, which never enables type-aware linting +// (no `parserOptions.project`, no typed `@typescript-eslint` rules) for ANY +// file, test or not. Measured with a positive control (a typed, wrong-keyed +// `EngineAggregateOptions` planted in an excluded `objectql` test file: both +// `pnpm --filter @objectstack/objectql typecheck` and `pnpm exec eslint +// --no-inline-config` on that file stayed silent) and cross-checked against +// PR #8406, where the identical shape independently surfaced in +// `packages/lint` on the same day. See the measurement and the file/package +// split in `scripts/check-query-options-erasure-ratchet.mjs`'s header. export const QUERY_OPTIONS_TEST_GLOBS = [ '**/*.test.{ts,tsx,mts,cts}', '**/*.spec.{ts,tsx,mts,cts}', diff --git a/scripts/check-query-options-erasure-ratchet.mjs b/scripts/check-query-options-erasure-ratchet.mjs index e0289c7074..5bf66d3341 100644 --- a/scripts/check-query-options-erasure-ratchet.mjs +++ b/scripts/check-query-options-erasure-ratchet.mjs @@ -20,6 +20,43 @@ // number instead, so the surface is measured and cannot grow unnoticed // without a per-file ratchet going red on a legitimate rejection test. // +// ⚠️ #8210: driving this number down does NOT make `tsc` catch a malformed +// options bag for every file it counts, and no other type-aware checker +// fills the gap either — measured directly (see below), not assumed. At +// the time of writing, 6 of the 9 packages holding test-surface sites +// (`objectql`, `runtime`, `spec`, `drivers/driver-mongodb`, +// `plugins/plugin-approvals`, `plugins/plugin-auth`) exclude `**/*.test.ts` +// from their `tsconfig.json` — ~143 of the 240 counted sites sit in files +// `pnpm --filter typecheck` never reads at all (`drivers/driver-memory`, +// `drivers/driver-sql` and `drivers/driver-sqlite-wasm` hold the other ~97 +// and ARE type-checked). Re-derive the split any time with +// `git log`-free measurement: run the two `measure()` calls below against +// `QUERY_OPTIONS_TEST_GLOBS`, bucket the resulting files by whether their +// package's `tsconfig.json` excludes `**/*.test.ts`. +// +// ESLint does not fill the gap either — measured, not assumed: this +// repo's ONE `eslint.config.mjs` never sets `parserOptions.project` (or +// any other type-aware option) and never registers a +// `@typescript-eslint/eslint-plugin` typed rule, so no ESLint pass here is +// type-aware, over test files or non-test files alike. Positive control: +// a `const opts: EngineAggregateOptions = { aggregations: [{ func: … }] }` +// (wrong key — the declared one is `function`) planted in an excluded +// `objectql` test file left BOTH `pnpm --filter @objectstack/objectql +// typecheck` (exit 0) and `pnpm exec eslint --no-inline-config` on that +// file (0 problems) silent. A second, independently measured instance: +// PR #8406's patch round found `pnpm --filter @objectstack/lint typecheck` +// structurally blind to 12 new TS2339s in `packages/lint` test files, +// caught only by the TEST_DEBT hidden-layer measurement — same shape, +// different package, same day. +// +// What driving this number down DOES buy, honestly: it removes an `any` +// that would otherwise blind whatever type-aware tool eventually DOES run +// over the file (an editor's language service today; `tsc` itself on the +// day a package's exclusion lifts), and it is the precondition for that +// day rather than a substitute for it. Where a package's `tsconfig.json` +// excludes `**/*.test.ts`, typing the options bag is real, low-cost +// hygiene — it is just not, today, a compiler guard against a wrong key. +// // It fails when: // • a non-test file NOT in the baseline reports a site (that already fails // `pnpm lint` — reported here too so one command explains the picture), or @@ -154,7 +191,11 @@ export function diffRatchet({ baseline, current, testCeiling, testSites, addedBa `input is DELIBERATELY off-contract (a test asserting the engine rejects an ` + `unknown option) — write \`as unknown as EngineQueryOptions\`, which names the ` + `contract being bypassed, keeps the rest of the call checked, and is not counted ` + - `here. Raising this number is a reviewed edit, not a remedy.`, + `here. Raising this number is a reviewed edit, not a remedy. Note (#8210): in a ` + + `package whose tsconfig excludes \`**/*.test.ts\`, typing the options here does ` + + `NOT make \`tsc\` (or ESLint — neither is type-aware over this repo's test files) ` + + `catch a wrong key; it removes an \`any\` and is a precondition for the day that ` + + `exclusion lifts, not a compiler guard today.`, ); } else if (testSites < testCeiling) { errors.push(