From 50c984eb9b6d723265ec69986c59f69b648b0e8f Mon Sep 17 00:00:00 2001 From: isamu Date: Thu, 27 Aug 2026 18:31:51 +0900 Subject: [PATCH] =?UTF-8?q?fix:=20severity=20=E3=81=AE=E5=A4=89=E6=8F=9B?= =?UTF-8?q?=E3=82=92=20eslint.config.js=20=E3=81=8B=E3=82=89=E5=87=BA?= =?UTF-8?q?=E3=81=97=E3=81=A6=E3=80=81=E3=83=86=E3=82=B9=E3=83=88=E3=81=A7?= =?UTF-8?q?=E5=AE=88=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #48 eslint.config.js はロジックを持っている。raise 変換がプリセットの warn を error に 上げる —— 「CI で warn は無価値」だから。そしてそれを守るものが何も無かった。 raise の中身を entry => entry にすると lint / lint:overrides / test すべて exit 0 の まま、実際には 14 件が error -> warn に落ちる。全部 security プラグインで、 detect-child-process / detect-eval-with-expression / detect-non-literal-fs-filename など。警告は CI を落とさず lint の出力を読むものも無いので、security の指摘 14 件が 黙って参考情報に格下げされても誰も気づかない。 これは型検査では捕まらない。raise は型としては正しいまま、意味だけが変わる。 raise / enforced を eslint.severity.js に出した。eslint.config.js が直接 import する ので TypeScript にはできない（ESLint はあのファイルをそのまま読む。.ts にすると 1 つの 関数のために jiti をローダに入れることになる）。型は eslint.severity.d.ts で与えている —— @ts-expect-error で黙らせない。 決め手のテストはフィクスチャではなく解決後の実 config に対して「プリセットのブロックに warn が 1 つも残っていない」と主張する。フィクスチャは壊れた変換とも辻褄が合うため。 eslint.config.d.ts はその import のために置いたもので、default を unknown[] と宣言して いる —— 制御できない形を構造的に主張しないため。読む側が絞る。 挙動保存: 旧 config と新 config の解決結果を全キー比較（497 キー、ルールの severity と ブロックの全キー）—— 差分 0。 break-verify: 4 変異とも撃墜（恒等関数 / "warn" だけ見て数値 1 を見落とす / オプションを捨てて "error" を返す / off まで上げてしまう）。 #48 は当初「型検査されていない 3 ファイル」だったが、2 つは #56/#59/#62 で解決済みで、 残る 1 つについては現実的な設定ミス 7 種を仕込んで測ったところ全部が既存のゲートに 捕まった（うちルール名の綴り違いは yarn lint が素通りさせ、#67 の lint:overrides だけが 捕まえる）。型検査を足すには eslint-plugin-security の declare module シムが要り、それは 実質 any なので安全を足さない。issue を実態に合わせて書き換えたうえで閉じる。 全ゲートを終了コードで確認: format:check 0 / lint 0 / typecheck 0 / test 0 (537 pass) / typecheck:summary 0（床 3 つとも維持）/ lint:overrides 0。 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GTPpB2eHQ9eovAs6QRNsTH --- eslint.config.d.ts | 9 ++++ eslint.config.js | 19 +------- eslint.severity.d.ts | 18 ++++++++ eslint.severity.js | 48 ++++++++++++++++++++ test/test_eslintSeverity.ts | 90 +++++++++++++++++++++++++++++++++++++ 5 files changed, 167 insertions(+), 17 deletions(-) create mode 100644 eslint.config.d.ts create mode 100644 eslint.severity.d.ts create mode 100644 eslint.severity.js create mode 100644 test/test_eslintSeverity.ts 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 9ca1a09..ec2f23b 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(", ")}`); +});