diff --git a/eslint.config.d.ts b/eslint.config.d.ts new file mode 100644 index 0000000..04c1af1 --- /dev/null +++ b/eslint.config.d.ts @@ -0,0 +1,9 @@ +/** Types for `eslint.config.js`. It is plain JS — ESLint loads it as-is — and its default export is + * a flat config array whose blocks come from plugins that do not all ship types. + * + * Declared as `unknown[]` on purpose. A structural type here would be this repository asserting a + * shape it does not control and cannot verify, which is the kind of claim the rest of this + * codebase spends its time deleting. Callers narrow what they read: see + * `scripts/overrides-report.ts`, which classifies every block and REPORTS the ones it cannot. */ +declare const config: unknown[]; +export default config; diff --git a/eslint.config.js b/eslint.config.js index b01547a..0739604 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -3,23 +3,8 @@ import tseslint from "typescript-eslint"; import sonarjs from "eslint-plugin-sonarjs"; import security from "eslint-plugin-security"; -// A preset decides WHICH rules to run; this file decides how much they matter. A warning does not -// fail CI and nothing here reads lint output, so a rule left at warn is a rule that reports a -// violation and ships it. Every preset rule is raised to error, and a rule that must not fail the -// build is turned off or downgraded BY NAME, with the reason, in one of the blocks below. -// -// Written as a transform rather than a list of rule names because the list is what rots: a preset -// that adds a warn-level rule in a future release arrives already enforced. Severity only — the -// preset's own options are preserved, and a rule it ships as `off` stays off (that was its -// decision, and a different one). -const raise = (entry) => { - const severity = Array.isArray(entry) ? entry[0] : entry; - if (severity !== 1 && severity !== "warn") return entry; - return Array.isArray(entry) ? ["error", ...entry.slice(1)] : "error"; -}; - -const enforced = (config) => - config.rules ? { ...config, rules: Object.fromEntries(Object.entries(config.rules).map(([id, entry]) => [id, raise(entry)])) } : config; +// Severity policy lives in its own file so it can be tested — see its header for what that buys. +import { enforced } from "./eslint.severity.js"; // Per-file entries further down come in three kinds, and the kind is the point. An `off` says the // rule is WRONG about that file and carries the reason. A `warn` is debt: the finding stays on diff --git a/eslint.severity.d.ts b/eslint.severity.d.ts new file mode 100644 index 0000000..bd182cc --- /dev/null +++ b/eslint.severity.d.ts @@ -0,0 +1,18 @@ +/** Types for `eslint.severity.js`, which is plain JS because `eslint.config.js` imports it and + * ESLint loads that file as-is — a `.ts` module would need `jiti` in the loader path for one + * function. Declared here rather than inferred so `test/` sees real types instead of `any`. */ + +/** A rule's configured severity, whether written bare (`"warn"`) or with options. */ +export type Severity = number | string; + +export type RuleEntry = Severity | [Severity, ...unknown[]]; + +/** Raised to `error` if the preset shipped it as a warning, returned untouched otherwise. */ +export declare const raise: (entry: RuleEntry) => RuleEntry; + +/** One preset config with every rule in it raised, and every other key left alone. + * + * The constraint is `Record` rather than a shape naming `rules`, because a flat + * config block carries whatever ESLint allows — `files`, `ignores`, `languageOptions`, `plugins`, + * `settings`, a preset's `name` — and a narrower type would reject the blocks this is FOR. */ +export declare const enforced: >(config: Block) => Block; diff --git a/eslint.severity.js b/eslint.severity.js new file mode 100644 index 0000000..7a84808 --- /dev/null +++ b/eslint.severity.js @@ -0,0 +1,48 @@ +/** How much a preset's rules matter here. + * + * A preset decides WHICH rules to run; this decides how much they matter. A warning does not fail + * CI and nothing here reads lint output, so a rule left at `warn` is a rule that reports a + * violation and ships it anyway. + * + * Written as a transform rather than a list of rule names because the list is what rots: a preset + * that adds a warn-level rule in a future release arrives already enforced. + * + * IN ITS OWN FILE BECAUSE IT IS LOGIC, and `eslint.config.js` is the one file no test could reach + * while it lived there. Measured: replacing the body of {@link raise} with `entry => entry` left + * `yarn lint`, `yarn lint:overrides` and `yarn test` all green while silently dropping FOURTEEN + * `security/*` rules from error to warn — `detect-child-process`, `detect-eval-with-expression`, + * `detect-non-literal-fs-filename` and eleven more. `test/test_eslintSeverity.ts` is what makes + * that mutation red. + * + * Plain `.js` with JSDoc types rather than `.ts`: `eslint.config.js` imports it directly, and + * ESLint loads that file as-is. A `.ts` module would need `jiti` in the loader path to be worth + * anything, which is a dependency for one function. */ + +/** A rule's configured severity, whether written bare (`"warn"`) or with options (`["warn", {…}]`). + * @typedef {number | string} Severity + * @typedef {Severity | [Severity, ...unknown[]]} RuleEntry */ + +/** Raised to `error` if the preset shipped it as a warning, and returned untouched otherwise. + * + * SEVERITY ONLY. The preset's own options are preserved — a rule configured + * `["warn", { max: 4 }]` becomes `["error", { max: 4 }]`, never `"error"` — and a rule the preset + * ships as `off` stays off, because that was its decision and a different one from "this matters + * less". Both numeric (`1`) and word (`"warn"`) forms count, since presets use both. + * + * @param {RuleEntry} entry + * @returns {RuleEntry} */ +export const raise = (entry) => { + const severity = Array.isArray(entry) ? entry[0] : entry; + if (severity !== 1 && severity !== "warn") return entry; + return Array.isArray(entry) ? ["error", ...entry.slice(1)] : "error"; +}; + +/** One preset config with every rule in it raised. A block carrying no `rules` is returned as-is: + * that is where the parser, the plugins and the globals live, and rewriting it would be rewriting + * how the repo is linted rather than how much its findings matter. + * + * @template {{ rules?: Record | undefined }} Block + * @param {Block} config + * @returns {Block} */ +export const enforced = (config) => + config.rules ? { ...config, rules: Object.fromEntries(Object.entries(config.rules).map(([id, entry]) => [id, raise(entry)])) } : config; diff --git a/test/test_eslintSeverity.ts b/test/test_eslintSeverity.ts new file mode 100644 index 0000000..c8f684d --- /dev/null +++ b/test/test_eslintSeverity.ts @@ -0,0 +1,90 @@ +// What a preset's severities become here, and what must survive untouched. +// +// This logic used to live inside `eslint.config.js`, where no test could reach it. That is not a +// tidiness point: with the body of `raise` replaced by `entry => entry`, `yarn lint`, +// `yarn lint:overrides` and `yarn test` all stayed green while FOURTEEN `security/*` rules +// silently dropped from error to warn — a warning fails nothing and nobody reads lint output, so +// those findings would have shipped. The last assertion below is the one that goes red for it. + +import { test } from "node:test"; +import assert from "node:assert/strict"; + +// Plain JS, typed by `eslint.severity.d.ts`: `eslint.config.js` imports it directly, so it cannot +// be TypeScript without putting `jiti` in ESLint's loader path for one function. +import { enforced, raise } from "../eslint.severity.js"; + +test("a warning becomes an error, in both the word and numeric forms presets use", () => { + assert.equal(raise("warn"), "error"); + assert.equal(raise(1), "error"); +}); + +test("everything that is not a warning is returned untouched", () => { + assert.equal(raise("error"), "error"); + assert.equal(raise(2), 2); + assert.equal(raise("off"), "off"); + assert.equal(raise(0), 0); +}); + +/** `off` is the case worth stating out loud. A preset that ships a rule disabled made a decision, + * and it is a different decision from "this matters less" — raising it would turn a rule the + * preset deliberately left alone into a build failure. */ +test("a rule the preset ships as off stays off", () => { + assert.equal(raise("off"), "off"); + assert.equal(raise(0), 0); + assert.deepEqual(raise(["off", { allow: ["x"] }]), ["off", { allow: ["x"] }]); +}); + +test("options survive the raise — severity only, never the whole entry", () => { + assert.deepEqual(raise(["warn", { max: 4 }]), ["error", { max: 4 }]); + assert.deepEqual(raise([1, { max: 4 }, "extra"]), ["error", { max: 4 }, "extra"]); + assert.deepEqual(raise(["error", { max: 4 }]), ["error", { max: 4 }]); +}); + +test("enforced raises every rule in a block and leaves the block's other keys alone", () => { + const block = { files: ["a.ts"], languageOptions: { globals: { x: "readonly" } }, rules: { a: "warn", b: "error", c: "off", d: [1, { max: 2 }] } }; + assert.deepEqual(enforced(block), { + files: ["a.ts"], + languageOptions: { globals: { x: "readonly" } }, + rules: { a: "error", b: "error", c: "off", d: ["error", { max: 2 }] }, + }); +}); + +/** A block carrying no `rules` is where the parser, the plugins and the globals live. Rewriting it + * would be rewriting HOW the repo is linted rather than how much its findings matter. */ +test("a block with no rules is returned as-is", () => { + const parser = { files: ["**/*.ts"], languageOptions: { parserOptions: { projectService: true } } }; + assert.deepEqual(enforced(parser), parser); + assert.deepEqual(enforced({ ignores: ["dist/"] }), { ignores: ["dist/"] }); +}); + +test("enforced does not mutate what it is given", () => { + const rules = { a: "warn" }; + const block = { rules }; + enforced(block); + assert.deepEqual(rules, { a: "warn" }, "the caller's object was rewritten in place"); +}); + +/** THE ONE THAT CATCHES A NEUTERED TRANSFORM. The others pin the function's shape; this pins what + * it is FOR. `eslint-plugin-security` ships every rule at `warn`, so if `raise` stops raising, + * fourteen security rules quietly become advisory and CI keeps passing. Asserted against the real + * resolved config rather than a fixture, because a fixture would agree with a broken transform. */ +test("the resolved config leaves no preset rule at warn — that is what the transform is for", async () => { + const config: unknown = (await import("../eslint.config.js")).default; + assert.ok(Array.isArray(config)); + + const warned: string[] = []; + config.forEach((block: unknown, index: number) => { + if (typeof block !== "object" || block === null) return; + const entries: Record = { ...block }; + // Per-file blocks are the DEBT ledger and are warn on purpose; the presets are not. + if (entries["files"] !== undefined) return; + const rules = entries["rules"]; + if (typeof rules !== "object" || rules === null) return; + Object.entries({ ...rules }).forEach(([id, entry]) => { + const severity: unknown = Array.isArray(entry) ? entry[0] : entry; + if (severity === "warn" || severity === 1) warned.push(`${index}:${id}`); + }); + }); + + assert.deepEqual(warned, [], `preset rules left at warn, so their findings would ship: ${warned.join(", ")}`); +});