test(gates): the register test reads every config block, option and key, not eight samples and severities
E32 review round 1 (T1–T5, s1). The test that holds docs/gates.md to the effective configuration could itself be passed by a narrowing it did not look at: - ESLint: besides each scope's sample, every config block is read on its own. A block that targets files no sample reaches fails as an "unsampled block"; a no-restricted-* list re-declared without an entry src has, an option that lets something through (allow…/ignore…/except…), a block-level `ignores` and EVERY global ignores object (not only the first) count. A scope switching a src rule at warn off now counts too (it skipped them). Two live narrowings surfaced and are registered: no-floating-promises' allowForKnownSafeCalls (src and e2e) and the three network owners' no-restricted-globals. - TypeScript: a strict sub-flag set false beside `strict: true`, an extends/exclude/files, a changed include, and a frontend-root config file no project checks. eslint.config.js is that file: listed, with the 43 errors strict checkJs reports in it (measured, none a defect). - Ruff, basedpyright, pytest and knip are read as key whitelists (an unknown key fails until someone decides whether it narrows); pyright's information/hint count as weak; ruff's extend-select is pinned; pytest's three keys are pinned exactly; knip's script must run both modes. Each new check was probed with a planted narrowing (an unsampled block, a block ignores, a second global ignores, a warn rule off in a scope, strictNullChecks false, a ruff extend-exclude, a pytest -m) and failed on it.
This commit is contained in:
+15
-2
@@ -14,7 +14,11 @@ name its invariant. Whatever is left narrowed is listed here, and
|
||||
not an oversight; a narrowing missing from here is a test failure.
|
||||
|
||||
How the test matches: each narrowing must appear in a table row below as its `code-quoted` id, in a
|
||||
row whose **Scope** cell names the scope exactly as the test reports it.
|
||||
row whose **Scope** cell names the scope exactly as the test reports it. For ESLint it reads each
|
||||
scope's effective config through a sample file AND every config block on its own: a block that
|
||||
targets files no sample reaches fails as an `unsampled block` (add a sample), a rule a block or src
|
||||
sets weaker than src counts, so does a `no-restricted-*` list that leaves out an entry src has, and
|
||||
so does an option that lets something through (`allow…`, `ignore…`, `except…`) (E32 review).
|
||||
|
||||
## ESLint (frontend)
|
||||
|
||||
@@ -34,6 +38,11 @@ them itself (`no-undef`, `no-redeclare`, `no-dupe-keys`, …), nor the formattin
|
||||
| src | `react-hooks/unsupported-syntax` — warn | Preset level; fails under `--max-warnings 0`. | 0 |
|
||||
| src | `react-refresh/only-export-components` — warn | Fails under `--max-warnings 0`; `allowConstantExport` lets a module export constants beside its component. | 0 |
|
||||
| src | `@typescript-eslint/no-unused-expressions` — narrowed | `allowShortCircuit` + `allowTernary`: `cond && f()` / `a ? f() : g()` side-effect statements are an idiom here. | — |
|
||||
| src | `@typescript-eslint/no-floating-promises` — narrowed | `allowForKnownSafeCalls`: React Query's `invalidateQueries`, `refetchQueries`, `cancelQueries`, `resetQueries`, `prefetchQuery`, `prefetchInfiniteQuery` never reject unless asked to (`throwOnError`), so a bare call is safe by name. | 6 names |
|
||||
| e2e | `@typescript-eslint/no-floating-promises` — narrowed | The same `allowForKnownSafeCalls` list (the typed rules are one object for both scopes). | 6 names |
|
||||
| lib/http.ts | `no-restricted-globals` — narrowed | A network owner (`NETWORK_OWNERS` in eslint.config.js): the one module that may call `fetch` (`send`, which `req` and the public pages go through). Every other network global stays restricted there. | 1 global |
|
||||
| lib/api/core.ts | `no-restricted-globals` — narrowed | A network owner: `uploadReq` is the one `XMLHttpRequest` (upload progress). | 1 global |
|
||||
| lib/messagesSocket.ts | `no-restricted-globals` — narrowed | A network owner: the chat socket is the one `WebSocket`. | 1 global |
|
||||
| src tests | `no-restricted-globals`, `no-restricted-properties` — off | A test stands a double in for the network (`vi.stubGlobal("fetch", …)`) and may reach it directly. | — |
|
||||
| src tests | `local/literal-a11y-text`, `local/button-primitive`, `local/input-primitive`, `local/surface-role`, `local/glyph-link` — off | A test renders throwaway markup (a raw button as a focus target, an untranslated label) that is not product UI. | — |
|
||||
| FocusSafeButton.tsx | `local/focus-safe-disabled` — off | It IS the focus-safe primitive the rule points everyone else to. | — |
|
||||
@@ -51,10 +60,14 @@ none real in the sample; the curated typed pair `no-floating-promises` + `no-dep
|
||||
|
||||
| Scope | Narrowed | Why | Measured |
|
||||
| --- | --- | --- | --- |
|
||||
| config files | `eslint.config.js — not type-checked` | Linted (the `js configs` row) and executed by every lint run, but in no tsc project. Under the node project's `strict` + `checkJs` it reports 43 errors: 42 implicit-`any` parameters in the local rules' visitors (JSX nodes, which ESLint's own `Rule` typings do not model) and one inferred `string` severity. None is a defect; typing them means moving the local rules onto typescript-eslint's `TSESTree` types — a refactor of its own, not done in E32. | 43 (E32 review) |
|
||||
| all three projects | `skipLibCheck` | The `.d.ts` of installed packages are not type-checked (ours are). Standard for an app: a package's own types are its maintainers' gate, and checking them costs minutes and reports errors we cannot fix. | — |
|
||||
|
||||
Every project has `strict`, `noUncheckedIndexedAccess`, `noUnusedLocals`, `noUnusedParameters` and
|
||||
`noFallthroughCasesInSwitch` on; the test fails if one goes off.
|
||||
`noFallthroughCasesInSwitch` on; the test fails if one goes off, if a `strict` sub-flag
|
||||
(`strictNullChecks`, `noImplicitAny`, …) is set false beside it, if a project gains an `extends`,
|
||||
`exclude` or `files`, or if its `include` changes — and a config file at the frontend root that no
|
||||
project checks needs a row.
|
||||
|
||||
## Ruff (backend + scripts)
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { readFileSync } from "node:fs";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { readdirSync, readFileSync } from "node:fs";
|
||||
import { fileURLToPath, pathToFileURL } from "node:url";
|
||||
import { ESLint, type Linter } from "eslint";
|
||||
import prettierConfig from "eslint-config-prettier";
|
||||
import ts from "typescript";
|
||||
@@ -68,8 +68,16 @@ const TSC_OWNED = new Set(
|
||||
);
|
||||
const family = (rule: string) => (rule.includes("/") ? `${rule.split("/")[0]}/*` : rule);
|
||||
|
||||
// Each scope is checked through a sample file's EFFECTIVE config (what a rule family not applied there
|
||||
// means), and every config block that targets files is checked on its own (below) — so a block no
|
||||
// sample reaches fails rather than passing unseen (E32 review: a narrowing scoped to any other file
|
||||
// changed none of the sampled configs and needed no row).
|
||||
const SCOPES: Record<string, string> = {
|
||||
src: "src/components/Feed.tsx",
|
||||
"src/lib": "src/lib/format.ts",
|
||||
"lib/http.ts": "src/lib/http.ts",
|
||||
"lib/api/core.ts": "src/lib/api/core.ts",
|
||||
"lib/messagesSocket.ts": "src/lib/messagesSocket.ts",
|
||||
"src tests": "src/components/Feed.ytsort.dom.test.tsx",
|
||||
"FocusSafeButton.tsx": "src/components/ui/FocusSafeButton.tsx",
|
||||
"AnchoredPopover.tsx": "src/components/ui/AnchoredPopover.tsx",
|
||||
@@ -79,6 +87,55 @@ const SCOPES: Record<string, string> = {
|
||||
"js configs": "eslint.config.js",
|
||||
};
|
||||
|
||||
/** A flat-config `files` glob as a regex — the shapes this config uses: `**`, `*`, `{a,b}`. */
|
||||
function globRegex(glob: string): RegExp {
|
||||
let out = "";
|
||||
for (let i = 0; i < glob.length; i++) {
|
||||
const c = glob[i]!;
|
||||
if (glob.startsWith("**/", i)) {
|
||||
out += "(?:.*/)?";
|
||||
i += 2;
|
||||
} else if (c === "*") out += "[^/]*";
|
||||
else if (c === "{") {
|
||||
const end = glob.indexOf("}", i);
|
||||
out += `(?:${glob
|
||||
.slice(i + 1, end)
|
||||
.split(",")
|
||||
.join("|")})`;
|
||||
i = end;
|
||||
} else out += /[.+?^$()|[\]\\]/.test(c) ? `\\${c}` : c;
|
||||
}
|
||||
return new RegExp(`^${out}$`);
|
||||
}
|
||||
|
||||
type Block = Linter.Config & { files?: (string | string[])[]; ignores?: string[] };
|
||||
const blocks = async (): Promise<Block[]> =>
|
||||
((await import(pathToFileURL(`${FRONTEND}eslint.config.js`).href)) as { default: Block[] })
|
||||
.default;
|
||||
|
||||
// A rule option whose key says it lets something through (`allowShortCircuit`,
|
||||
// `allowForKnownSafeCalls`, `ignoreRestSiblings`, `exceptAfterSingleLine`) narrows the rule, and a
|
||||
// `no-restricted-*` list re-declared for a scope narrows it by every entry it leaves out.
|
||||
const RELAXING_KEY = /^(allow|ignore|except)/;
|
||||
const relaxes = (options: unknown[]): boolean =>
|
||||
options.some(
|
||||
(o) =>
|
||||
o !== null &&
|
||||
typeof o === "object" &&
|
||||
Object.entries(o).some(
|
||||
([k, v]) => RELAXING_KEY.test(k) && v !== false && !(Array.isArray(v) && v.length === 0),
|
||||
),
|
||||
);
|
||||
function restrictedEntries(entry: Linter.RuleEntry | undefined): Set<string> {
|
||||
const opts: unknown[] = Array.isArray(entry) ? entry.slice(1) : [];
|
||||
const flat = opts.flatMap((o) => {
|
||||
if (o === null || typeof o !== "object" || Array.isArray(o)) return [o];
|
||||
const { paths = [], patterns = [] } = o as { paths?: unknown[]; patterns?: unknown[] };
|
||||
return "paths" in o || "patterns" in o ? [...paths, ...patterns] : [o];
|
||||
});
|
||||
return new Set(flat.map((e) => JSON.stringify(e)));
|
||||
}
|
||||
|
||||
async function eslintNarrowings(): Promise<[string, string][]> {
|
||||
const eslint = new ESLint({ cwd: FRONTEND });
|
||||
const rules: Record<string, Rules> = {};
|
||||
@@ -91,28 +148,46 @@ async function eslintNarrowings(): Promise<[string, string][]> {
|
||||
// a core rule off because its typescript-eslint twin replaces it is reported as the twin
|
||||
(!rule.includes("/") && `@typescript-eslint/${rule}` in all);
|
||||
const found: [string, string][] = [];
|
||||
// src: everything at off or warn
|
||||
for (const [rule, v] of Object.entries(src))
|
||||
if (severity(v) < 2 && !ignorable(rule, src)) found.push(["src", rule]);
|
||||
// every other scope: what is WEAKER there than in src (a rule family not applied at all is one id)
|
||||
// src: everything at off or warn, and every rule whose options let something through
|
||||
for (const [rule, v] of Object.entries(src)) {
|
||||
if (ignorable(rule, src)) continue;
|
||||
if (severity(v) < 2 || (Array.isArray(v) && relaxes(v.slice(1)))) found.push(["src", rule]);
|
||||
}
|
||||
// every other scope: what is WEAKER there than in src (a rule family not applied at all is one
|
||||
// id). A src rule at warn counts too: switched off for a scope, it is narrowed there.
|
||||
for (const scope of Object.keys(SCOPES).filter((s) => s !== "src")) {
|
||||
const ids = new Set<string>();
|
||||
for (const [rule, v] of Object.entries(src)) {
|
||||
if (ignorable(rule, src) || severity(v) < 2) continue;
|
||||
if (ignorable(rule, src) || severity(v) === 0) continue;
|
||||
const there = severity(rules[scope]![rule]);
|
||||
if (there < 2 && !(there === 1 && severity(v) === 1))
|
||||
ids.add(there < 0 ? family(rule) : rule);
|
||||
if (there < severity(v)) ids.add(there < 0 ? family(rule) : rule);
|
||||
}
|
||||
for (const id of ids) found.push([scope, id]);
|
||||
}
|
||||
for (const ignored of eslintIgnores()) found.push(["everywhere", ignored]);
|
||||
return found;
|
||||
// every config block: a global `ignores` is a narrowing everywhere; one that targets files must
|
||||
// reach a sample, and what it re-declares is held to src
|
||||
for (const block of await blocks()) {
|
||||
if (!block.files) {
|
||||
if (Object.keys(block).join() === "ignores")
|
||||
for (const ignored of block.ignores ?? []) found.push(["everywhere", ignored]);
|
||||
continue;
|
||||
}
|
||||
|
||||
/** The global `ignores` of eslint.config.js (the first, file-less config object). */
|
||||
function eslintIgnores(): string[] {
|
||||
const m = /\{\s*ignores:\s*\[([^\]]*)\]\s*\}/.exec(read("frontend/eslint.config.js"));
|
||||
return m ? [...m[1]!.matchAll(/"([^"]+)"/g)].map((x) => x[1]!) : [];
|
||||
const globs = block.files.flat().map(globRegex);
|
||||
const scope = Object.keys(SCOPES).find((s) => globs.some((g) => g.test(SCOPES[s]!)));
|
||||
if (!scope) {
|
||||
found.push(["unsampled block", block.files.flat().join(", ")]);
|
||||
continue;
|
||||
}
|
||||
if (block.ignores?.length) found.push([scope, `ignores: ${block.ignores.join(", ")}`]);
|
||||
for (const [rule, v] of Object.entries(block.rules ?? {})) {
|
||||
if (ignorable(rule, src)) continue;
|
||||
if (rule.includes("no-restricted-")) {
|
||||
const own = restrictedEntries(v);
|
||||
if ([...restrictedEntries(src[rule])].some((e) => !own.has(e))) found.push([scope, rule]);
|
||||
} else if (Array.isArray(v) && relaxes(v.slice(1))) found.push([scope, rule]);
|
||||
}
|
||||
}
|
||||
return found;
|
||||
}
|
||||
|
||||
describe("the gate register (docs/gates.md)", () => {
|
||||
@@ -142,60 +217,146 @@ describe("the gate register (docs/gates.md)", () => {
|
||||
const opts = jsonc(path).compilerOptions as Record<string, unknown>;
|
||||
expect(REQUIRED_ON.filter((flag) => opts[flag] !== true)).toEqual([]);
|
||||
});
|
||||
// `strict: true` is a bundle: a sub-flag set false beside it, an `extends` this test cannot see
|
||||
// through, or a narrowed `include` / an `exclude` weakens the project while the five flags above
|
||||
// still read true (E32 review).
|
||||
const STRICT_FAMILY = [
|
||||
"noImplicitAny",
|
||||
"strictNullChecks",
|
||||
"strictFunctionTypes",
|
||||
"strictBindCallApply",
|
||||
"strictPropertyInitialization",
|
||||
"noImplicitThis",
|
||||
"alwaysStrict",
|
||||
"useUnknownInCatchVariables",
|
||||
"strictBuiltinIteratorReturn",
|
||||
];
|
||||
const INCLUDE: Record<string, string[]> = {
|
||||
"frontend/tsconfig.json": ["src"],
|
||||
"frontend/tsconfig.node.json": [
|
||||
"vite.config.ts",
|
||||
"vitest.config.ts",
|
||||
"reactPlugin.ts",
|
||||
"devOnlyGuard.ts",
|
||||
"playwright.config.ts",
|
||||
"postcss.config.js",
|
||||
],
|
||||
"frontend/e2e/tsconfig.json": [".", "../playwright.config.ts"],
|
||||
};
|
||||
it("lists every weakening compiler flag", () => {
|
||||
const found: [string, string][] = [];
|
||||
for (const path of PROJECTS) {
|
||||
const opts = jsonc(path).compilerOptions as Record<string, unknown>;
|
||||
const cfg = jsonc(path);
|
||||
const opts = cfg.compilerOptions as Record<string, unknown>;
|
||||
if (opts.skipLibCheck === true) found.push(["all three projects", "skipLibCheck"]);
|
||||
for (const flag of STRICT_FAMILY) if (opts[flag] === false) found.push([path, flag]);
|
||||
for (const key of ["extends", "exclude", "files"]) if (key in cfg) found.push([path, key]);
|
||||
expect(cfg.include, path).toEqual(INCLUDE[path]);
|
||||
}
|
||||
expect(missingFrom(found)).toEqual([]);
|
||||
});
|
||||
it("type-checks every config file at the frontend root, or lists it", () => {
|
||||
const checked = new Set(INCLUDE["frontend/tsconfig.node.json"]);
|
||||
const roots = readdirSync(FRONTEND).filter((f) => /[.](ts|js|mjs|cjs)$/.test(f));
|
||||
const found = roots
|
||||
.filter((f) => !checked.has(f))
|
||||
.map((f): [string, string] => ["config files", `${f} — not type-checked`]);
|
||||
expect(missingFrom(found)).toEqual([]);
|
||||
});
|
||||
|
||||
// --- Ruff -----------------------------------------------------------------------------------------
|
||||
it("lists every ruff ignore and immutable call", () => {
|
||||
// Ruff, pyright, pytest and knip are read as KEY whitelists: a key this test does not know (an
|
||||
// `extend-exclude`, a `lint.extend-ignore`, a narrowing `select`, a pyright `exclude`, a pytest
|
||||
// `-m "not slow"`, a knip `rules`) fails until someone decides whether it narrows (E32 review —
|
||||
// only `ignore` and the immutable calls used to be read).
|
||||
const tomlKeys = (toml: string) =>
|
||||
toml
|
||||
.split("\n")
|
||||
.map((l) => /^\s*([A-Za-z0-9_.-]+)\s*=/.exec(l)?.[1])
|
||||
.filter((k): k is string => k !== undefined);
|
||||
it("lists every ruff ignore and immutable call, and knows every key", () => {
|
||||
const toml = read("backend/ruff.toml");
|
||||
const list = (key: string) => {
|
||||
const m = new RegExp(`^${key}\\s*=\\s*\\[([^\\]]*)\\]`, "m").exec(toml);
|
||||
return m ? [...m[1]!.matchAll(/"([^"]+)"/g)].map((x) => x[1]!) : [];
|
||||
};
|
||||
expect(tomlKeys(toml).sort()).toEqual(
|
||||
["extend-immutable-calls", "extend-select", "ignore", "line-length", "target-version"].sort(),
|
||||
);
|
||||
// the selection on top of ruff's defaults (E, F) — dropping one narrows the lane
|
||||
expect(list("extend-select")).toEqual([
|
||||
"LOG",
|
||||
"G",
|
||||
"T20",
|
||||
"TRY400",
|
||||
"TRY401",
|
||||
"S110",
|
||||
"S112",
|
||||
"BLE001",
|
||||
"RUF100",
|
||||
"PLW1510",
|
||||
"B",
|
||||
]);
|
||||
const found: [string, string][] = [
|
||||
...list("ignore").map((code): [string, string] => ["backend + scripts", code]),
|
||||
...list("extend-immutable-calls").map((c): [string, string] => ["backend + scripts", c]),
|
||||
];
|
||||
expect(toml).not.toMatch(/per-file-ignores/); // none today — a new one needs a row and this line
|
||||
expect(missingFrom(found)).toEqual([]);
|
||||
});
|
||||
|
||||
// --- basedpyright ---------------------------------------------------------------------------------
|
||||
it("lists the type-check mode, the baseline and every rule override", () => {
|
||||
const PYRIGHT_KEYS = [
|
||||
"include",
|
||||
"extraPaths",
|
||||
"typeCheckingMode",
|
||||
"pythonVersion",
|
||||
"pythonPlatform",
|
||||
];
|
||||
it("lists the type-check mode, the baseline and every rule override, and knows every key", () => {
|
||||
const cfg = jsonc("backend/pyrightconfig.json");
|
||||
const found: [string, string][] = [];
|
||||
if (cfg.typeCheckingMode !== "all" && cfg.typeCheckingMode !== "strict")
|
||||
found.push(["backend/app + alembic", "typeCheckingMode"]);
|
||||
found.push(["backend/app + alembic", "baseline.json"]);
|
||||
const weak = [false, "none", "warning", "information", "hint"];
|
||||
for (const [key, value] of Object.entries(cfg))
|
||||
if (key.startsWith("report") && (value === false || value === "none" || value === "warning"))
|
||||
if (key.startsWith("report") && weak.includes(value as string | boolean))
|
||||
found.push(["backend/app + alembic", key]);
|
||||
expect(
|
||||
Object.keys(cfg).filter((k) => !k.startsWith("report") && !PYRIGHT_KEYS.includes(k)),
|
||||
).toEqual([]);
|
||||
expect(cfg.include).toEqual(["app", "alembic"]); // tests/ out — its row says why
|
||||
expect(missingFrom(found)).toEqual([]);
|
||||
});
|
||||
|
||||
// --- pytest ---------------------------------------------------------------------------------------
|
||||
it("keeps pytest strict: no deselection, no warning filters, --strict-markers", () => {
|
||||
it("keeps pytest strict: the whole tests/ tree, no selection, no warning filters, --strict-markers", () => {
|
||||
const ini = read("backend/pytest.ini");
|
||||
expect(ini).toMatch(/--strict-markers/);
|
||||
expect(ini).not.toMatch(/--deselect|--ignore|-p no:|filterwarnings|\s-k\s/);
|
||||
const keys = ini
|
||||
.split("\n")
|
||||
.filter((l) => !l.trim().startsWith("#"))
|
||||
.map((l) => /^([a-z_]+)\s*=\s*(.*)$/.exec(l))
|
||||
.filter((m) => m !== null);
|
||||
expect(Object.fromEntries(keys.map((m) => [m[1], m[2]!.trim()]))).toEqual({
|
||||
testpaths: "tests",
|
||||
pythonpath: ".",
|
||||
addopts: "-q -rs --strict-markers",
|
||||
});
|
||||
});
|
||||
|
||||
// --- knip -----------------------------------------------------------------------------------------
|
||||
it("lists every knip production exclusion and ignore", () => {
|
||||
it("lists every knip production exclusion and every other key, and runs both modes", () => {
|
||||
const cfg = jsonc("frontend/knip.jsonc") as { project?: string[] } & Record<string, unknown>;
|
||||
const found: [string, string][] = (cfg.project ?? [])
|
||||
.filter((p) => p.startsWith("!"))
|
||||
.map((p): [string, string] => ["--production", p]);
|
||||
for (const key of Object.keys(cfg).filter((k) => k.startsWith("ignore")))
|
||||
for (const key of Object.keys(cfg).filter(
|
||||
(k) => !["$schema", "project", "playwright"].includes(k),
|
||||
))
|
||||
found.push(["knip", key]);
|
||||
expect(missingFrom(found)).toEqual([]);
|
||||
const pkg = JSON.parse(read("frontend/package.json")) as { scripts: Record<string, string> };
|
||||
expect(pkg.scripts.knip).toBe("knip && knip --production");
|
||||
});
|
||||
|
||||
// --- accepted findings ----------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user